Skip to content

Revert "sched/spinlock: remove nesting spinlock support" #14190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
anchao merged 1 commit into from
Oct 13, 2024
Merged

Revert "sched/spinlock: remove nesting spinlock support" #14190

anchao merged 1 commit into from
Oct 13, 2024

Conversation

@crafcat7
Copy link
Contributor

@crafcat7 crafcat7 commented Oct 12, 2024

Summary

This reverts commit 5aa13bc, Related to #14167

I found that this submission would cause ci warning, So should we revert it first?

machine/arch_atomic.c: In function ‘__atomic_load_2’:
machine/arch_atomic.c:51:22: warning: infinite recursion detected [-Winfinite-recursion]
   51 |   type weak_function __atomic_load_##n (FAR const volatile void *ptr, \
      |                      ^~~~~~~~~~~~~~
machine/arch_atomic.c:354:1: note: in expansion of macro ‘LOAD’
  354 | LOAD(2, uint16_t)
      | ^~~~
In file included from /home/crafcat7/SSD/vela/nuttx.git/nuttx/include/nuttx/atomic.h:84,
                 from /home/crafcat7/SSD/vela/nuttx.git/nuttx/include/nuttx/fs/fs.h:40,
                 from /home/crafcat7/SSD/vela/nuttx.git/nuttx/include/nuttx/sched.h:47,
                 from /home/crafcat7/SSD/vela/nuttx.git/nuttx/include/nuttx/arch.h:89,
                 from /home/crafcat7/SSD/vela/nuttx.git/nuttx/include/nuttx/spinlock.h:38,
                 from machine/arch_atomic.c:31:
In function ‘spin_lock_wo_note’,
    inlined from ‘spin_lock_irqsave_wo_note’ at /home/crafcat7/SSD/vela/nuttx.git/nuttx/include/nuttx/spinlock.h:531:7,
    inlined from ‘spin_lock_irqsave’ at /home/crafcat7/SSD/vela/nuttx.git/nuttx/include/nuttx/spinlock.h:586:11,
    inlined from ‘__atomic_load_2’ at machine/arch_atomic.c:354:1:
/home/crafcat7/SSD/vela/nuttx.git/nuttx/include/nuttx/spinlock.h:244:10: note: recursive call
  244 |   while (atomic_load((FAR atomic_ushort *)&lock->tickets.owner) != ticket)
      |          ^~~~~~~~~~~
In function ‘spin_lock_wo_note’,
    inlined from ‘spin_lock_irqsave_wo_note’ at /home/crafcat7/SSD/vela/nuttx.git/nuttx/include/nuttx/spinlock.h:535:7,
    inlined from ‘spin_lock_irqsave’ at /home/crafcat7/SSD/vela/nuttx.git/nuttx/include/nuttx/spinlock.h:586:11,
    inlined from ‘__atomic_load_2’ at machine/arch_atomic.c:354:1:
/home/crafcat7/SSD/vela/nuttx.git/nuttx/include/nuttx/spinlock.h:244:10: note: recursive call
  244 |   while (atomic_load((FAR atomic_ushort *)&lock->tickets.owner) != ticket)
      |          ^~~~~~~~~~~
machine/arch_atomic.c: In function ‘__atomic_fetch_add_2’:
machine/arch_atomic.c:120:22: warning: infinite recursion detected [-Winfinite-recursion]
  120 |   type weak_function __atomic_fetch_add_##n (FAR volatile void *ptr,   \
      |                      ^~~~~~~~~~~~~~~~~~~
machine/arch_atomic.c:450:1: note: in expansion of macro ‘FETCH_ADD’
  450 | FETCH_ADD(2, uint16_t)
      | ^~~~~~~~~
In function ‘spin_lock_wo_note’,
    inlined from ‘spin_lock_irqsave_wo_note’ at /home/crafcat7/SSD/vela/nuttx.git/nuttx/include/nuttx/spinlock.h:531:7,
    inlined from ‘spin_lock_irqsave’ at /home/crafcat7/SSD/vela/nuttx.git/nuttx/include/nuttx/spinlock.h:586:11,
    inlined from ‘__atomic_fetch_add_2’ at machine/arch_atomic.c:450:1:
/home/crafcat7/SSD/vela/nuttx.git/nuttx/include/nuttx/spinlock.h:243:5: note: recursive call
  243 |     atomic_fetch_add((FAR atomic_ushort *)&lock->tickets.next, 1);
      |     ^~~~~~~~~~~~~~~~
In function ‘spin_lock_wo_note’,
    inlined from ‘spin_lock_irqsave_wo_note’ at /home/crafcat7/SSD/vela/nuttx.git/nuttx/include/nuttx/spinlock.h:535:7,
    inlined from ‘spin_lock_irqsave’ at /home/crafcat7/SSD/vela/nuttx.git/nuttx/include/nuttx/spinlock.h:586:11,
    inlined from ‘__atomic_fetch_add_2’ at machine/arch_atomic.c:450:1:
/home/crafcat7/SSD/vela/nuttx.git/nuttx/include/nuttx/spinlock.h:243:5: note: recursive call
  243 |     atomic_fetch_add((FAR atomic_ushort *)&lock->tickets.next, 1);
      |     ^~~~~~~~~~~~~~~~

Impact

None,it just fix CI warning

Testing

Build Host(s): Linux x86
Target(s): sim/smp

After that, there will be no warnings for compilation

╭─crafcat7@hp-prodesk-680-g4-mt ~/SSD/vela/nuttx.git/nuttx ‹master› 
╰─$ ./tools/configure.sh -l sim:smp                                                                                                                                          130 ↵
  Copy files
  Select CONFIG_HOST_LINUX=y
  Refreshing...
CP: arch/dummy/Kconfig to /home/crafcat7/SSD/vela/nuttx.git/nuttx/arch/dummy/dummy_kconfig
CP: boards/dummy/Kconfig to /home/crafcat7/SSD/vela/nuttx.git/nuttx/boards/dummy/dummy_kconfig
LN: platform/board to /home/crafcat7/SSD/vela/nuttx.git/apps/platform/dummy
LN: include/arch to arch/sim/include
LN: include/arch/board to /home/crafcat7/SSD/vela/nuttx.git/nuttx/boards/sim/sim/sim/include
LN: drivers/platform to /home/crafcat7/SSD/vela/nuttx.git/nuttx/drivers/dummy
LN: include/arch/chip to /home/crafcat7/SSD/vela/nuttx.git/nuttx/arch/sim/include/sim
LN: arch/sim/src/chip to /home/crafcat7/SSD/vela/nuttx.git/nuttx/arch/sim/src/sim
LN: arch/sim/src/board to /home/crafcat7/SSD/vela/nuttx.git/nuttx/boards/sim/sim/sim/src
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/audioutils
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/benchmarks
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/boot
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/canutils
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/crypto
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/database
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/examples/mcuboot
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/examples
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/fsutils
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/games
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/graphics
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/industry
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/inertial
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/interpreters/luamodules
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/interpreters
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/logging
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/lte
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/math
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/mlearning
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/netutils
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/sdr
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/system
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/testing
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/videoutils
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/wireless/bluetooth
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/wireless/ieee802154
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps/wireless
mkkconfig in /home/crafcat7/SSD/vela/nuttx.git/apps
Loaded configuration '.config'
Configuration saved to '.config'
╭─crafcat7@hp-prodesk-680-g4-mt ~/SSD/vela/nuttx.git/nuttx ‹master› 
╰─$ make -j
Create version.h
LN: platform/board to /home/crafcat7/SSD/vela/nuttx.git/apps/platform/dummy
Register: hello
Register: nsh
Register: sh
Register: getprime
Register: smp
Register: ostest
Register: taskset
CP:  /home/crafcat7/SSD/vela/nuttx.git/nuttx/include/nuttx/config.h
CP:  /home/crafcat7/SSD/vela/nuttx.git/nuttx/include/nuttx/fs/hostfs.h
LD:  nuttx

@github-actions github-actions bot added Area: OS Components OS Components issues Size: S The size of the change in this PR is small labels Oct 12, 2024
@GUIDINGLI
Copy link
Contributor

GUIDINGLI commented Oct 12, 2024

@anchao

@crafcat7 crafcat7 mentioned this pull request Oct 12, 2024
Closed
1 task
@xiaoxiang781216 xiaoxiang781216 linked an issue Oct 12, 2024 that may be closed by this pull request
[BUG] new warnings when building sim/smp #14167
Closed
1 task
anchao
anchao approved these changes Oct 12, 2024
View reviewed changes
Copy link
Contributor

@anchao anchao left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to be a problem with the implementation of Spinlock. I am out of office now and let us revert first

@crafcat7
Copy link
Contributor Author

crafcat7 commented Oct 12, 2024

I sorted out the reasons for the compilation warning.
If we use NuttX atomic, then there is in the process

spin_lock_irqsave->spin_lock_irqsave_wo_note->spin_lock_wo_note

and the atomic function is also used in spin_lock_wo_note

@xiaoxiang781216
Copy link
Contributor

xiaoxiang781216 commented Oct 12, 2024

I sorted out the reasons for the compilation warning. If we use NuttX atomic, then there is in the process

spin_lock_irqsave->spin_lock_irqsave_wo_note->spin_lock_wo_note

and the atomic function is also used in spin_lock_wo_note

@crafcat7 let's fix the warning instead reverting the patch.

@masayuki2009
Copy link
Contributor

masayuki2009 commented Oct 12, 2024 •
edited
Loading

@xiaoxiang781216

I wrote the following comments in
#14079 (comment)

I noticed that spresense:wifi_smp causes a deadlock with this PR.
I think this PR would affect some SMP configurations that use real hardware.
So I suggest we revert this PR until all the issues have been resolved.

@anchao anchao merged commit 9e81f5e into apache:master Oct 13, 2024
This was referenced Oct 13, 2024
Closed
Closed
@hujun260 hujun260 mentioned this pull request Oct 13, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anchao anchao anchao approved these changes

Assignees

No one assigned

Labels

Area: OS Components OS Components issues Size: S The size of the change in this PR is small

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] new warnings when building sim/smp

5 participants

@crafcat7 @GUIDINGLI @xiaoxiang781216 @masayuki2009 @anchao