On 2/15/19 5:16 PM, Mike Snitzer wrote: > On Fri, Feb 15 2019 at 9:33am -0500, > Nikos Tsironis <ntsironis@xxxxxxxxxxx> 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 -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
