RE: [PATCH 04/12] bcache: Don't reinvent the wheel but use existing llist API

박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@lge.com)
2017-09-26T07:15:10Z

> -----Original Message-----
> From: Coly Li [mailto:i@coly.li]
> Sent: Tuesday, September 26, 2017 4:09 PM
> To: 박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@lge.com);
> Michael Lyle; Coly Li
> Cc: linux-bcache@vger.kernel.org; linux-block@vger.kernel.org;
> axboe@kernel.dk; Eric Wheeler; Kent Overstreet; kernel-team@lge.com
> Subject: Re: [PATCH 04/12] bcache: Don't reinvent the wheel but use existing
> llist API
>
> On 2017/9/26 下午2:39, 박병철/선임연구원/SW
> Platform(연)AOT팀(byungchul.park@lge.com) wrote:
> >> -----Original Message-----
> >> From: Michael Lyle [mailto:mlyle@lyle.org]
> >> Sent: Tuesday, September 26, 2017 1:38 PM
> >> To: Coly Li
> >> Cc: linux-bcache@vger.kernel.org; linux-block@vger.kernel.org;
> >> axboe@kernel.dk; Eric Wheeler; Byungchul Park; Kent Overstreet
> >> Subject: Re: [PATCH 04/12] bcache: Don't reinvent the wheel but use existing
> >> llist API
> >>
> >> I believe this introduces a critical bug.
> >>
> >> cl->list is used to link together the llists for both things waiting,
> >> and for things that are being woken.
> >>
> >> If a closure that is woken decides to wait again, it will corrupt the
> >> llist that __closure_wake_up is using.
> >>
> >> The previous iteration structure gets the next element of the list
> >> before waking and is therefore safe.
> >
> > Do you mean we have to use llist_for_each_entry_safe() instead of non-safe
> version?
> > Is it ok if we use it instead?
>
> Yes, we should use llist_for_each_entry_safe(), there is a quite
> implicit race here.

Hi coly,

As you know, my first patch used the safe version, but you suggested to replace
It with non-safe one. :(

I will change it so it does the same as the original patch did. :)

Thank you very much,
Byungchul

> --
> Coly Li