| Message ID | 3066500d7a6eb3e03f10adf98b87fdb3b1c49db8.1634822969.git.asml.silence@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | optimise blk_try_enter_queue() | expand |
Hello, On Thu, Oct 21, 2021 at 02:30:51PM +0100, Pavel Begunkov wrote: > Add percpu_ref_tryget_live_rcu(), which is a version of > percpu_ref_tryget_live() but the user is responsible for enclosing it in > a RCU read lock section. > > Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> > --- > include/linux/percpu-refcount.h | 33 +++++++++++++++++++++++---------- > 1 file changed, 23 insertions(+), 10 deletions(-) > > diff --git a/include/linux/percpu-refcount.h b/include/linux/percpu-refcount.h > index ae16a9856305..b31d3f3312ce 100644 > --- a/include/linux/percpu-refcount.h > +++ b/include/linux/percpu-refcount.h > @@ -266,6 +266,28 @@ static inline bool percpu_ref_tryget(struct percpu_ref *ref) > return percpu_ref_tryget_many(ref, 1); > } > > +/** > + * percpu_ref_tryget_live_rcu - same as percpu_ref_tryget_live() but the > + * caller is responsible for taking RCU. > + * > + * This function is safe to call as long as @ref is between init and exit. > + */ > +static inline bool percpu_ref_tryget_live_rcu(struct percpu_ref *ref) > +{ > + unsigned long __percpu *percpu_count; > + bool ret = false; > + > + WARN_ON_ONCE(!rcu_read_lock_held()); > + > + if (likely(__ref_is_percpu(ref, &percpu_count))) { > + this_cpu_inc(*percpu_count); > + ret = true; > + } else if (!(ref->percpu_count_ptr & __PERCPU_REF_DEAD)) { > + ret = atomic_long_inc_not_zero(&ref->data->count); > + } > + return ret; > +} > + > /** > * percpu_ref_tryget_live - try to increment a live percpu refcount > * @ref: percpu_ref to try-get Nit: it's dumb convention at this point, but do you mind copying this guy up. I like consistency. > @@ -283,20 +305,11 @@ static inline bool percpu_ref_tryget(struct percpu_ref *ref) > */ > static inline bool percpu_ref_tryget_live(struct percpu_ref *ref) > { > - unsigned long __percpu *percpu_count; > bool ret = false; > > rcu_read_lock(); > - > - if (__ref_is_percpu(ref, &percpu_count)) { > - this_cpu_inc(*percpu_count); > - ret = true; > - } else if (!(ref->percpu_count_ptr & __PERCPU_REF_DEAD)) { > - ret = atomic_long_inc_not_zero(&ref->data->count); > - } > - > + ret = percpu_ref_tryget_live_rcu(ref); > rcu_read_unlock(); > - > return ret; > } > > -- > 2.33.1 > Currently I'm not carrying anything and I don't expect any percpu_ref work to come in. Jens, feel free to pick this up. Acked-by: Dennis Zhou <dennis@kernel.org> Thanks, Dennis
On 10/21/21 15:01, Dennis Zhou wrote: > Hello, > > On Thu, Oct 21, 2021 at 02:30:51PM +0100, Pavel Begunkov wrote: >> Add percpu_ref_tryget_live_rcu(), which is a version of >> percpu_ref_tryget_live() but the user is responsible for enclosing it in >> a RCU read lock section. >> >> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> >> --- >> include/linux/percpu-refcount.h | 33 +++++++++++++++++++++++---------- >> 1 file changed, 23 insertions(+), 10 deletions(-) >> [...] >> + >> /** >> * percpu_ref_tryget_live - try to increment a live percpu refcount >> * @ref: percpu_ref to try-get > > Nit: it's dumb convention at this point, but do you mind copying this > guy up. I like consistency. Looks Jens already took it. If you still want it moved, do you mind it in a separate patch? And I'm not sure I follow where you want it to be, currently it's right before percpu_ref_tryget_live, which uses it.
On Fri, Oct 22, 2021 at 10:22:30AM +0100, Pavel Begunkov wrote: > On 10/21/21 15:01, Dennis Zhou wrote: > > Hello, > > > > On Thu, Oct 21, 2021 at 02:30:51PM +0100, Pavel Begunkov wrote: > > > Add percpu_ref_tryget_live_rcu(), which is a version of > > > percpu_ref_tryget_live() but the user is responsible for enclosing it in > > > a RCU read lock section. > > > > > > Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> > > > --- > > > include/linux/percpu-refcount.h | 33 +++++++++++++++++++++++---------- > > > 1 file changed, 23 insertions(+), 10 deletions(-) > > > > [...] > > > + > > > /** > > > * percpu_ref_tryget_live - try to increment a live percpu refcount > > > * @ref: percpu_ref to try-get > > > > Nit: it's dumb convention at this point, but do you mind copying this > > guy up. I like consistency. > > Looks Jens already took it. If you still want it moved, do you mind > it in a separate patch? > > And I'm not sure I follow where you want it to be, currently it's > right before percpu_ref_tryget_live, which uses it. Don't worry about it. I meant the @ref comment line. Honestly it's not really useful, it's just every other header block has that convention among most of percpu related files. If I have to make any changes in the future, I'll clean it up then. Thanks, Dennis > > -- > Pavel Begunkov
diff --git a/include/linux/percpu-refcount.h b/include/linux/percpu-refcount.h index ae16a9856305..b31d3f3312ce 100644 --- a/include/linux/percpu-refcount.h +++ b/include/linux/percpu-refcount.h @@ -266,6 +266,28 @@ static inline bool percpu_ref_tryget(struct percpu_ref *ref) return percpu_ref_tryget_many(ref, 1); } +/** + * percpu_ref_tryget_live_rcu - same as percpu_ref_tryget_live() but the + * caller is responsible for taking RCU. + * + * This function is safe to call as long as @ref is between init and exit. + */ +static inline bool percpu_ref_tryget_live_rcu(struct percpu_ref *ref) +{ + unsigned long __percpu *percpu_count; + bool ret = false; + + WARN_ON_ONCE(!rcu_read_lock_held()); + + if (likely(__ref_is_percpu(ref, &percpu_count))) { + this_cpu_inc(*percpu_count); + ret = true; + } else if (!(ref->percpu_count_ptr & __PERCPU_REF_DEAD)) { + ret = atomic_long_inc_not_zero(&ref->data->count); + } + return ret; +} + /** * percpu_ref_tryget_live - try to increment a live percpu refcount * @ref: percpu_ref to try-get @@ -283,20 +305,11 @@ static inline bool percpu_ref_tryget(struct percpu_ref *ref) */ static inline bool percpu_ref_tryget_live(struct percpu_ref *ref) { - unsigned long __percpu *percpu_count; bool ret = false; rcu_read_lock(); - - if (__ref_is_percpu(ref, &percpu_count)) { - this_cpu_inc(*percpu_count); - ret = true; - } else if (!(ref->percpu_count_ptr & __PERCPU_REF_DEAD)) { - ret = atomic_long_inc_not_zero(&ref->data->count); - } - + ret = percpu_ref_tryget_live_rcu(ref); rcu_read_unlock(); - return ret; }
Add percpu_ref_tryget_live_rcu(), which is a version of percpu_ref_tryget_live() but the user is responsible for enclosing it in a RCU read lock section. Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> --- include/linux/percpu-refcount.h | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-)