lkml.org 
[lkml]   [2014]   [Sep]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 5/8] block: introduce blk_flush_queue to drive flush machinery
On 09/09/2014 12:43 PM, Christoph Hellwig wrote:
> On Tue, Sep 09, 2014 at 09:05:46PM +0800, Ming Lei wrote:
>> This patch introduces 'struct blk_flush_queue' and puts all
>> flush machinery related stuff into this strcuture, so that
>
> s/stuff/fields/
> s/strcuture/structure/
>
> Looks good, but a few more nitpicks below.
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
>> +int blk_init_flush(struct request_queue *q)
>> +{
>> + int ret;
>> + struct blk_flush_queue *fq = kzalloc(sizeof(*fq), GFP_KERNEL);
>>
>> + if (!fq)
>> return -ENOMEM;
>>
>> + q->fq = fq;
>
> I think it would be cleaner to return the flush data structure and
> assign it in the caller.

I was going to suggest renaming because of this as well. If we do this:

q->fq = blk_init_flush(q);

then it's immediately clear what it does, whereas blk_init_flush(q)
means very little on its own. I'd change the naming to
blk_alloc_flush_queue() and blk_free_flush_queue().

--
Jens Axboe



\
 
 \ /
  Last update: 2014-09-09 21:21    [W:0.059 / U:0.444 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site