On Mon 14-11-11 11:26:00, Christoph Hellwig wrote: > On Mon, Nov 14, 2011 at 05:15:24PM +0100, 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@xxxxxxxxxxxxx> > > Tested-by: Kazuya Mio <k-mio@xxxxxxxxxxxxx> > > Signed-off-by: Jan Kara <jack@xxxxxxx> > > --- > > mm/filemap.c | 11 +++++++++-- > > 1 files changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/mm/filemap.c b/mm/filemap.c > > index c0018f2..166b30e 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,15 @@ again: > > written += copied; > > > > balance_dirty_pages_ratelimited(mapping); > > - > > + /* > > + * We check the signal independently of balance_dirty_pages() > > + * because we need not wait and check for signal there although > > + * this loop could have taken significant amount of time... > > + */ > > + if (fatal_signal_pending(current)) { > > + status = -EINTR; > > + break; > > + } > > Hmm. If we need to check again every time adding the return value to > balance_dirty_pages is rather pointless. > > I have a bit of a problem parsing the comment - does it try to say that > we might spend too much time after the fatal_signal_pending in the > balance_dirty_pages code so that we have to check it again? Why not > repeat the check at the end of balance_dirty_pages_ratelimited and thus > avoid having to duplicate the thing in all callers? The point I was trying to get across is that all: iov_iter_fault_in_readable() ->write_begin() ... ->write_end() can take a rather long time. Considering that balance_dirty_pages_ratelimited() needn't call balance_dirty_pages() or balance_dirty_pages() can just return without doing anything because there aren't many dirty pages, the end result is that balance_dirty_pages_ratelimited() won't return EINTR for rather long time although there is a signal pending. That's why I want to check for signal pending directly in the loop in generic_perform_write(). There can be loops where the only blocking point is in balance_dirty_pages() and then using the return value makes sense but I think such loops would be in minority so maybe the return value doesn't make much sense after all. 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