All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boaz Harrosh <bharrosh@panasas.com>
To: Peng Tao <bergwolf@gmail.com>
Cc: Christoph Hellwig <hch@infradead.org>,
	Trond Myklebust <Trond.Myklebust@netapp.com>,
	<linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 1/2] NFS: call block plug around direct write
Date: Wed, 16 May 2012 10:43:45 +0300	[thread overview]
Message-ID: <4FB35AB1.7030306@panasas.com> (raw)
In-Reply-To: <CA+a=Yy73hqYxgbBhroJmGpvJOkNRPoTiX6uLeSjY1-PXT=t7jQ@mail.gmail.com>

On 05/16/2012 03:35 AM, Peng Tao wrote:

> On Wed, May 16, 2012 at 12:23 AM, Boaz Harrosh <bharrosh@panasas.com> wrote:
>> On 05/15/2012 07:08 PM, Christoph Hellwig wrote:
>>
>>> On Tue, May 15, 2012 at 11:38:22PM +0800, Peng Tao wrote:
>>>> We bypass generic_file_aio_write() but would want to call block plug.
>>>
>>> We in this case is the pnfs block driver at most.  Thus these should
>>> be pnfs block code.
> Agreed. Just that for buffer IO case all call into block plug, so I
> thought it might be OK in DIO case as well... Anyway. I will make it
> block specific and call it LD()->dio_begin and LD()->dio_end. I am
> cooking some dio alignment patches for block layout driver and would
> love to put them there as well.
> 
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>> I thought so too.
>>
>> But reading the code more closely it might be hard for the blocklayout
>> to figure out the right state to call these two. Specially the call to
>> blk_finish_plug(). So you might need to add a new LD API such as
>> LD()->finish_plug() which is empty for others.
>>
>> But again inspecting the code it looks like blk_start_plug() is a no-op
>> and blk_finish_plug() is specially optimized for the empty case.
>>
> They are empty only when !CONFIG_BLOCK. I will make it block layout
> specific as said above.
> 


You miss understood. At the very top of blk_finish_plug() there
is an if (list_empty()) return. So if there was no activity between
plug/unplug the function is very fast.

But your above plan for alignment+plug sounds very good.

Thanks
Boaz

> Thanks,
> Tao
>> So is it worth it, the extra effort? I do understand the temptation
>> to get lazy here.
>>
>> Just my $0.017
>> Boaz



  reply	other threads:[~2012-05-16  7:44 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-15 15:38 [PATCH 1/2] NFS: call block plug around direct write Peng Tao
2012-05-15 15:38 ` [PATCH 2/2] NFS: call block plug around direct read Peng Tao
2012-05-16 17:12   ` Myklebust, Trond
2012-05-15 16:08 ` [PATCH 1/2] NFS: call block plug around direct write Christoph Hellwig
2012-05-15 16:23   ` Boaz Harrosh
2012-05-16  0:35     ` Peng Tao
2012-05-16  7:43       ` Boaz Harrosh [this message]
2012-05-16 17:12 ` Myklebust, Trond

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FB35AB1.7030306@panasas.com \
    --to=bharrosh@panasas.com \
    --cc=Trond.Myklebust@netapp.com \
    --cc=bergwolf@gmail.com \
    --cc=hch@infradead.org \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.