Re: [PATCH 2/2] fs: Make write(2) interruptible by a signal

[Wu Fengguang <fengguang.wu@intel.com>]

On Wed, Nov 16, 2011 at 08:54:45PM +0800, Jan Kara wrote:
> On Wed 16-11-11 19:44:21, Wu Fengguang wrote:
> > Jan,
> > 
> > Due to the (very low) possibility of data loss by partial writes, IMHO
> > it would safer to test this patch in linux-next until next merge window,
> > would you agree?
>   Fine with me. Thanks.

Great. The two patches are now in:

http://git.kernel.org/?p=linux/kernel/git/wfg/linux.git;a=shortlog;h=refs/heads/writeback-for-next

When applying the patch I changed the title "... by a signal" to
"...  by SIGKILL" to reflect the updated patch content. Hopefully
this is also what's in your mind.

> > Pushing the first patch will address the main problem, anyway.
>   Hopefully, yes.

I looked over Kazuya's posts in linux-ext4 and think the first patch
alone does have good chance address the problem of busy looping in
balance_dirty_pages() due to dirty pages never drop under fs error
conditions. 

Thanks,
Fengguang

> > On Wed, Nov 16, 2011 at 07:12:15PM +0800, Jan Kara wrote:
> > > Currently write(2) to a file is not interruptible by a signal. Sometimes this
> > > is desirable (e.g. when you want to quickly kill a process hogging your disk or
> > > when some process gets blocked in balance_dirty_pages() indefinitely due to a
> > > filesystem being in an error condition).
> > > 
> > > Reported-by: Kazuya Mio <k-mio@sx.jp.nec.com>
> > > Tested-by: Kazuya Mio <k-mio@sx.jp.nec.com>
> > > Signed-off-by: Jan Kara <jack@suse.cz>
> > > ---
> > >  mm/filemap.c |    6 ++++--
> > >  1 files changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/mm/filemap.c b/mm/filemap.c
> > > index c0018f2..c106d3b 100644
> > > --- a/mm/filemap.c
> > > +++ b/mm/filemap.c
> > > @@ -2407,7 +2407,6 @@ static ssize_t generic_perform_write(struct file *file,
> > >  						iov_iter_count(i));
> > >  
> > >  again:
> > > -
> > >  		/*
> > >  		 * Bring in the user page that we will copy from _first_.
> > >  		 * Otherwise there's a nasty deadlock on copying from the
> > > @@ -2463,7 +2462,10 @@ again:
> > >  		written += copied;
> > >  
> > >  		balance_dirty_pages_ratelimited(mapping);
> > > -
> > > +		if (fatal_signal_pending(current)) {
> > > +			status = -EINTR;
> > > +			break;
> > > +		}
> > >  	} while (iov_iter_count(i));
> > >  
> > >  	return written ? written : status;
> > > -- 
> > > 1.7.1
> -- 
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR