| Message ID | 20171003140406.26060-6-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
Hello, On Tue, Oct 03, 2017 at 10:04:03PM +0800, Ming Lei wrote: > Block layer need to call this function after holding > rcu lock in a real hot path, so introduce this helper. The patch description is too anemic. It doesn't even describe what changes are being made, let alone justifying them. Thanks.
On Tue, 2017-10-03 at 22:04 +0800, Ming Lei wrote: > Block layer need to call this function after holding > rcu lock in a real hot path, so introduce this helper. Since it is allowed to nest rcu_read_lock_sched() calls I don't think that this patch is necessary. Bart.
On Tue, Oct 03, 2017 at 07:14:59AM -0700, Tejun Heo wrote: > Hello, > > On Tue, Oct 03, 2017 at 10:04:03PM +0800, Ming Lei wrote: > > Block layer need to call this function after holding > > rcu lock in a real hot path, so introduce this helper. > > The patch description is too anemic. It doesn't even describe what > changes are being made, let alone justifying them. Sorry for Cc you, and this change is needed by the following patch: https://marc.info/?l=linux-scsi&m=150703956929333&w=2 Block layer need to call percpu_ref_tryget_live() with holding RCU read lock, given it is a hot I/O path, that is why I make this patch so that we can avoid nested RCU read lock and save a bit cost. -- Ming
On Tue, Oct 03, 2017 at 06:40:24PM +0000, Bart Van Assche wrote: > On Tue, 2017-10-03 at 22:04 +0800, Ming Lei wrote: > > Block layer need to call this function after holding > > rcu lock in a real hot path, so introduce this helper. > > Since it is allowed to nest rcu_read_lock_sched() calls I don't think > that this patch is necessary. Yeah, I know that, with this patch, we can avoid the nest RCU lock. As I mentioned, it is a real hot path, so maybe better to not introduce the extra RCU read lock/unlock. If you guys doesn't care the little cost, I can remove that.
Hello, On Wed, Oct 04, 2017 at 03:20:40AM +0800, Ming Lei wrote: > On Tue, Oct 03, 2017 at 07:14:59AM -0700, Tejun Heo wrote: > > Hello, > > > > On Tue, Oct 03, 2017 at 10:04:03PM +0800, Ming Lei wrote: > > > Block layer need to call this function after holding > > > rcu lock in a real hot path, so introduce this helper. > > > > The patch description is too anemic. It doesn't even describe what > > changes are being made, let alone justifying them. > > Sorry for Cc you, and this change is needed by the following > patch: > > https://marc.info/?l=linux-scsi&m=150703956929333&w=2 > > Block layer need to call percpu_ref_tryget_live() > with holding RCU read lock, given it is a hot I/O > path, that is why I make this patch so that we can > avoid nested RCU read lock and save a bit cost. I'm not necessarily against it but please name it sth like percpu_ref_tryget_live_locked(), add lockdep assertions in the inner function and document properly. Thanks.
diff --git a/include/linux/percpu-refcount.h b/include/linux/percpu-refcount.h index c13dceb87b60..a0f22586a28d 100644 --- a/include/linux/percpu-refcount.h +++ b/include/linux/percpu-refcount.h @@ -221,6 +221,21 @@ static inline bool percpu_ref_tryget(struct percpu_ref *ref) return ret; } +static inline bool __percpu_ref_tryget_live(struct percpu_ref *ref) +{ + unsigned long __percpu *percpu_count; + bool ret = false; + + 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->count); + } + + return ret; +} + /** * percpu_ref_tryget_live - try to increment a live percpu refcount * @ref: percpu_ref to try-get @@ -238,18 +253,10 @@ 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; + bool ret; rcu_read_lock_sched(); - - 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->count); - } - + ret = __percpu_ref_tryget_live(ref); rcu_read_unlock_sched(); return ret;
Block layer need to call this function after holding rcu lock in a real hot path, so introduce this helper. Cc: Bart Van Assche <Bart.VanAssche@wdc.com> Cc: Tejun Heo <tj@kernel.org> Signed-off-by: Ming Lei <ming.lei@redhat.com> --- include/linux/percpu-refcount.h | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-)