Messages in this thread | ![/](/images/icornerl.gif) | | Date | Fri, 8 Apr 2022 12:14:54 +0800 | Subject | Re: [PATCH] percpu_ref: call wake_up_all() after percpu_ref_put() completes | From | Qi Zheng <> |
| |
On 2022/4/8 12:10 PM, Andrew Morton wrote: > On Fri, 8 Apr 2022 12:06:20 +0800 Qi Zheng <zhengqi.arch@bytedance.com> wrote: > >> >> >>>>>> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com> >>>>> >>>>> Are any users affected by this? If so, I think a Fixes tag >>>>> is necessary. >>>> >>>> Looks all current users(blk_pre_runtime_suspend() and set_in_sync()) are >>>> affected by this. >>>> >>>> I see that this patch has been merged into the mm tree, can Andrew help >>>> me add the following Fixes tag? >>> >>> Andrew is helpful ;) >>> >>> Do you see reasons why we should backport this into -stable trees? >>> It's 8 years old, so my uninformed guess is "no"? >> >> Hmm, although the commit 490c79a65708 add wake_up_all(), it is no >> problem for the usage at that time, maybe the correct Fixes tag is the >> following: >> >> Fixes: 210f7cdcf088 ("percpu-refcount: support synchronous switch to >> atomic mode.") >> >> But in fact, there is no problem with it, but all current users expect >> the refcount is stable after percpu_ref_switch_to_atomic_sync() returns. >> >> I have no idea as which Fixes tag to add. > > Well the solution to that problem is to add cc:stable and let Greg > figure it out ;) > > The more serious question is "should we backport this". What is the > end-user-visible impact of the bug? Do our users need the fix or not?
The impact on the current user is that it is possible to miss an opportunity to reach 0 due to the case B in the commit message:
/* The value of &ref is unstable! */ percpu_ref_is_zero(&ref) (B)percpu_ref_put(ref);
Thanks, Qi
>
-- Thanks, Qi
| ![\](/images/icornerr.gif) |