On Tue 15-11-11 22:15:28, Wu Fengguang wrote:
> 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().
Ah, OK, I see. Frankly, I'd rather keep the loop break in
generic_perform_write() and have the code straightforward. I don't see any
security (data exposure) problem there and if someone complains on the
grounds that the behavior has changed, we can revert the change in the
worst case.
Honza
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
