Re: [PATCH 4/7] sempahore: add a helper for a concurrency limiter

[Linus Torvalds]

On Thu, Mar 30, 2023 at 4:56 AM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>
> Sure thing; still completely untested...

Seems obvious enough.

Looking at the people who use a semaphore as a mutex, one core user stands out:

>  kernel/printk/printk.c                                        |    2 +-

.. and I'm not entirely sure why that uses a semaphore. It may be
*entirely* legacy, and should just be changed to be a mutex.

But it may also be that the 'console_sem' has some subtle reason why
it wants to be a semaphore, and why it then plays games with lockdep
(which doesn't support counting semaphores) and does things like

  #define down_console_sem() do { \
        down(&console_sem);\
        mutex_acquire(&console_lock_dep_map, 0, 0, _RET_IP_);\
  } while (0)

Anyway, I think your patch is obviously safe as-is, and I think it's
long overdue to make it clear that the only real reason to use
semaphores rather than mutexes is if you do need the counting thing.

Of course, there is the thing about lockdep, and also about how
semaphores these days have no architecture-specific parts, so if
anybody wants to play deep games with their locking, that may be a
reason for using them.

Although we also do have some other issues - I think down_trylock() is
ok in irq contexts, but mutex_trylock() is not. Maybe that's why
printk uses semaphores? I forget.

                Linus