All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Marco Elver <elver@google.com>
Cc: Frederic Weisbecker <frederic@kernel.org>,
	Ingo Molnar <mingo@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
	Dmitry Vyukov <dvyukov@google.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	linuxppc-dev@lists.ozlabs.org, linux-perf-users@vger.kernel.org,
	x86@kernel.org, linux-sh@vger.kernel.org,
	kasan-dev@googlegroups.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 10/14] locking/percpu-rwsem: Add percpu_is_write_locked() and percpu_is_read_locked()
Date: Wed, 17 Aug 2022 14:47:52 +0200	[thread overview]
Message-ID: <YvzjeEHYX9d5dhAt@worktop.programming.kicks-ass.net> (raw)
In-Reply-To: <20220704150514.48816-11-elver@google.com>

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

> +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.

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

WARNING: multiple messages have this Message-ID (diff)
From: Peter Zijlstra <peterz@infradead.org>
To: Marco Elver <elver@google.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	linux-sh@vger.kernel.org,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Frederic Weisbecker <frederic@kernel.org>,
	x86@kernel.org, linuxppc-dev@lists.ozlabs.org,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
	kasan-dev@googlegroups.com, Namhyung Kim <namhyung@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Jiri Olsa <jolsa@redhat.com>, Ingo Molnar <mingo@kernel.org>,
	Dmitry Vyukov <dvyukov@google.com>
Subject: Re: [PATCH v3 10/14] locking/percpu-rwsem: Add percpu_is_write_locked() and percpu_is_read_locked()
Date: Wed, 17 Aug 2022 14:47:52 +0200	[thread overview]
Message-ID: <YvzjeEHYX9d5dhAt@worktop.programming.kicks-ass.net> (raw)
In-Reply-To: <20220704150514.48816-11-elver@google.com>

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

> +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.

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

  parent reply	other threads:[~2022-08-17 12:48 UTC|newest]

Thread overview: 98+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-04 15:05 [PATCH v3 00/14] perf/hw_breakpoint: Optimize for thousands of tasks Marco Elver
2022-07-04 15:05 ` Marco Elver
2022-07-04 15:05 ` [PATCH v3 01/14] perf/hw_breakpoint: Add KUnit test for constraints accounting Marco Elver
2022-07-04 15:05   ` Marco Elver
2022-07-04 15:10   ` Dmitry Vyukov
2022-07-04 15:10     ` Dmitry Vyukov
2022-07-20 15:22     ` Ian Rogers
2022-07-20 15:22       ` Ian Rogers
2022-07-21 16:22   ` Mark Rutland
2022-07-21 16:22     ` Mark Rutland
2022-07-22  9:10     ` Will Deacon
2022-07-22  9:10       ` Will Deacon
2022-07-22  9:20       ` Dmitry Vyukov
2022-07-22  9:20         ` Dmitry Vyukov
2022-07-22 10:10         ` Will Deacon
2022-07-22 10:10           ` Will Deacon
2022-07-22 10:31           ` Dmitry Vyukov
2022-07-22 10:31             ` Dmitry Vyukov
2022-07-22 11:03             ` Will Deacon
2022-07-22 11:03               ` Will Deacon
2022-07-22 13:41               ` Dmitry Vyukov
2022-07-22 13:41                 ` Dmitry Vyukov
2022-07-25 11:00     ` Marco Elver
2022-07-25 11:00       ` Marco Elver
2022-07-04 15:05 ` [PATCH v3 02/14] perf/hw_breakpoint: Provide hw_breakpoint_is_used() and use in test Marco Elver
2022-07-04 15:05   ` Marco Elver
2022-07-04 15:09   ` Dmitry Vyukov
2022-07-04 15:09     ` Dmitry Vyukov
2022-07-20 15:22     ` Ian Rogers
2022-07-20 15:22       ` Ian Rogers
2022-07-04 15:05 ` [PATCH v3 03/14] perf/hw_breakpoint: Clean up headers Marco Elver
2022-07-04 15:05   ` Marco Elver
2022-07-20 15:23   ` Ian Rogers
2022-07-20 15:23     ` Ian Rogers
2022-07-04 15:05 ` [PATCH v3 04/14] perf/hw_breakpoint: Optimize list of per-task breakpoints Marco Elver
2022-07-04 15:05   ` Marco Elver
2022-07-20 15:29   ` Ian Rogers
2022-07-20 15:29     ` Ian Rogers
2022-07-20 15:39     ` Marco Elver
2022-07-20 15:39       ` Marco Elver
2022-07-04 15:05 ` [PATCH v3 05/14] perf/hw_breakpoint: Mark data __ro_after_init Marco Elver
2022-07-04 15:05   ` Marco Elver
2022-07-20 15:30   ` Ian Rogers
2022-07-20 15:30     ` Ian Rogers
2022-07-04 15:05 ` [PATCH v3 06/14] perf/hw_breakpoint: Optimize constant number of breakpoint slots Marco Elver
2022-07-04 15:05   ` Marco Elver
2022-07-20 15:31   ` Ian Rogers
2022-07-20 15:31     ` Ian Rogers
2022-07-04 15:05 ` [PATCH v3 07/14] perf/hw_breakpoint: Make hw_breakpoint_weight() inlinable Marco Elver
2022-07-04 15:05   ` Marco Elver
2022-07-20 15:32   ` Ian Rogers
2022-07-20 15:32     ` Ian Rogers
2022-07-04 15:05 ` [PATCH v3 08/14] perf/hw_breakpoint: Remove useless code related to flexible breakpoints Marco Elver
2022-07-04 15:05   ` Marco Elver
2022-07-20 15:34   ` Ian Rogers
2022-07-20 15:34     ` Ian Rogers
2022-07-04 15:05 ` [PATCH v3 09/14] powerpc/hw_breakpoint: Avoid relying on caller synchronization Marco Elver
2022-07-04 15:05   ` Marco Elver
2022-07-20 15:35   ` Ian Rogers
2022-07-20 15:35     ` Ian Rogers
2022-07-04 15:05 ` [PATCH v3 10/14] locking/percpu-rwsem: Add percpu_is_write_locked() and percpu_is_read_locked() Marco Elver
2022-07-04 15:05   ` Marco Elver
2022-07-20 15:36   ` Ian Rogers
2022-07-20 15:36     ` Ian Rogers
2022-08-17 12:47   ` Peter Zijlstra [this message]
2022-08-17 12:47     ` Peter Zijlstra
2022-08-29  6:00     ` Marco Elver
2022-08-29  6:00       ` Marco Elver
2022-07-04 15:05 ` [PATCH v3 11/14] perf/hw_breakpoint: Reduce contention with large number of tasks Marco Elver
2022-07-04 15:05   ` Marco Elver
2022-07-20 15:38   ` Ian Rogers
2022-07-20 15:38     ` Ian Rogers
2022-08-17 13:03   ` Peter Zijlstra
2022-08-17 13:03     ` Peter Zijlstra
2022-08-17 13:14     ` Marco Elver
2022-08-17 13:14       ` Marco Elver
2022-08-29  8:38       ` Peter Zijlstra
2022-08-29  8:38         ` Peter Zijlstra
2022-08-29  9:38         ` Marco Elver
2022-08-29  9:38           ` Marco Elver
2022-07-04 15:05 ` [PATCH v3 12/14] perf/hw_breakpoint: Introduce bp_slots_histogram Marco Elver
2022-07-04 15:05   ` Marco Elver
2022-07-20 15:40   ` Ian Rogers
2022-07-20 15:40     ` Ian Rogers
2022-07-04 15:05 ` [PATCH v3 13/14] perf/hw_breakpoint: Optimize max_bp_pinned_slots() for CPU-independent task targets Marco Elver
2022-07-04 15:05   ` Marco Elver
2022-07-20 15:42   ` Ian Rogers
2022-07-20 15:42     ` Ian Rogers
2022-07-04 15:05 ` [PATCH v3 14/14] perf/hw_breakpoint: Optimize toggle_bp_slot() " Marco Elver
2022-07-04 15:05   ` Marco Elver
2022-07-20 15:44   ` Ian Rogers
2022-07-20 15:44     ` Ian Rogers
2022-07-12 13:39 ` [PATCH v3 00/14] perf/hw_breakpoint: Optimize for thousands of tasks Marco Elver
2022-07-12 13:39   ` Marco Elver
2022-07-20 15:47   ` Ian Rogers
2022-07-20 15:47     ` Ian Rogers
2022-08-16 14:12     ` Marco Elver
2022-08-16 14:12       ` Marco Elver

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YvzjeEHYX9d5dhAt@worktop.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=dvyukov@google.com \
    --cc=elver@google.com \
    --cc=frederic@kernel.org \
    --cc=jolsa@redhat.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=linux-sh@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@kernel.org \
    --cc=mpe@ellerman.id.au \
    --cc=namhyung@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.