Re: [PATCH 1/1] block: rework flush sequencing for blk-mq

[Hannes Reinecke <hare@suse.de>]

On 03/08/2014 07:13 PM, Mike Snitzer wrote:
> On Sat, Mar 8, 2014 at 2:51 PM, Hannes Reinecke <hare@suse.de> wrote:
>> On 03/08/2014 06:33 PM, Mike Snitzer wrote:
>>>
>>> On Sat, Mar 8, 2014 at 10:52 AM, Christoph Hellwig <hch@infradead.org>
>>> wrote:
>>>>
>>>> On Fri, Mar 07, 2014 at 03:45:09PM -0500, Jeff Moyer wrote:
>>>>>
>>>>> Hi, Christoph,
>>>>>
>>>>> Did you mean to switch from list_add to list_add_tail?  That seems like
>>>>> a change that warrants mention.
>>>>
>>>>
>>>> No, that wasn't intentional and should be fixed.  Btw, there was another
>>>> issue with that commit, in that dm-multipath also needs to allocate
>>>> ->flush_rq.  I saw a patch from Hannes fixing it in the SuSE tree, and
>>>> would really love to see him submit that for mainline as well.
>>>
>>>
>>> Ugh, rq-based DM calls blk_init_allocated_queue.. (ah, looks like best
>>> to move q->flush_rq allocation from blk_init_queue_node to
>>> blk_alloc_queue_node?).
>
> The above suggestion would work.  But we'd lose the side-effect
> benefit to bio-based DM not needing q->flush_rq allocated at all.
>
> But I'm not immediately seeing a clean way to get that benefit (of not
> allocating for bio-based request_queue) while always allocating it for
> request-based queues.
>
> Actually, how about moving the flush_rq allocation to
> blk_init_allocated_queue(), it'd accomplish both.. what do others
> think of that?
>
>>>> Unfortunately SuSE seems to have lots of block and dm fixes and even
>>>> features that they don't submit upstream.
>>>
>>>
>>> Yeah, it is certainly disturbing.  No excuse for sitting on fixes like
>>> this.
>>>
>>> Hannes, _please_ get this dm-mpath flush_rq fix for 3.14 posted ASAP.
>>> Jens or I will need to get it to Linus next week.
>>>
>> Hey, calm down.
>
> I'm calm.. was just a bit frustrated.  But this isn't a big deal.
> I'll make an effort to reach out to relevant people sooner when
> similar stuff is reported against recently upstreamed code.  Would be
> cool if you did the same.  I can relate to needing to have the distro
> vendor hat on (first needing to determine/answer "is this issue
> specific to our hacked distro kernel?", etc).
>
The patch I made wasn't in the context of 'recently upstreamed code', it 
was due to a backport Jan Kara did for our next distro kernels (3.12-based).
I then made a fix just by code inspection, which _looked_ as if should 
work, and the asked the reporter to test.
I figured that the same issue would be upstream, too, that's right.
But as of now I'm still waiting for feedback on that patch.

I was about to test the patch next week and check for upstream 
application. But surely _NOT_ before I've got any proof that the patch 
fixes the issue.

Due diligence and all that.

I've just applied the code to our kernel tree because it
a) looks as if it would fix the issue
and
b) we're still in beta testing, so having the patch in-kernel makes 
thing easier for testing.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)