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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2017/9/26 下午3:15, 박병철/선임연구원/SW
Platform(연)AOT팀(byungchul.park@xxxxxxx) wrote:
>> -----Original Message-----
>> From: Coly Li [mailto:i@xxxxxxx]
>> Sent: Tuesday, September 26, 2017 4:09 PM
>> To: 박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@xxxxxxx);
>> Michael Lyle; Coly Li
>> Cc: linux-bcache@xxxxxxxxxxxxxxx; linux-block@xxxxxxxxxxxxxxx;
>> axboe@xxxxxxxxx; Eric Wheeler; Kent Overstreet; kernel-team@xxxxxxx
>> 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@xxxxxxx) wrote:
>>>> -----Original Message-----
>>>> From: Michael Lyle [mailto:mlyle@xxxxxxxx]
>>>> Sent: Tuesday, September 26, 2017 1:38 PM
>>>> To: Coly Li
>>>> Cc: linux-bcache@xxxxxxxxxxxxxxx; linux-block@xxxxxxxxxxxxxxx;
>>>> axboe@xxxxxxxxx; 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. :(

No doubt, it's my fault. I didn't find the implicit potential race.

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

Yes, please. Because the original patch is merged into Linux upstream,
post a fix to replace llist_for_each_entry() by
llist_for_each_entry_safe(). We still have chance to catch up 4.14.

Thank you !

-- 
Coly Li



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux