[ Talking to myself while mulling this series over ... ]
On Thu, Sep 2, 2021 at 4:34 PM Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> Instead of having it lock/unlock halfway through the function (and
> have magic "Oh, the caller already holds the lock, so don't lock"
> semantics except with misleading names) I really think that function
> should just have been split in two, and then the locked region can be
> minimized in the caller only taking it for the first part.
If there's some reason why it can't sanely be split into two (too many
common variables or some odd control flow or whatever), at least the
locking logic should be changed from
if (lock)
local_irq_save(flags);
to something along the lines of
if (!caller_already_locked)
local_irq_save(flags);
so that when you read that function on its own, it's clear that the
lock is always held over that critical section - and the issue is that
perhaps the lock was already taken by the caller.
This ignores the whole misleading "lock" name when it isn't even yet a lock.
Linus
