All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch]mm: add more blk plug
@ 2011-09-26  2:30 Shaohua Li
  2011-09-26  7:30 ` Christoph Hellwig
  0 siblings, 1 reply; 12+ messages in thread
From: Shaohua Li @ 2011-09-26  2:30 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Andrew Morton, Christoph Hellwig, lkml

Some filesystems implement .writepages. We don't have blk plug
in such filesystems for .writepages.

Signed-off-by: Shaohua Li <shaohua.li@intel.com>

diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 0e309cd..b9e73a1 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -1253,13 +1253,16 @@ EXPORT_SYMBOL(generic_writepages);
 int do_writepages(struct address_space *mapping, struct writeback_control *wbc)
 {
 	int ret;
+	struct blk_plug plug;
 
 	if (wbc->nr_to_write <= 0)
 		return 0;
+	blk_start_plug(&plug);
 	if (mapping->a_ops->writepages)
 		ret = mapping->a_ops->writepages(mapping, wbc);
 	else
 		ret = generic_writepages(mapping, wbc);
+	blk_finish_plug(&plug);
 	return ret;
 }
 



^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [patch]mm: add more blk plug
  2011-09-26  2:30 [patch]mm: add more blk plug Shaohua Li
@ 2011-09-26  7:30 ` Christoph Hellwig
  2011-09-26  7:38   ` Shaohua Li
  0 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2011-09-26  7:30 UTC (permalink / raw)
  To: Shaohua Li; +Cc: Jens Axboe, Andrew Morton, Christoph Hellwig, lkml

On Mon, Sep 26, 2011 at 10:30:26AM +0800, Shaohua Li wrote:
> Some filesystems implement .writepages. We don't have blk plug
> in such filesystems for .writepages.

Please add the plugging in the actual ->writepages instances.


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [patch]mm: add more blk plug
  2011-09-26  7:30 ` Christoph Hellwig
@ 2011-09-26  7:38   ` Shaohua Li
  2011-09-26  9:45     ` Christoph Hellwig
  0 siblings, 1 reply; 12+ messages in thread
From: Shaohua Li @ 2011-09-26  7:38 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, Andrew Morton, lkml

On Mon, 2011-09-26 at 15:30 +0800, Christoph Hellwig wrote:
> On Mon, Sep 26, 2011 at 10:30:26AM +0800, Shaohua Li wrote:
> > Some filesystems implement .writepages. We don't have blk plug
> > in such filesystems for .writepages.
> 
> Please add the plugging in the actual ->writepages instances.
there are several filesystems have ->writepages. Can you share an hint
why we don't add plugging in the do_writepages?

Thanks,
Shaohua


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [patch]mm: add more blk plug
  2011-09-26  7:38   ` Shaohua Li
@ 2011-09-26  9:45     ` Christoph Hellwig
  2011-09-28  3:09       ` [patch]ext4: add block plug for .writepages Shaohua Li
  0 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2011-09-26  9:45 UTC (permalink / raw)
  To: Shaohua Li; +Cc: Christoph Hellwig, Jens Axboe, Andrew Morton, lkml

On Mon, Sep 26, 2011 at 03:38:47PM +0800, Shaohua Li wrote:
> On Mon, 2011-09-26 at 15:30 +0800, Christoph Hellwig wrote:
> > On Mon, Sep 26, 2011 at 10:30:26AM +0800, Shaohua Li wrote:
> > > Some filesystems implement .writepages. We don't have blk plug
> > > in such filesystems for .writepages.
> > 
> > Please add the plugging in the actual ->writepages instances.
> there are several filesystems have ->writepages. Can you share an hint
> why we don't add plugging in the do_writepages?

Because do_writepages is completely generic code with no knowledge of
block I/O.  I really don't want to have block plugging be intimately
tied into core VM/writeback code all over.  We've been there with
->sync_page, and it was a major pain - nevermind the additional (small)
overhead for the not block based filesystems.


^ permalink raw reply	[flat|nested] 12+ messages in thread
* [patch]ext4: add block plug for .writepages
  2011-09-26  9:45     ` Christoph Hellwig
@ 2011-09-28  3:09       ` Shaohua Li
  2011-10-09  6:09         ` Shaohua Li
  2011-10-10 16:51         ` Ted Ts'o
  0 siblings, 2 replies; 12+ messages in thread
From: Shaohua Li @ 2011-09-28  3:09 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe, Ted Ts'o
  Cc: Andrew Morton, lkml, linux-ext4

On Mon, 2011-09-26 at 17:45 +0800, Christoph Hellwig wrote:
> On Mon, Sep 26, 2011 at 03:38:47PM +0800, Shaohua Li wrote:
> > On Mon, 2011-09-26 at 15:30 +0800, Christoph Hellwig wrote:
> > > On Mon, Sep 26, 2011 at 10:30:26AM +0800, Shaohua Li wrote:
> > > > Some filesystems implement .writepages. We don't have blk plug
> > > > in such filesystems for .writepages.
> > > 
> > > Please add the plugging in the actual ->writepages instances.
> > there are several filesystems have ->writepages. Can you share an hint
> > why we don't add plugging in the do_writepages?
> 
> Because do_writepages is completely generic code with no knowledge of
> block I/O.  I really don't want to have block plugging be intimately
> tied into core VM/writeback code all over.  We've been there with
> ->sync_page, and it was a major pain - nevermind the additional (small)
> overhead for the not block based filesystems.
I searched a little bit, looks only ext4 need it. here is the patch.


Add block plug for ext4 .writepages. Though ext4 .writepages
already handles request merge very well, block plug is still
helpful to reduce block lock contention.

Signed-off-by: Shaohua Li <shaohua.li@intel.com>

---
 fs/ext4/inode.c |    3 +++
 1 file changed, 3 insertions(+)

Index: linux/fs/ext4/inode.c
===================================================================
--- linux.orig/fs/ext4/inode.c	2011-09-28 10:13:50.000000000 +0800
+++ linux/fs/ext4/inode.c	2011-09-28 10:23:31.000000000 +0800
@@ -2046,6 +2046,7 @@ static int ext4_da_writepages(struct add
 	struct ext4_sb_info *sbi = EXT4_SB(mapping->host->i_sb);
 	pgoff_t done_index = 0;
 	pgoff_t end;
+	struct blk_plug plug;
 
 	trace_ext4_da_writepages(inode, wbc);
 
@@ -2124,6 +2125,7 @@ retry:
 	if (wbc->sync_mode == WB_SYNC_ALL || wbc->tagged_writepages)
 		tag_pages_for_writeback(mapping, index, end);
 
+	blk_start_plug(&plug);
 	while (!ret && wbc->nr_to_write > 0) {
 
 		/*
@@ -2188,6 +2190,7 @@ retry:
 			 */
 			break;
 	}
+	blk_finish_plug(&plug);
 	if (!io_done && !cycled) {
 		cycled = 1;
 		index = 0;



^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [patch]ext4: add block plug for .writepages
  2011-09-28  3:09       ` [patch]ext4: add block plug for .writepages Shaohua Li
@ 2011-10-09  6:09         ` Shaohua Li
  2011-10-10 16:51         ` Ted Ts'o
  1 sibling, 0 replies; 12+ messages in thread
From: Shaohua Li @ 2011-10-09  6:09 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe, Ted Ts'o
  Cc: Andrew Morton, lkml, linux-ext4

On Wed, 2011-09-28 at 11:09 +0800, Shaohua Li wrote:
> On Mon, 2011-09-26 at 17:45 +0800, Christoph Hellwig wrote:
> > On Mon, Sep 26, 2011 at 03:38:47PM +0800, Shaohua Li wrote:
> > > On Mon, 2011-09-26 at 15:30 +0800, Christoph Hellwig wrote:
> > > > On Mon, Sep 26, 2011 at 10:30:26AM +0800, Shaohua Li wrote:
> > > > > Some filesystems implement .writepages. We don't have blk plug
> > > > > in such filesystems for .writepages.
> > > > 
> > > > Please add the plugging in the actual ->writepages instances.
> > > there are several filesystems have ->writepages. Can you share an hint
> > > why we don't add plugging in the do_writepages?
> > 
> > Because do_writepages is completely generic code with no knowledge of
> > block I/O.  I really don't want to have block plugging be intimately
> > tied into core VM/writeback code all over.  We've been there with
> > ->sync_page, and it was a major pain - nevermind the additional (small)
> > overhead for the not block based filesystems.
> I searched a little bit, looks only ext4 need it. here is the patch.
> 
> 
> Add block plug for ext4 .writepages. Though ext4 .writepages
> already handles request merge very well, block plug is still
> helpful to reduce block lock contention.
> 
ping


> Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> 
> ---
>  fs/ext4/inode.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> Index: linux/fs/ext4/inode.c
> ===================================================================
> --- linux.orig/fs/ext4/inode.c	2011-09-28 10:13:50.000000000 +0800
> +++ linux/fs/ext4/inode.c	2011-09-28 10:23:31.000000000 +0800
> @@ -2046,6 +2046,7 @@ static int ext4_da_writepages(struct add
>  	struct ext4_sb_info *sbi = EXT4_SB(mapping->host->i_sb);
>  	pgoff_t done_index = 0;
>  	pgoff_t end;
> +	struct blk_plug plug;
>  
>  	trace_ext4_da_writepages(inode, wbc);
>  
> @@ -2124,6 +2125,7 @@ retry:
>  	if (wbc->sync_mode == WB_SYNC_ALL || wbc->tagged_writepages)
>  		tag_pages_for_writeback(mapping, index, end);
>  
> +	blk_start_plug(&plug);
>  	while (!ret && wbc->nr_to_write > 0) {
>  
>  		/*
> @@ -2188,6 +2190,7 @@ retry:
>  			 */
>  			break;
>  	}
> +	blk_finish_plug(&plug);
>  	if (!io_done && !cycled) {
>  		cycled = 1;
>  		index = 0;
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [patch]ext4: add block plug for .writepages
  2011-09-28  3:09       ` [patch]ext4: add block plug for .writepages Shaohua Li
  2011-10-09  6:09         ` Shaohua Li
@ 2011-10-10 16:51         ` Ted Ts'o
  2011-10-11  0:07             ` NamJae Jeon
  2011-10-11 10:40             ` Kyungmin Park
  1 sibling, 2 replies; 12+ messages in thread
From: Ted Ts'o @ 2011-10-10 16:51 UTC (permalink / raw)
  To: Shaohua Li; +Cc: Christoph Hellwig, Jens Axboe, Andrew Morton, lkml, linux-ext4

On Wed, Sep 28, 2011 at 11:09:40AM +0800, Shaohua Li wrote:
> I searched a little bit, looks only ext4 need it. here is the patch.
> 
> 
> Add block plug for ext4 .writepages. Though ext4 .writepages
> already handles request merge very well, block plug is still
> helpful to reduce block lock contention.
> 
> Signed-off-by: Shaohua Li <shaohua.li@intel.com>

Thanks, applied.

					- Ted

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [patch]ext4: add block plug for .writepages
  2011-10-10 16:51         ` Ted Ts'o
@ 2011-10-11  0:07             ` NamJae Jeon
  2011-10-11 10:40             ` Kyungmin Park
  1 sibling, 0 replies; 12+ messages in thread
From: NamJae Jeon @ 2011-10-11  0:07 UTC (permalink / raw)
  To: Ted Ts'o, Shaohua Li, Christoph Hellwig, Jens Axboe,
	Andrew Morton, lkml, linux-ext4

2011/10/11 Ted Ts'o <tytso@mit.edu>:
> On Wed, Sep 28, 2011 at 11:09:40AM +0800, Shaohua Li wrote:
>> I searched a little bit, looks only ext4 need it. here is the patch.
>>
>>
>> Add block plug for ext4 .writepages. Though ext4 .writepages
>> already handles request merge very well, block plug is still
>> helpful to reduce block lock contention.
>>
>> Signed-off-by: Shaohua Li <shaohua.li@intel.com>
You found good point.
If there are other filesystem that didn't use generic_writepages,
these are needed as this patch.

Reviewed-by: NamJae Jeon <linkinjeon@gmail.com>
>
> Thanks, applied.
>
>                                        - Ted
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [patch]ext4: add block plug for .writepages
@ 2011-10-11  0:07             ` NamJae Jeon
  0 siblings, 0 replies; 12+ messages in thread
From: NamJae Jeon @ 2011-10-11  0:07 UTC (permalink / raw)
  To: Ted Ts'o, Shaohua Li, Christoph Hellwig, Jens Axboe,
	Andrew Morton, lkml

2011/10/11 Ted Ts'o <tytso@mit.edu>:
> On Wed, Sep 28, 2011 at 11:09:40AM +0800, Shaohua Li wrote:
>> I searched a little bit, looks only ext4 need it. here is the patch.
>>
>>
>> Add block plug for ext4 .writepages. Though ext4 .writepages
>> already handles request merge very well, block plug is still
>> helpful to reduce block lock contention.
>>
>> Signed-off-by: Shaohua Li <shaohua.li@intel.com>
You found good point.
If there are other filesystem that didn't use generic_writepages,
these are needed as this patch.

Reviewed-by: NamJae Jeon <linkinjeon@gmail.com>
>
> Thanks, applied.
>
>                                        - Ted
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [patch]ext4: add block plug for .writepages
  2011-10-10 16:51         ` Ted Ts'o
@ 2011-10-11 10:40             ` Kyungmin Park
  2011-10-11 10:40             ` Kyungmin Park
  1 sibling, 0 replies; 12+ messages in thread
From: Kyungmin Park @ 2011-10-11 10:40 UTC (permalink / raw)
  To: Ted Ts'o, Shaohua Li, Christoph Hellwig, Jens Axboe,
	Andrew Morton, lkml, linux-ext4

On Tue, Oct 11, 2011 at 1:51 AM, Ted Ts'o <tytso@mit.edu> wrote:
> On Wed, Sep 28, 2011 at 11:09:40AM +0800, Shaohua Li wrote:
>> I searched a little bit, looks only ext4 need it. here is the patch.
>>
>>
>> Add block plug for ext4 .writepages. Though ext4 .writepages
>> already handles request merge very well, block plug is still
>> helpful to reduce block lock contention.
>>
>> Signed-off-by: Shaohua Li <shaohua.li@intel.com>

Does it require to add blk_finish_plug(&plug) when error case?

+        blk_start_plug(&plug);
         while (!ret && wbc->nr_to_write > 0) {

                ...
                /* start a new transaction*/
                handle = ext4_journal_start(inode, needed_blocks);
                if (IS_ERR(handle)) {
                        ret = PTR_ERR(handle);
                        ext4_msg(inode->i_sb, KERN_CRIT, "%s: jbd2_start: "
                               "%ld pages, ino %lu; err %d", __func__,
                                wbc->nr_to_write, inode->i_ino, ret);
+                      blk_finish_plug(&plug);
                        goto out_writepages;
                }

Thank you,
Kyungmin Park-

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [patch]ext4: add block plug for .writepages
@ 2011-10-11 10:40             ` Kyungmin Park
  0 siblings, 0 replies; 12+ messages in thread
From: Kyungmin Park @ 2011-10-11 10:40 UTC (permalink / raw)
  To: Ted Ts'o, Shaohua Li, Christoph Hellwig, Jens Axboe,
	Andrew Morton, lkml

On Tue, Oct 11, 2011 at 1:51 AM, Ted Ts'o <tytso@mit.edu> wrote:
> On Wed, Sep 28, 2011 at 11:09:40AM +0800, Shaohua Li wrote:
>> I searched a little bit, looks only ext4 need it. here is the patch.
>>
>>
>> Add block plug for ext4 .writepages. Though ext4 .writepages
>> already handles request merge very well, block plug is still
>> helpful to reduce block lock contention.
>>
>> Signed-off-by: Shaohua Li <shaohua.li@intel.com>

Does it require to add blk_finish_plug(&plug) when error case?

+        blk_start_plug(&plug);
         while (!ret && wbc->nr_to_write > 0) {

                ...
                /* start a new transaction*/
                handle = ext4_journal_start(inode, needed_blocks);
                if (IS_ERR(handle)) {
                        ret = PTR_ERR(handle);
                        ext4_msg(inode->i_sb, KERN_CRIT, "%s: jbd2_start: "
                               "%ld pages, ino %lu; err %d", __func__,
                                wbc->nr_to_write, inode->i_ino, ret);
+                      blk_finish_plug(&plug);
                        goto out_writepages;
                }

Thank you,
Kyungmin Park-

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [patch]ext4: add block plug for .writepages
  2011-10-11 10:40             ` Kyungmin Park
  (?)
@ 2011-10-12  0:53             ` Shaohua Li
  -1 siblings, 0 replies; 12+ messages in thread
From: Shaohua Li @ 2011-10-12  0:53 UTC (permalink / raw)
  To: Kyungmin Park
  Cc: Ted Ts'o, Christoph Hellwig, Jens Axboe, Andrew Morton, lkml,
	linux-ext4

On Tue, 2011-10-11 at 18:40 +0800, Kyungmin Park wrote:
> On Tue, Oct 11, 2011 at 1:51 AM, Ted Ts'o <tytso@mit.edu> wrote:
> > On Wed, Sep 28, 2011 at 11:09:40AM +0800, Shaohua Li wrote:
> >> I searched a little bit, looks only ext4 need it. here is the patch.
> >>
> >>
> >> Add block plug for ext4 .writepages. Though ext4 .writepages
> >> already handles request merge very well, block plug is still
> >> helpful to reduce block lock contention.
> >>
> >> Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> 
> Does it require to add blk_finish_plug(&plug) when error case?
oops, we need.

Subject: ext4: calling blk_finish_plug in error path

Forgot calling blk_finish_plug in error code path. Thanks Kyungmin Park pointed
it out.

Signed-off-by: Shaohua Li <shaohua.li@intel.com>
---
 fs/ext4/inode.c |    1 +
 1 file changed, 1 insertion(+)

Index: linux/fs/ext4/inode.c
===================================================================
--- linux.orig/fs/ext4/inode.c	2011-10-12 08:46:32.000000000 +0800
+++ linux/fs/ext4/inode.c	2011-10-12 08:48:10.000000000 +0800
@@ -2144,6 +2144,7 @@ retry:
 			ext4_msg(inode->i_sb, KERN_CRIT, "%s: jbd2_start: "
 			       "%ld pages, ino %lu; err %d", __func__,
 				wbc->nr_to_write, inode->i_ino, ret);
+			blk_finish_plug(&plug);
 			goto out_writepages;
 		}
 



^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2011-10-12  0:46 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-26  2:30 [patch]mm: add more blk plug Shaohua Li
2011-09-26  7:30 ` Christoph Hellwig
2011-09-26  7:38   ` Shaohua Li
2011-09-26  9:45     ` Christoph Hellwig
2011-09-28  3:09       ` [patch]ext4: add block plug for .writepages Shaohua Li
2011-10-09  6:09         ` Shaohua Li
2011-10-10 16:51         ` Ted Ts'o
2011-10-11  0:07           ` NamJae Jeon
2011-10-11  0:07             ` NamJae Jeon
2011-10-11 10:40           ` Kyungmin Park
2011-10-11 10:40             ` Kyungmin Park
2011-10-12  0:53             ` Shaohua Li

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.