Re: [PATCH 06/12] writeback: remove unused nonblocking and congestion checks (gfs2)

Re: [PATCH 06/12] writeback: remove unused nonblocking and congestion checks (gfs2)

[Steven Whitehouse]

Hi,

That looks ok to me, modulo the warnings below. The only "non-obvious"
thing in this area which the GFS2 writepage[s] code relies upon is the
assumption that if we have a ->writepages() then ->writepage() will
never be called from a context which requires the fs to actually do a
write (i.e. the fs can refuse this if required). That is also only the
case for journaled data files - normal writes don't have that
requirement.

 CC [M]  fs/gfs2/aops.o
fs/gfs2/aops.c: In function ‘gfs2_write_jdata_pagevec’:
fs/gfs2/aops.c:272: warning: unused variable ‘bdi’
fs/gfs2/aops.c: In function ‘gfs2_write_cache_jdata’:
fs/gfs2/aops.c:336: warning: unused variable ‘bdi’

Once the warnings are fixed:

Acked-by: Steven Whitehouse <swhiteho@redhat.com>

Do you want me to add this patch into my tree, or were you planning to
submit via a different tree?

Steve.

On Wed, 2009-11-18 at 16:26 +0800, Wu Fengguang wrote:
> plain text document attachment
> (writeback-remove-congested-checks-linux_fs_gfs2_aops.patch)
> No one is calling wb_writeback and write_cache_pages with
> wbc.nonblocking=1 any more. And lumpy pageout will want to do
> nonblocking writeback without the congestion wait.
> 
> CC: Steven Whitehouse <swhiteho@redhat.com>
> Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
> ---
>  fs/gfs2/aops.c |   10 ----------
>  1 file changed, 10 deletions(-)
> 
> --- linux.orig/fs/gfs2/aops.c	2009-11-06 09:22:35.000000000 +0800
> +++ linux/fs/gfs2/aops.c	2009-11-06 09:52:15.000000000 +0800
> @@ -313,11 +313,6 @@ static int gfs2_write_jdata_pagevec(stru
>  
>  		if (ret || (--(wbc->nr_to_write) <= 0))
>  			ret = 1;
> -		if (wbc->nonblocking && bdi_write_congested(bdi)) {
> -			wbc->encountered_congestion = 1;
> -			ret = 1;
> -		}
> -
>  	}
>  	gfs2_trans_end(sdp);
>  	return ret;
> @@ -348,11 +343,6 @@ static int gfs2_write_cache_jdata(struct
>  	int scanned = 0;
>  	int range_whole = 0;
>  
> -	if (wbc->nonblocking && bdi_write_congested(bdi)) {
> -		wbc->encountered_congestion = 1;
> -		return 0;
> -	}
> -
>  	pagevec_init(&pvec, 0);
>  	if (wbc->range_cyclic) {
>  		index = mapping->writeback_index; /* Start from prev offset */
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 06/12] writeback: remove unused nonblocking and congestion checks (gfs2)

[Wu Fengguang]

Hi Steven,

On Wed, Nov 18, 2009 at 05:59:46PM +0800, Steven Whitehouse wrote:
> Hi,
> 
> That looks ok to me, modulo the warnings below. The only "non-obvious"
> thing in this area which the GFS2 writepage[s] code relies upon is the
> assumption that if we have a ->writepages() then ->writepage() will
> never be called from a context which requires the fs to actually do a
> write (i.e. the fs can refuse this if required). That is also only the
> case for journaled data files - normal writes don't have that
> requirement.

Thank you for the tips. I don't think pageout() or migration
writeout() has that hard expectation for ->writepage() :)

>  CC [M]  fs/gfs2/aops.o
> fs/gfs2/aops.c: In function ‘gfs2_write_jdata_pagevec’:
> fs/gfs2/aops.c:272: warning: unused variable ‘bdi’
> fs/gfs2/aops.c: In function ‘gfs2_write_cache_jdata’:
> fs/gfs2/aops.c:336: warning: unused variable ‘bdi’

Ah sorry!

> Once the warnings are fixed:
> 
> Acked-by: Steven Whitehouse <swhiteho@redhat.com>
> 
> Do you want me to add this patch into my tree, or were you planning to
> submit via a different tree?

Thanks, can you pull the updated patch directly to your tree?

Thanks,
Fengguang
---
writeback: remove unused nonblocking and congestion checks (gfs2)

No one is calling wb_writeback and write_cache_pages with
wbc.nonblocking=1 any more. And lumpy pageout will want to do
nonblocking writeback without the congestion wait.

Acked-by: Steven Whitehouse <swhiteho@redhat.com>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 fs/gfs2/aops.c |   12 ------------
 1 file changed, 12 deletions(-)

--- linux.orig/fs/gfs2/aops.c	2009-11-18 16:39:32.000000000 +0800
+++ linux/fs/gfs2/aops.c	2009-11-18 18:01:14.000000000 +0800
@@ -269,7 +269,6 @@ static int gfs2_write_jdata_pagevec(stru
 	pgoff_t end_index = i_size >> PAGE_CACHE_SHIFT;
 	unsigned offset = i_size & (PAGE_CACHE_SIZE-1);
 	unsigned nrblocks = nr_pages * (PAGE_CACHE_SIZE/inode->i_sb->s_blocksize);
-	struct backing_dev_info *bdi = mapping->backing_dev_info;
 	int i;
 	int ret;
 
@@ -313,11 +312,6 @@ static int gfs2_write_jdata_pagevec(stru
 
 		if (ret || (--(wbc->nr_to_write) <= 0))
 			ret = 1;
-		if (wbc->nonblocking && bdi_write_congested(bdi)) {
-			wbc->encountered_congestion = 1;
-			ret = 1;
-		}
-
 	}
 	gfs2_trans_end(sdp);
 	return ret;
@@ -338,7 +332,6 @@ static int gfs2_write_jdata_pagevec(stru
 static int gfs2_write_cache_jdata(struct address_space *mapping,
 				  struct writeback_control *wbc)
 {
-	struct backing_dev_info *bdi = mapping->backing_dev_info;
 	int ret = 0;
 	int done = 0;
 	struct pagevec pvec;
@@ -348,11 +341,6 @@ static int gfs2_write_cache_jdata(struct
 	int scanned = 0;
 	int range_whole = 0;
 
-	if (wbc->nonblocking && bdi_write_congested(bdi)) {
-		wbc->encountered_congestion = 1;
-		return 0;
-	}
-
 	pagevec_init(&pvec, 0);
 	if (wbc->range_cyclic) {
 		index = mapping->writeback_index; /* Start from prev offset */

Re: [PATCH 12/12] bdi: use bdi_stat_sum() for more accurate debugfs stats

[Peter Zijlstra]

On Wed, 2009-11-18 at 16:27 +0800, Wu Fengguang wrote:
> plain text document attachment (bdi-debug-dump-sum.patch)
> CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
> ---
>  mm/backing-dev.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- linux.orig/mm/backing-dev.c	2009-11-18 16:25:28.000000000 +0800
> +++ linux/mm/backing-dev.c	2009-11-18 16:26:10.000000000 +0800
> @@ -104,8 +104,8 @@ static int bdi_debug_stats_show(struct s
>  		   "wb_mask:          %8lx\n"
>  		   "wb_list:          %8u\n"
>  		   "wb_cnt:           %8u\n",
> -		   (unsigned long) K(bdi_stat(bdi, BDI_WRITEBACK)),
> -		   (unsigned long) K(bdi_stat(bdi, BDI_RECLAIMABLE)),
> +		   (unsigned long) K(bdi_stat_sum(bdi, BDI_WRITEBACK)),
> +		   (unsigned long) K(bdi_stat_sum(bdi, BDI_RECLAIMABLE)),
>  		   K(bdi_thresh), K(dirty_thresh),
>  		   K(background_thresh), nr_wb, nr_dirty, nr_io, nr_more_io,
>  		   !list_empty(&bdi->bdi_list), bdi->state, bdi->wb_mask,
> 

Is this really important? This patch is basically a local DoS for large
machines.

Imagine someone doing:

while :; do cat /debug/bdi/*/stats; done

on a 512 cpu box.

Re: [PATCH 06/12] writeback: remove unused nonblocking and congestion checks (gfs2)

[Steven Whitehouse]

Hi,

On Wed, 2009-11-18 at 18:09 +0800, Wu Fengguang wrote:
> Hi Steven,
> 
> On Wed, Nov 18, 2009 at 05:59:46PM +0800, Steven Whitehouse wrote:
> > Hi,
> > 
> > That looks ok to me, modulo the warnings below. The only "non-obvious"
> > thing in this area which the GFS2 writepage[s] code relies upon is the
> > assumption that if we have a ->writepages() then ->writepage() will
> > never be called from a context which requires the fs to actually do a
> > write (i.e. the fs can refuse this if required). That is also only the
> > case for journaled data files - normal writes don't have that
> > requirement.
> 
> Thank you for the tips. I don't think pageout() or migration
> writeout() has that hard expectation for ->writepage() :)
> 
> >  CC [M]  fs/gfs2/aops.o
> > fs/gfs2/aops.c: In function ‘gfs2_write_jdata_pagevec’:
> > fs/gfs2/aops.c:272: warning: unused variable ‘bdi’
> > fs/gfs2/aops.c: In function ‘gfs2_write_cache_jdata’:
> > fs/gfs2/aops.c:336: warning: unused variable ‘bdi’
> 
> Ah sorry!
> 
> > Once the warnings are fixed:
> > 
> > Acked-by: Steven Whitehouse <swhiteho@redhat.com>
> > 
> > Do you want me to add this patch into my tree, or were you planning to
> > submit via a different tree?
> 
> Thanks, can you pull the updated patch directly to your tree?
> 
Yes, its in my -nmw tree now. Thanks,

Steve.


--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/12] writeback: remove unused nonblocking and congestion checks (xfs)

[Dave Chinner]

On Wed, Nov 18, 2009 at 04:26:55PM +0800, Wu Fengguang wrote:
> No one is calling wb_writeback and write_cache_pages with
> wbc.nonblocking=1 any more. And lumpy pageout will want to do
> nonblocking writeback without the congestion wait.

....

> --- linux.orig/fs/xfs/linux-2.6/xfs_aops.c	2009-11-06 09:22:35.000000000 +0800
> +++ linux/fs/xfs/linux-2.6/xfs_aops.c	2009-11-06 09:52:21.000000000 +0800
> @@ -908,12 +908,8 @@ xfs_convert_page(
>  
>  			bdi = inode->i_mapping->backing_dev_info;
>  			wbc->nr_to_write--;
> -			if (bdi_write_congested(bdi)) {
> -				wbc->encountered_congestion = 1;

bdi is unused now, so can be removed as well. Otherwise looks fine.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 12/12] bdi: use bdi_stat_sum() for more accurate debugfs stats

[Wu Fengguang]

On Wed, Nov 18, 2009 at 06:38:49PM +0800, Peter Zijlstra wrote:
> On Wed, 2009-11-18 at 16:27 +0800, Wu Fengguang wrote:
> > plain text document attachment (bdi-debug-dump-sum.patch)
> > CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
> > ---
> >  mm/backing-dev.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > --- linux.orig/mm/backing-dev.c	2009-11-18 16:25:28.000000000 +0800
> > +++ linux/mm/backing-dev.c	2009-11-18 16:26:10.000000000 +0800
> > @@ -104,8 +104,8 @@ static int bdi_debug_stats_show(struct s
> >  		   "wb_mask:          %8lx\n"
> >  		   "wb_list:          %8u\n"
> >  		   "wb_cnt:           %8u\n",
> > -		   (unsigned long) K(bdi_stat(bdi, BDI_WRITEBACK)),
> > -		   (unsigned long) K(bdi_stat(bdi, BDI_RECLAIMABLE)),
> > +		   (unsigned long) K(bdi_stat_sum(bdi, BDI_WRITEBACK)),
> > +		   (unsigned long) K(bdi_stat_sum(bdi, BDI_RECLAIMABLE)),
> >  		   K(bdi_thresh), K(dirty_thresh),
> >  		   K(background_thresh), nr_wb, nr_dirty, nr_io, nr_more_io,
> >  		   !list_empty(&bdi->bdi_list), bdi->state, bdi->wb_mask,
> > 
> 
> Is this really important? This patch is basically a local DoS for large
> machines.

I did this patch after seeing inaccurate exported numbers,
it may be confusing..

> Imagine someone doing:
> 
> while :; do cat /debug/bdi/*/stats; done
> 
> on a 512 cpu box.

Yes there will be overheads. However it's always possible to
create local DoS with some other kind of busy loop?

Thanks,
Fengguang

Re: [PATCH 07/12] writeback: remove unused nonblocking and congestion checks (xfs)

[Wu Fengguang]

On Thu, Nov 19, 2009 at 05:27:55AM +0800, Dave Chinner wrote:
> On Wed, Nov 18, 2009 at 04:26:55PM +0800, Wu Fengguang wrote:
> > No one is calling wb_writeback and write_cache_pages with
> > wbc.nonblocking=1 any more. And lumpy pageout will want to do
> > nonblocking writeback without the congestion wait.
> 
> ....
> 
> > --- linux.orig/fs/xfs/linux-2.6/xfs_aops.c	2009-11-06 09:22:35.000000000 +0800
> > +++ linux/fs/xfs/linux-2.6/xfs_aops.c	2009-11-06 09:52:21.000000000 +0800
> > @@ -908,12 +908,8 @@ xfs_convert_page(
> >  
> >  			bdi = inode->i_mapping->backing_dev_info;
> >  			wbc->nr_to_write--;
> > -			if (bdi_write_congested(bdi)) {
> > -				wbc->encountered_congestion = 1;
> 
> bdi is unused now, so can be removed as well. Otherwise looks fine.

Thanks, here is the updated patch.

---
writeback: remove unused nonblocking and congestion checks (xfs)

No one is calling wb_writeback and write_cache_pages with
wbc.nonblocking=1 any more. And lumpy pageout will want to do
nonblocking writeback without the congestion wait.

CC: Dave Chinner <david@fromorbit.com> 
CC: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 fs/xfs/linux-2.6/xfs_aops.c |    9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

--- linux.orig/fs/xfs/linux-2.6/xfs_aops.c	2009-11-19 16:00:51.000000000 +0800
+++ linux/fs/xfs/linux-2.6/xfs_aops.c	2009-11-19 16:04:33.000000000 +0800
@@ -904,16 +904,9 @@ xfs_convert_page(
 
 	if (startio) {
 		if (count) {
-			struct backing_dev_info *bdi;
-
-			bdi = inode->i_mapping->backing_dev_info;
 			wbc->nr_to_write--;
-			if (bdi_write_congested(bdi)) {
-				wbc->encountered_congestion = 1;
-				done = 1;
-			} else if (wbc->nr_to_write <= 0) {
+			if (wbc->nr_to_write <= 0)
 				done = 1;
-			}
 		}
 		xfs_start_page_writeback(page, !page_dirty, count);
 	}

Re: [PATCH 04/12] writeback: remove unused nonblocking and congestion checks (afs)

[Wu Fengguang]

Hi David,

Will you pick up this updated patch? It also removes the unused bdi.

Thanks,
Fengguang
---
writeback: remove unused nonblocking and congestion checks (afs)

No one is calling wb_writeback and write_cache_pages with
wbc.nonblocking=1 any more. And lumpy pageout will want to do
nonblocking writeback without the congestion wait.

CC: David Howells <dhowells@redhat.com>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 fs/afs/write.c |   19 +------------------
 1 file changed, 1 insertion(+), 18 deletions(-)

--- linux.orig/fs/afs/write.c	2009-11-18 16:39:33.000000000 +0800
+++ linux/fs/afs/write.c	2009-11-19 16:07:31.000000000 +0800
@@ -438,7 +438,6 @@ no_more:
  */
 int afs_writepage(struct page *page, struct writeback_control *wbc)
 {
-	struct backing_dev_info *bdi = page->mapping->backing_dev_info;
 	struct afs_writeback *wb;
 	int ret;
 
@@ -455,8 +454,6 @@ int afs_writepage(struct page *page, str
 	}
 
 	wbc->nr_to_write -= ret;
-	if (wbc->nonblocking && bdi_write_congested(bdi))
-		wbc->encountered_congestion = 1;
 
 	_leave(" = 0");
 	return 0;
@@ -469,7 +466,6 @@ static int afs_writepages_region(struct 
 				 struct writeback_control *wbc,
 				 pgoff_t index, pgoff_t end, pgoff_t *_next)
 {
-	struct backing_dev_info *bdi = mapping->backing_dev_info;
 	struct afs_writeback *wb;
 	struct page *page;
 	int ret, n;
@@ -529,11 +525,6 @@ static int afs_writepages_region(struct 
 
 		wbc->nr_to_write -= ret;
 
-		if (wbc->nonblocking && bdi_write_congested(bdi)) {
-			wbc->encountered_congestion = 1;
-			break;
-		}
-
 		cond_resched();
 	} while (index < end && wbc->nr_to_write > 0);
 
@@ -548,24 +539,16 @@ static int afs_writepages_region(struct 
 int afs_writepages(struct address_space *mapping,
 		   struct writeback_control *wbc)
 {
-	struct backing_dev_info *bdi = mapping->backing_dev_info;
 	pgoff_t start, end, next;
 	int ret;
 
 	_enter("");
 
-	if (wbc->nonblocking && bdi_write_congested(bdi)) {
-		wbc->encountered_congestion = 1;
-		_leave(" = 0 [congest]");
-		return 0;
-	}
-
 	if (wbc->range_cyclic) {
 		start = mapping->writeback_index;
 		end = -1;
 		ret = afs_writepages_region(mapping, wbc, start, end, &next);
-		if (start > 0 && wbc->nr_to_write > 0 && ret == 0 &&
-		    !(wbc->nonblocking && wbc->encountered_congestion))
+		if (start > 0 && wbc->nr_to_write > 0 && ret == 0)
 			ret = afs_writepages_region(mapping, wbc, 0, start,
 						    &next);
 		mapping->writeback_index = next;

Re: [PATCH 03/12] writeback: remove unused nonblocking and congestion checks (pohmelfs)

[Wu Fengguang]

Hi Evgeniy,

Will you pick up this updated patch? It also removes the unused bdi.

Thanks,
Fengguang
---
writeback: remove unused nonblocking and congestion checks (pohmelfs)

No one is calling wb_writeback and write_cache_pages with
wbc.nonblocking=1 any more. And lumpy pageout will want to do
nonblocking writeback without the congestion wait.

CC: Evgeniy Polyakov <zbr@ioremap.net>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 drivers/staging/pohmelfs/inode.c |   10 ----------
 1 file changed, 10 deletions(-)

--- linux.orig/drivers/staging/pohmelfs/inode.c	2009-11-18 16:39:36.000000000 +0800
+++ linux/drivers/staging/pohmelfs/inode.c	2009-11-19 16:10:29.000000000 +0800
@@ -143,7 +143,6 @@ static int pohmelfs_writepages(struct ad
 	struct inode *inode = mapping->host;
 	struct pohmelfs_inode *pi = POHMELFS_I(inode);
 	struct pohmelfs_sb *psb = POHMELFS_SB(inode->i_sb);
-	struct backing_dev_info *bdi = mapping->backing_dev_info;
 	int err = 0;
 	int done = 0;
 	int nr_pages;
@@ -152,11 +151,6 @@ static int pohmelfs_writepages(struct ad
 	int scanned = 0;
 	int range_whole = 0;
 
-	if (wbc->nonblocking && bdi_write_congested(bdi)) {
-		wbc->encountered_congestion = 1;
-		return 0;
-	}
-
 	if (wbc->range_cyclic) {
 		index = mapping->writeback_index; /* Start from prev offset */
 		end = -1;
@@ -248,10 +242,6 @@ retry:
 
 			if (wbc->nr_to_write <= 0)
 				done = 1;
-			if (wbc->nonblocking && bdi_write_congested(bdi)) {
-				wbc->encountered_congestion = 1;
-				done = 1;
-			}
 
 			continue;
 out_continue:

Re: [PATCH 12/12] bdi: use bdi_stat_sum() for more accurate debugfs stats

[Peter Zijlstra]

On Thu, 2009-11-19 at 15:59 +0800, Wu Fengguang wrote:
> On Wed, Nov 18, 2009 at 06:38:49PM +0800, Peter Zijlstra wrote:
> > On Wed, 2009-11-18 at 16:27 +0800, Wu Fengguang wrote:
> > > plain text document attachment (bdi-debug-dump-sum.patch)
> > > CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > > Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
> > > ---
> > >  mm/backing-dev.c |    4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > --- linux.orig/mm/backing-dev.c	2009-11-18 16:25:28.000000000 +0800
> > > +++ linux/mm/backing-dev.c	2009-11-18 16:26:10.000000000 +0800
> > > @@ -104,8 +104,8 @@ static int bdi_debug_stats_show(struct s
> > >  		   "wb_mask:          %8lx\n"
> > >  		   "wb_list:          %8u\n"
> > >  		   "wb_cnt:           %8u\n",
> > > -		   (unsigned long) K(bdi_stat(bdi, BDI_WRITEBACK)),
> > > -		   (unsigned long) K(bdi_stat(bdi, BDI_RECLAIMABLE)),
> > > +		   (unsigned long) K(bdi_stat_sum(bdi, BDI_WRITEBACK)),
> > > +		   (unsigned long) K(bdi_stat_sum(bdi, BDI_RECLAIMABLE)),
> > >  		   K(bdi_thresh), K(dirty_thresh),
> > >  		   K(background_thresh), nr_wb, nr_dirty, nr_io, nr_more_io,
> > >  		   !list_empty(&bdi->bdi_list), bdi->state, bdi->wb_mask,
> > > 
> > 
> > Is this really important? This patch is basically a local DoS for large
> > machines.
> 
> I did this patch after seeing inaccurate exported numbers,
> it may be confusing..
> 
> > Imagine someone doing:
> > 
> > while :; do cat /debug/bdi/*/stats; done
> > 
> > on a 512 cpu box.
> 
> Yes there will be overheads. However it's always possible to
> create local DoS with some other kind of busy loop?

Preferably not of the kind that renders your box unusable, esp by unpriv
users.

But yes there are probably a few local DoS' around, but I don't think
that warrants adding another one.

Re: [PATCH 12/12] bdi: use bdi_stat_sum() for more accurate debugfs stats

[Wu Fengguang]

On Thu, Nov 19, 2009 at 04:12:04PM +0800, Peter Zijlstra wrote:
> On Thu, 2009-11-19 at 15:59 +0800, Wu Fengguang wrote:
> > On Wed, Nov 18, 2009 at 06:38:49PM +0800, Peter Zijlstra wrote:
> > > On Wed, 2009-11-18 at 16:27 +0800, Wu Fengguang wrote:
> > > > plain text document attachment (bdi-debug-dump-sum.patch)
> > > > CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > > > Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
> > > > ---
> > > >  mm/backing-dev.c |    4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > --- linux.orig/mm/backing-dev.c	2009-11-18 16:25:28.000000000 +0800
> > > > +++ linux/mm/backing-dev.c	2009-11-18 16:26:10.000000000 +0800
> > > > @@ -104,8 +104,8 @@ static int bdi_debug_stats_show(struct s
> > > >  		   "wb_mask:          %8lx\n"
> > > >  		   "wb_list:          %8u\n"
> > > >  		   "wb_cnt:           %8u\n",
> > > > -		   (unsigned long) K(bdi_stat(bdi, BDI_WRITEBACK)),
> > > > -		   (unsigned long) K(bdi_stat(bdi, BDI_RECLAIMABLE)),
> > > > +		   (unsigned long) K(bdi_stat_sum(bdi, BDI_WRITEBACK)),
> > > > +		   (unsigned long) K(bdi_stat_sum(bdi, BDI_RECLAIMABLE)),
> > > >  		   K(bdi_thresh), K(dirty_thresh),
> > > >  		   K(background_thresh), nr_wb, nr_dirty, nr_io, nr_more_io,
> > > >  		   !list_empty(&bdi->bdi_list), bdi->state, bdi->wb_mask,
> > > > 
> > > 
> > > Is this really important? This patch is basically a local DoS for large
> > > machines.
> > 
> > I did this patch after seeing inaccurate exported numbers,
> > it may be confusing..
> > 
> > > Imagine someone doing:
> > > 
> > > while :; do cat /debug/bdi/*/stats; done
> > > 
> > > on a 512 cpu box.
> > 
> > Yes there will be overheads. However it's always possible to
> > create local DoS with some other kind of busy loop?
> 
> Preferably not of the kind that renders your box unusable, esp by unpriv
> users.
> 
> But yes there are probably a few local DoS' around, but I don't think
> that warrants adding another one.

OK I'll drop this patch. I'm not feeling strong on either way.

Andrew/Jens, do you prefer for me to repost the non-fs-specific patches?

Thanks,
Fengguang

Re: [PATCH 03/12] writeback: remove unused nonblocking and congestion checks (pohmelfs)

[Evgeniy Polyakov]

H1.

On Thu, Nov 19, 2009 at 04:11:54PM +0800, Wu Fengguang (fengguang.wu@intel.com) wrote:
> Will you pick up this updated patch? It also removes the unused bdi.

I'm ok with the changes, but I suppose it should be pushed as a single
patchet into the tree, no?

-- 
	Evgeniy Polyakov

Re: [PATCH 04/12] writeback: remove unused nonblocking and congestion checks (afs)

[David Howells]

Wu Fengguang <fengguang.wu@intel.com> wrote:

> Will you pick up this updated patch? It also removes the unused bdi.

Sure - but it might be in a week and a bit.  I'm scrambling to get stuff done
before going on holiday next week.

David

Re: [PATCH 03/12] writeback: remove unused nonblocking and congestion checks (pohmelfs)

[Wu Fengguang]

Hi Evgeniy,

On Thu, Nov 19, 2009 at 07:04:13PM +0800, Evgeniy Polyakov wrote:
> H1.
> 
> On Thu, Nov 19, 2009 at 04:11:54PM +0800, Wu Fengguang (fengguang.wu@intel.com) wrote:
> > Will you pick up this updated patch? It also removes the unused bdi.
> 
> I'm ok with the changes, but I suppose it should be pushed as a single
> patchet into the tree, no?

Either way works. I would submit a single patchset that includes the
generic ones and the fs specific patches that have not been picked up
into their fs tree :)

Thanks,
Fengguang

Re: [PATCH 07/12] writeback: remove unused nonblocking and congestion checks (xfs)

[Dave Chinner]

On Thu, Nov 19, 2009 at 04:05:22PM +0800, Wu Fengguang wrote:
> On Thu, Nov 19, 2009 at 05:27:55AM +0800, Dave Chinner wrote:
> > On Wed, Nov 18, 2009 at 04:26:55PM +0800, Wu Fengguang wrote:
> > > No one is calling wb_writeback and write_cache_pages with
> > > wbc.nonblocking=1 any more. And lumpy pageout will want to do
> > > nonblocking writeback without the congestion wait.
> > 
> > ....
> > 
> > > --- linux.orig/fs/xfs/linux-2.6/xfs_aops.c	2009-11-06 09:22:35.000000000 +0800
> > > +++ linux/fs/xfs/linux-2.6/xfs_aops.c	2009-11-06 09:52:21.000000000 +0800
> > > @@ -908,12 +908,8 @@ xfs_convert_page(
> > >  
> > >  			bdi = inode->i_mapping->backing_dev_info;
> > >  			wbc->nr_to_write--;
> > > -			if (bdi_write_congested(bdi)) {
> > > -				wbc->encountered_congestion = 1;
> > 
> > bdi is unused now, so can be removed as well. Otherwise looks fine.
> 
> Thanks, here is the updated patch.

Looks good.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 03/12] writeback: remove unused nonblocking and congestion checks (pohmelfs)

[Evgeniy Polyakov]

Hi.

On Fri, Nov 20, 2009 at 09:53:17AM +0800, Wu Fengguang (fengguang.wu@intel.com) wrote:
> > On Thu, Nov 19, 2009 at 04:11:54PM +0800, Wu Fengguang (fengguang.wu@intel.com) wrote:
> > > Will you pick up this updated patch? It also removes the unused bdi.
> > 
> > I'm ok with the changes, but I suppose it should be pushed as a single
> > patchet into the tree, no?
> 
> Either way works. I would submit a single patchset that includes the
> generic ones and the fs specific patches that have not been picked up
> into their fs tree :)

Please push it within your patchset to eliminate merge dependencies if
any. Thank you.

-- 
	Evgeniy Polyakov

Re: [PATCH 08/12] ext4: remove encountered_congestion trace

[tytso]

On Wed, Nov 18, 2009 at 04:26:56PM +0800, Wu Fengguang wrote:
> It is no longer set and scheduled to be removed.
> 
> CC: Theodore Ts'o <tytso@mit.edu>
> Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>

Thanks, I've queued it in the ext4 patch queue.

	     	       	      - Ted

Re: [PATCH 09/12] ext4: remove unused parameter wbc from __ext4_journalled_writepage()

[tytso]

On Wed, Nov 18, 2009 at 04:26:57PM +0800, Wu Fengguang wrote:
> CC: Theodore Ts'o <tytso@mit.edu> 
> CC: Jan Kara <jack@suse.cz> 
> Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>

Thanks, I've queued it in the ext4 patch queue.

	     	       	      	   	 - Ted