LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Mike Galbraith <efault@gmx.de>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [vm] writing to UDF DVD+RW (/dev/sr0) while under memory pressure: box ==> doorstop
Date: Wed, 9 Jan 2008 15:01:39 -0800	[thread overview]
Message-ID: <20080109150139.311f68d3.akpm@linux-foundation.org> (raw)
In-Reply-To: <1199877080.4340.19.camel@homer.simson.net>

On Wed, 09 Jan 2008 12:11:20 +0100
Mike Galbraith <efault@gmx.de> wrote:

> 
> On Tue, 2008-01-08 at 16:27 +0100, Mike Galbraith wrote:
> > On Tue, 2008-01-08 at 16:21 +0100, Mike Galbraith wrote:
> > > On Tue, 2008-01-08 at 03:38 -0800, Andrew Morton wrote:
> > > > 
> > > > Well.  From your earlier trace it appeared that something was causing
> > > > the filesystem to perform synchronous inode writes - sync_dirty_buffer() was
> > > > called.
> > > > 
> > > > This will cause many more seeks than would occur if we were doing full
> > > > delayed writing, with obvious throughput implications.
> > > 
> > > Yes, with UDF, the IO was _incredibly_ slow.  With ext2, it was better,
> > > though still very bad.  I tested with that other OS, and it gets ~same
> > > throughput with UDF as I got with ext2 (ick).
> > > 
> > > UDF does udf_clear_inode() -> write_inode_now(inode, 1)
> > > 
> > > I suppose I could try write_inode_now(inode, 0).  Might unstick the box.
> > 
> > (nope, still sync, UDF still deadly)
> 
> write_inode_now() is a fibber.

Sure is.  Looks like it was busted by:

commit fa94396d2792f5093aab7cf66e1fc1da0c9fc442
Author: akpm <akpm>
Date:   Tue Feb 4 17:01:43 2003 +0000

    [PATCH] Remove unneeded code in fs/fs-writeback.c
    
    We do not need to pass the `wait' argument down to __sync_single_inode().
    That information is now present at wbc->sync_mode.
    

> The below seems to fix it in that writes dribbling to the DVD+RW at the
> whopping 1 to 10 pages/sec I'm seeing no longer turn box into a
> doorstop.  It's probably busted as heck.

The VFS change looks good.  Not sure about the UDF details.

> Think I'll cc linux-mm, and go find something safer to play with.
> 
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 0fca820..f1cce24 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -657,7 +657,7 @@ int write_inode_now(struct inode *inode, int sync)
>  	int ret;
>  	struct writeback_control wbc = {
>  		.nr_to_write = LONG_MAX,
> -		.sync_mode = WB_SYNC_ALL,
> +		.sync_mode = sync ? WB_SYNC_ALL : WB_SYNC_NONE,
>  		.range_start = 0,
>  		.range_end = LLONG_MAX,
>  	};
> diff --git a/fs/udf/inode.c b/fs/udf/inode.c
> index 6ff8151..d1fc116 100644
> --- a/fs/udf/inode.c
> +++ b/fs/udf/inode.c
> @@ -117,7 +117,7 @@ void udf_clear_inode(struct inode *inode)
>  		udf_discard_prealloc(inode);
>  		udf_truncate_tail_extent(inode);
>  		unlock_kernel();
> -		write_inode_now(inode, 1);
> +		write_inode_now(inode, 0);
>  	}
>  	kfree(UDF_I_DATA(inode));
>  	UDF_I_DATA(inode) = NULL;
> 

WB_SYNC_* should die.

I wonder why UDF was doing a synchronous write in there.  In fact I wonder
why it's writing the inode at all?  extN doesn't do that.  If for some
reason it really does want to make the inode immediately reclaimable then
simply shoving it down into the /dev/hda1 pagecache should be sufficient
(ie: what you did)..


hm.

So are you saying that the fs throughput is unaltered by this change,
but that the side-effects which your workload has on the overall
machine are lessened?

  reply	other threads:[~2008-01-09 23:02 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-04 11:46 Mike Galbraith
2008-01-06  9:42 ` Mike Galbraith
2008-01-06 18:01   ` Mike Galbraith
2008-01-06 20:29     ` Andrew Morton
2008-01-07  5:33       ` Mike Galbraith
2008-01-07  5:55         ` Mike Galbraith
2008-01-08 11:05       ` Mike Galbraith
2008-01-08 11:38         ` Andrew Morton
2008-01-08 15:21           ` Mike Galbraith
2008-01-08 15:27             ` Mike Galbraith
2008-01-09 11:11               ` Mike Galbraith
2008-01-09 23:01                 ` Andrew Morton [this message]
2008-01-10  4:21                   ` Mike Galbraith
2008-01-10 14:41                   ` Jan Kara
2008-01-10 15:29                     ` Mike Galbraith
2008-01-10 17:16                       ` Jan Kara

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=20080109150139.311f68d3.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=efault@gmx.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --subject='Re: [vm] writing to UDF DVD+RW (/dev/sr0) while under memory pressure: box ==> doorstop' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).