Re: [PATCH 1/2] mm: Make task in balance_dirty_pages() killable

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

On Tue, Nov 15, 2011 at 09:41:27PM +0800, Jan Kara wrote:
> On Tue 15-11-11 19:48:44, Wu Fengguang wrote:
> > > +static int balance_dirty_pages(struct address_space *mapping,
> > >  				unsigned long pages_dirtied)
> > >  {
> > >  	unsigned long nr_reclaimable;	/* = file_dirty + unstable_nfs */
> > > @@ -1020,6 +1023,7 @@ static void balance_dirty_pages(struct address_space *mapping,
> > >  	unsigned long pos_ratio;
> > >  	struct backing_dev_info *bdi = mapping->backing_dev_info;
> > >  	unsigned long start_time = jiffies;
> > > +	int err = 0;
> > >  
> > >  	for (;;) {
> > >  		/*
> > > @@ -1133,7 +1137,7 @@ pause:
> > >  					  pages_dirtied,
> > >  					  pause,
> > >  					  start_time);
> > > -		__set_current_state(TASK_UNINTERRUPTIBLE);
> > > +		__set_current_state(TASK_KILLABLE);
> > >  		io_schedule_timeout(pause);
> > >  
> > >  		dirty_thresh = hard_dirty_limit(dirty_thresh);
> > > @@ -1145,6 +1149,11 @@ pause:
> > >  		 */
> > >  		if (nr_dirty < dirty_thresh)
> > >  			break;
> > > +
> > > +		if (fatal_signal_pending(current)) {
> > > +			err = -EINTR;
> > > +			break;
> > > +		}
> > >  	}
> >   
> > The other alternative is to raise the limit on fatal_signal_pending:
> > 
> >                 if (fatal_signal_pending(current) && 
> >                     nr_dirty < dirty_thresh + dirty_thresh / 2)
> >                         break;
> > 
> > That should work well enough in practice and avoids touching the fs code.
>   Sorry, but I fail to see what would this bring us... Can you elaborate a
> bit please?

It's not bringing us something, but allows us to get rid of patch 2
as well as adding fatal_signal_pending() tests to all the other fs.

So no more worries about partial writes :)

Sure it leaves ->write_begin() blocks unhandled, however that should
be much less a problem than the blocks inside balance_dirty_pages().

Thanks,
Fengguang