* [RESEND PATCH] percpu_ref: Fix comment regarding percpu_ref_init flags @ 2020-02-21 23:16 ira.weiny 2020-02-21 23:56 ` Dennis Zhou 0 siblings, 1 reply; 5+ messages in thread From: ira.weiny @ 2020-02-21 23:16 UTC (permalink / raw) To: linux-kernel Cc: Petr Mladek, Tejun Heo, Dennis Zhou, Thomas Gleixner, Greg Kroah-Hartman, Roman Gushchin, Sakari Ailus, Ira Weiny From: Ira Weiny <ira.weiny@intel.com> The comment for percpu_ref_init() implies that using PERCPU_REF_ALLOW_REINIT will cause the refcount to start at 0. But this is not true. PERCPU_REF_ALLOW_REINIT starts the count at 1 as if the flags were zero. Add this fact to the kernel doc comment. Signed-off-by: Ira Weiny <ira.weiny@intel.com> --- RESEND: Add more people on the CC list to see if I'm wrong here. https://lore.kernel.org/lkml/20200206042810.GA29917@iweiny-DESK2.sc.intel.com/ --- lib/percpu-refcount.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/percpu-refcount.c b/lib/percpu-refcount.c index 4f6c6ebbbbde..48d7fcff70b6 100644 --- a/lib/percpu-refcount.c +++ b/lib/percpu-refcount.c @@ -50,9 +50,9 @@ static unsigned long __percpu *percpu_count_ptr(struct percpu_ref *ref) * @flags: PERCPU_REF_INIT_* flags * @gfp: allocation mask to use * - * Initializes @ref. If @flags is zero, @ref starts in percpu mode with a - * refcount of 1; analagous to atomic_long_set(ref, 1). See the - * definitions of PERCPU_REF_INIT_* flags for flag behaviors. + * Initializes @ref. If @flags is zero or PERCPU_REF_ALLOW_REINIT, @ref starts + * in percpu mode with a refcount of 1; analagous to atomic_long_set(ref, 1). + * See the definitions of PERCPU_REF_INIT_* flags for flag behaviors. * * Note that @release must not sleep - it may potentially be called from RCU * callback context by percpu_ref_kill(). -- 2.21.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RESEND PATCH] percpu_ref: Fix comment regarding percpu_ref_init flags 2020-02-21 23:16 [RESEND PATCH] percpu_ref: Fix comment regarding percpu_ref_init flags ira.weiny @ 2020-02-21 23:56 ` Dennis Zhou 2020-02-22 0:46 ` Roman Gushchin 0 siblings, 1 reply; 5+ messages in thread From: Dennis Zhou @ 2020-02-21 23:56 UTC (permalink / raw) To: ira.weiny Cc: linux-kernel, Petr Mladek, Tejun Heo, Dennis Zhou, Thomas Gleixner, Greg Kroah-Hartman, Roman Gushchin, Sakari Ailus Hi Ira, On Fri, Feb 21, 2020 at 03:16:07PM -0800, ira.weiny@intel.com wrote: > From: Ira Weiny <ira.weiny@intel.com> > > The comment for percpu_ref_init() implies that using > PERCPU_REF_ALLOW_REINIT will cause the refcount to start at 0. But > this is not true. PERCPU_REF_ALLOW_REINIT starts the count at 1 as > if the flags were zero. Add this fact to the kernel doc comment. > > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > --- > RESEND: > Add more people on the CC list to see if I'm wrong here. > https://lore.kernel.org/lkml/20200206042810.GA29917@iweiny-DESK2.sc.intel.com/ > --- > > lib/percpu-refcount.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/lib/percpu-refcount.c b/lib/percpu-refcount.c > index 4f6c6ebbbbde..48d7fcff70b6 100644 > --- a/lib/percpu-refcount.c > +++ b/lib/percpu-refcount.c > @@ -50,9 +50,9 @@ static unsigned long __percpu *percpu_count_ptr(struct percpu_ref *ref) > * @flags: PERCPU_REF_INIT_* flags > * @gfp: allocation mask to use > * > - * Initializes @ref. If @flags is zero, @ref starts in percpu mode with a > - * refcount of 1; analagous to atomic_long_set(ref, 1). See the > - * definitions of PERCPU_REF_INIT_* flags for flag behaviors. > + * Initializes @ref. If @flags is zero or PERCPU_REF_ALLOW_REINIT, @ref starts > + * in percpu mode with a refcount of 1; analagous to atomic_long_set(ref, 1). > + * See the definitions of PERCPU_REF_INIT_* flags for flag behaviors. Yeah. Prior we had both PERCPU_REF_INIT_ATOMIC and PERCPU_REF_INIT_DEAD with the latter implying the former. So 0 meant percpu and the others meant atomic. With PERCPU_REF_ALLOW_REINIT, it's probably easier to understand by saying if neither PERCPU_REF_INIT_ATOMIC or PERCPU_REF_INIT_DEAD is set, it starts out in percpu mode which is mentioned in the comments where the flags are defined. It's not great having implied flags, but it's worked so far. Also, it's not quite analagous to atomic_long_set(ref, 1) as there is a bias to prevent prematurely hitting 0. I can take this and massage the wording a bit. > * > * Note that @release must not sleep - it may potentially be called from RCU > * callback context by percpu_ref_kill(). > -- > 2.21.0 > Thanks, Dennis ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RESEND PATCH] percpu_ref: Fix comment regarding percpu_ref_init flags 2020-02-21 23:56 ` Dennis Zhou @ 2020-02-22 0:46 ` Roman Gushchin 2020-02-23 14:32 ` Ira Weiny 0 siblings, 1 reply; 5+ messages in thread From: Roman Gushchin @ 2020-02-22 0:46 UTC (permalink / raw) To: Dennis Zhou Cc: ira.weiny, linux-kernel, Petr Mladek, Tejun Heo, Thomas Gleixner, Greg Kroah-Hartman, Sakari Ailus On Fri, Feb 21, 2020 at 06:56:27PM -0500, Dennis Zhou wrote: > Hi Ira, > > On Fri, Feb 21, 2020 at 03:16:07PM -0800, ira.weiny@intel.com wrote: > > From: Ira Weiny <ira.weiny@intel.com> > > > > The comment for percpu_ref_init() implies that using > > PERCPU_REF_ALLOW_REINIT will cause the refcount to start at 0. But > > this is not true. PERCPU_REF_ALLOW_REINIT starts the count at 1 as > > if the flags were zero. Add this fact to the kernel doc comment. > > > > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > > --- > > RESEND: > > Add more people on the CC list to see if I'm wrong here. > > https://lore.kernel.org/lkml/20200206042810.GA29917@iweiny-DESK2.sc.intel.com/ > > --- > > > > lib/percpu-refcount.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/lib/percpu-refcount.c b/lib/percpu-refcount.c > > index 4f6c6ebbbbde..48d7fcff70b6 100644 > > --- a/lib/percpu-refcount.c > > +++ b/lib/percpu-refcount.c > > @@ -50,9 +50,9 @@ static unsigned long __percpu *percpu_count_ptr(struct percpu_ref *ref) > > * @flags: PERCPU_REF_INIT_* flags > > * @gfp: allocation mask to use > > * > > - * Initializes @ref. If @flags is zero, @ref starts in percpu mode with a > > - * refcount of 1; analagous to atomic_long_set(ref, 1). See the > > - * definitions of PERCPU_REF_INIT_* flags for flag behaviors. > > + * Initializes @ref. If @flags is zero or PERCPU_REF_ALLOW_REINIT, @ref starts > > + * in percpu mode with a refcount of 1; analagous to atomic_long_set(ref, 1). > > + * See the definitions of PERCPU_REF_INIT_* flags for flag behaviors. > > Yeah. Prior we had both PERCPU_REF_INIT_ATOMIC and PERCPU_REF_INIT_DEAD > with the latter implying the former. So 0 meant percpu and the others > meant atomic. With PERCPU_REF_ALLOW_REINIT, it's probably easier to > understand by saying if neither PERCPU_REF_INIT_ATOMIC or > PERCPU_REF_INIT_DEAD is set, it starts out in percpu mode which is > mentioned in the comments where the flags are defined. It's not great > having implied flags, but it's worked so far. > > Also, it's not quite analagous to atomic_long_set(ref, 1) as there is a > bias to prevent prematurely hitting 0. > > I can take this and massage the wording a bit. Hello Ira! Hello Dennis! Yeah, I'd simple say that it starts in the percpu mode, except the case when PERCPU_REF_INIT_ATOMIC is set, then (atomic mode, 1) and PERCPU_REF_INIT_DEAD is set, then (atomic mode, 0). PERCPU_REF_ALLOW_REINIT actually doesn't affect the initial state. Thanks! ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RESEND PATCH] percpu_ref: Fix comment regarding percpu_ref_init flags 2020-02-22 0:46 ` Roman Gushchin @ 2020-02-23 14:32 ` Ira Weiny 2020-03-05 21:27 ` Dennis Zhou 0 siblings, 1 reply; 5+ messages in thread From: Ira Weiny @ 2020-02-23 14:32 UTC (permalink / raw) To: Roman Gushchin Cc: Dennis Zhou, linux-kernel, Petr Mladek, Tejun Heo, Thomas Gleixner, Greg Kroah-Hartman, Sakari Ailus [snip] > > > diff --git a/lib/percpu-refcount.c b/lib/percpu-refcount.c > > > index 4f6c6ebbbbde..48d7fcff70b6 100644 > > > --- a/lib/percpu-refcount.c > > > +++ b/lib/percpu-refcount.c > > > @@ -50,9 +50,9 @@ static unsigned long __percpu *percpu_count_ptr(struct percpu_ref *ref) > > > * @flags: PERCPU_REF_INIT_* flags > > > * @gfp: allocation mask to use > > > * > > > - * Initializes @ref. If @flags is zero, @ref starts in percpu mode with a > > > - * refcount of 1; analagous to atomic_long_set(ref, 1). See the > > > - * definitions of PERCPU_REF_INIT_* flags for flag behaviors. > > > + * Initializes @ref. If @flags is zero or PERCPU_REF_ALLOW_REINIT, @ref starts > > > + * in percpu mode with a refcount of 1; analagous to atomic_long_set(ref, 1). > > > + * See the definitions of PERCPU_REF_INIT_* flags for flag behaviors. > > > > Yeah. Prior we had both PERCPU_REF_INIT_ATOMIC and PERCPU_REF_INIT_DEAD > > with the latter implying the former. So 0 meant percpu and the others > > meant atomic. With PERCPU_REF_ALLOW_REINIT, it's probably easier to > > understand by saying if neither PERCPU_REF_INIT_ATOMIC or > > PERCPU_REF_INIT_DEAD is set, it starts out in percpu mode which is > > mentioned in the comments where the flags are defined. It's not great > > having implied flags, but it's worked so far. > > > > Also, it's not quite analagous to atomic_long_set(ref, 1) as there is a > > bias to prevent prematurely hitting 0. > > > > I can take this and massage the wording a bit. > > Hello Ira! Hello Dennis! > > Yeah, I'd simple say that it starts in the percpu mode, except the case when > PERCPU_REF_INIT_ATOMIC is set, then (atomic mode, 1) and > PERCPU_REF_INIT_DEAD is set, then (atomic mode, 0). > > PERCPU_REF_ALLOW_REINIT actually doesn't affect the initial state. > Thanks for the clarification. Dennis let me know if you want me to resubmit the patch. Thanks! Ira ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RESEND PATCH] percpu_ref: Fix comment regarding percpu_ref_init flags 2020-02-23 14:32 ` Ira Weiny @ 2020-03-05 21:27 ` Dennis Zhou 0 siblings, 0 replies; 5+ messages in thread From: Dennis Zhou @ 2020-03-05 21:27 UTC (permalink / raw) To: Ira Weiny Cc: Roman Gushchin, Dennis Zhou, linux-kernel, Petr Mladek, Tejun Heo, Thomas Gleixner, Greg Kroah-Hartman, Sakari Ailus On Sun, Feb 23, 2020 at 06:32:23AM -0800, Ira Weiny wrote: > > [snip] > > > > > diff --git a/lib/percpu-refcount.c b/lib/percpu-refcount.c > > > > index 4f6c6ebbbbde..48d7fcff70b6 100644 > > > > --- a/lib/percpu-refcount.c > > > > +++ b/lib/percpu-refcount.c > > > > @@ -50,9 +50,9 @@ static unsigned long __percpu *percpu_count_ptr(struct percpu_ref *ref) > > > > * @flags: PERCPU_REF_INIT_* flags > > > > * @gfp: allocation mask to use > > > > * > > > > - * Initializes @ref. If @flags is zero, @ref starts in percpu mode with a > > > > - * refcount of 1; analagous to atomic_long_set(ref, 1). See the > > > > - * definitions of PERCPU_REF_INIT_* flags for flag behaviors. > > > > + * Initializes @ref. If @flags is zero or PERCPU_REF_ALLOW_REINIT, @ref starts > > > > + * in percpu mode with a refcount of 1; analagous to atomic_long_set(ref, 1). > > > > + * See the definitions of PERCPU_REF_INIT_* flags for flag behaviors. > > > > > > Yeah. Prior we had both PERCPU_REF_INIT_ATOMIC and PERCPU_REF_INIT_DEAD > > > with the latter implying the former. So 0 meant percpu and the others > > > meant atomic. With PERCPU_REF_ALLOW_REINIT, it's probably easier to > > > understand by saying if neither PERCPU_REF_INIT_ATOMIC or > > > PERCPU_REF_INIT_DEAD is set, it starts out in percpu mode which is > > > mentioned in the comments where the flags are defined. It's not great > > > having implied flags, but it's worked so far. > > > > > > Also, it's not quite analagous to atomic_long_set(ref, 1) as there is a > > > bias to prevent prematurely hitting 0. > > > > > > I can take this and massage the wording a bit. > > > > Hello Ira! Hello Dennis! > > > > Yeah, I'd simple say that it starts in the percpu mode, except the case when > > PERCPU_REF_INIT_ATOMIC is set, then (atomic mode, 1) and > > PERCPU_REF_INIT_DEAD is set, then (atomic mode, 0). > > > > PERCPU_REF_ALLOW_REINIT actually doesn't affect the initial state. > > > > Thanks for the clarification. Dennis let me know if you want me to resubmit > the patch. > > Thanks! > Ira > Sorry for the delay. I've applied it to for-5.7. Thanks, Dennis ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-03-05 21:27 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-21 23:16 [RESEND PATCH] percpu_ref: Fix comment regarding percpu_ref_init flags ira.weiny 2020-02-21 23:56 ` Dennis Zhou 2020-02-22 0:46 ` Roman Gushchin 2020-02-23 14:32 ` Ira Weiny 2020-03-05 21:27 ` Dennis Zhou
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).