On Wed, 17 Aug 2022 at 14:48, Peter Zijlstra [off-list ref] wrote:

On Mon, Jul 04, 2022 at 05:05:10PM +0200, Marco Elver wrote:

quoted

+bool percpu_is_read_locked(struct percpu_rw_semaphore *sem)
+{
+     return per_cpu_sum(*sem->read_count) != 0;
+}
+EXPORT_SYMBOL_GPL(percpu_is_read_locked);

I don't think this is correct; read_count can have spurious increments.

If we look at __percpu_down_read_trylock(), it does roughly something
like this:

        this_cpu_inc(*sem->read_count);
        smp_mb();
        if (!sem->block)
                return true;
        this_cpu_dec(*sem->read_count);
        return false;

So percpu_is_read_locked() needs to ensure the read_count is non-zero
*and* that block is not set.

I shall go and fix. v4 incoming (if more comments before that, please shout).

That said; I really dislike the whole _is_locked family with a passion.
Let me try and figure out what you need this for.

As in the other email, it's for the dbg_*() functions for kgdb's
benefit (avoiding deadlock if kgdb wants a breakpoint, while we're in
the process of handing out a breakpoint elsewhere and have the locks
taken).

Thanks,
-- Marco