| Message ID | 20241121162826.987947-1-surenb@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/3] seqlock: add raw_seqcount_try_begin | expand |
On 21.11.24 17:28, Suren Baghdasaryan wrote: > Add raw_seqcount_try_begin() to opens a read critical section of the given > seqcount_t if the counter is even. This enables eliding the critical > section entirely if the counter is odd, instead of doing the speculation > knowing it will fail. > > Suggested-by: Peter Zijlstra <peterz@infradead.org> > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > --- > Applies over Linus' ToT > > include/linux/seqlock.h | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h > index 5298765d6ca4..22c2c48b4265 100644 > --- a/include/linux/seqlock.h > +++ b/include/linux/seqlock.h > @@ -318,6 +318,28 @@ SEQCOUNT_LOCKNAME(mutex, struct mutex, true, mutex) > __seq; \ > }) > > +/** > + * raw_seqcount_try_begin() - begin a seqcount_t read critical section > + * w/o lockdep and w/o counter stabilization > + * @s: Pointer to seqcount_t or any of the seqcount_LOCKNAME_t variants > + * > + * Similar to raw_seqcount_begin(), except it enables eliding the critical > + * section entirely if odd, instead of doing the speculation knowing it will > + * fail. > + * > + * Useful when counter stabilization is more or less equivalent to taking > + * the lock and there is a slowpath that does that. > + * > + * If true, start will be set to the (even) sequence count read. > + * > + * Return: true when a read critical section is started. > + */ > +#define raw_seqcount_try_begin(s, start) \ > +({ \ > + start = raw_read_seqcount(s); \ > + !(start & 1); \ > +}) In gup_fast(), we simply do seq = raw_read_seqcount(¤t->mm->write_protect_seq); if (seq & 1) return 0; Should we be using that there as well? if (!raw_seqcount_try_begin(¤t->mm->write_protect_seqs, seq)) return 0;
On Fri, Nov 22, 2024 at 12:10:29PM +0100, David Hildenbrand wrote: > In gup_fast(), we simply do > > seq = raw_read_seqcount(¤t->mm->write_protect_seq); > if (seq & 1) > return 0; > > Should we be using that there as well? > > if (!raw_seqcount_try_begin(¤t->mm->write_protect_seqs, seq)) > return 0; Might as well. A quick grep doesn't find me another instance of this pattern, but does find me something 'funny' in net/netfilter/x_tables.c. Let's pretend I didn't see that for now ... *sigh* Want me to stick a patch like this on, or do you want to do that later, when the dust has settled?
On 22.11.24 12:19, Peter Zijlstra wrote: > On Fri, Nov 22, 2024 at 12:10:29PM +0100, David Hildenbrand wrote: > >> In gup_fast(), we simply do >> >> seq = raw_read_seqcount(¤t->mm->write_protect_seq); >> if (seq & 1) >> return 0; >> >> Should we be using that there as well? >> >> if (!raw_seqcount_try_begin(¤t->mm->write_protect_seqs, seq)) >> return 0; > > Might as well. A quick grep doesn't find me another instance of this > pattern, but does find me something 'funny' in net/netfilter/x_tables.c. > Let's pretend I didn't see that for now ... *sigh* :) I'm also not 100% sure about barriers in gup_fast() around the raw_seqcount .... and I'm pretending I didn't see any of that as well ... > > Want me to stick a patch like this on, or do you want to do that later, > when the dust has settled? Feel free to add a patch to just change that as well, it's an easy change. For this patch Reviewed-by: David Hildenbrand <david@redhat.com>
* Suren Baghdasaryan <surenb@google.com> [241121 11:28]: > Add raw_seqcount_try_begin() to opens a read critical section of the given > seqcount_t if the counter is even. This enables eliding the critical > section entirely if the counter is odd, instead of doing the speculation > knowing it will fail. > > Suggested-by: Peter Zijlstra <peterz@infradead.org> > Signed-off-by: Suren Baghdasaryan <surenb@google.com> Reviewed-by: Liam R. Howlett <Liam.Howlett@Oracle.com> > --- > Applies over Linus' ToT > > include/linux/seqlock.h | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h > index 5298765d6ca4..22c2c48b4265 100644 > --- a/include/linux/seqlock.h > +++ b/include/linux/seqlock.h > @@ -318,6 +318,28 @@ SEQCOUNT_LOCKNAME(mutex, struct mutex, true, mutex) > __seq; \ > }) > > +/** > + * raw_seqcount_try_begin() - begin a seqcount_t read critical section > + * w/o lockdep and w/o counter stabilization > + * @s: Pointer to seqcount_t or any of the seqcount_LOCKNAME_t variants > + * > + * Similar to raw_seqcount_begin(), except it enables eliding the critical > + * section entirely if odd, instead of doing the speculation knowing it will > + * fail. > + * > + * Useful when counter stabilization is more or less equivalent to taking > + * the lock and there is a slowpath that does that. > + * > + * If true, start will be set to the (even) sequence count read. > + * > + * Return: true when a read critical section is started. > + */ > +#define raw_seqcount_try_begin(s, start) \ > +({ \ > + start = raw_read_seqcount(s); \ > + !(start & 1); \ > +}) > + > /** > * raw_seqcount_begin() - begin a seqcount_t read critical section w/o > * lockdep and w/o counter stabilization > > base-commit: 43fb83c17ba2d63dfb798f0be7453ed55ca3f9c2 > -- > 2.47.0.338.g60cca15819-goog >
diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h index 5298765d6ca4..22c2c48b4265 100644 --- a/include/linux/seqlock.h +++ b/include/linux/seqlock.h @@ -318,6 +318,28 @@ SEQCOUNT_LOCKNAME(mutex, struct mutex, true, mutex) __seq; \ }) +/** + * raw_seqcount_try_begin() - begin a seqcount_t read critical section + * w/o lockdep and w/o counter stabilization + * @s: Pointer to seqcount_t or any of the seqcount_LOCKNAME_t variants + * + * Similar to raw_seqcount_begin(), except it enables eliding the critical + * section entirely if odd, instead of doing the speculation knowing it will + * fail. + * + * Useful when counter stabilization is more or less equivalent to taking + * the lock and there is a slowpath that does that. + * + * If true, start will be set to the (even) sequence count read. + * + * Return: true when a read critical section is started. + */ +#define raw_seqcount_try_begin(s, start) \ +({ \ + start = raw_read_seqcount(s); \ + !(start & 1); \ +}) + /** * raw_seqcount_begin() - begin a seqcount_t read critical section w/o * lockdep and w/o counter stabilization
Add raw_seqcount_try_begin() to opens a read critical section of the given seqcount_t if the counter is even. This enables eliding the critical section entirely if the counter is odd, instead of doing the speculation knowing it will fail. Suggested-by: Peter Zijlstra <peterz@infradead.org> Signed-off-by: Suren Baghdasaryan <surenb@google.com> --- Applies over Linus' ToT include/linux/seqlock.h | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) base-commit: 43fb83c17ba2d63dfb798f0be7453ed55ca3f9c2