[dm-devel] [PATCH v2] dm thin: Fix bug wrt FUA request completion

[Nikos Tsironis]

On 2/15/19 5:16 PM, Mike Snitzer wrote:
> On Fri, Feb 15 2019 at  9:33am -0500,
> Nikos Tsironis <ntsironis at arrikto.com> wrote:
> 
>> On 2/15/19 3:54 PM, Joe Thornber wrote:
>>> Ack.
>>>
>>> Thanks for this I was under the mistaken impression that FUA requests got split 
>>> by core dm into separate payload and PREFLUSH requests.
>>>
>>> I've audited dm-cache and that looks ok.
>>>
>>> How did you test this patch?  That missing bio_list_init() in V1 must
>>> have caused memory corruption?
>>>
>>> - Joe
>>
>> Hi Joe,
>>
>> bio_list_init() initializes the bio list's head and tail pointers to
>> NULL and pool_create() allocates the struct pool structure using
>> kzalloc() so the bio list was implicitly correctly initialized and no
>> memory corruption occurred.
> 
> Yes, exactly right.  v1 tested fine for me, so when I saw v2 I reasoned
> through why the bio_list_init() wasn't an issue and it is like you've
> said (kzalloc() saved us).
> 
> Can you help us understand how you identified this issue?  Did you have
> corruption after crash/powerfail and got to looking closer?
> 
> Thanks,
> Mike

Hi Mike,

I identified the issue through code inspection. My job involves working
with device mapper extensively, so I have been studying its code for
quite some time now. I have already submitted a couple of fixes/performance
improvements to dm-snapshot. I will be following them up with a more
extensive patch set.

In this particular case I was trying to understand how to properly
handle REQ_FUA and REQ_PREFLUSH in the context of periodic metadata
commits, like dm-thin and dm-cache do, and I stumbled on this bug while
reading dm-thin's code.

Nikos