On Mon, Jan 23 2017, Jeff Layton wrote:

On Tue, 2017-01-24 at 11:16 +1100, NeilBrown wrote:

quoted

On Mon, Jan 23 2017, Trond Myklebust wrote:

quoted

On Mon, 2017-01-23 at 17:35 -0500, Jeff Layton wrote:

quoted

On Mon, 2017-01-23 at 11:09 +0100, Kevin Wolf wrote:

quoted

However, if we look at the greater problem of hanging requests that
came
up in the more recent emails of this thread, it is only moved
rather
than solved. Chances are that already write() would hang now
instead of
only fsync(), but we still have a hard time dealing with this.

Well, it _is_ better with O_DIRECT as you can usually at least break
out
of the I/O with SIGKILL.

When I last looked at this, the problem with buffered I/O was that
you
often end up waiting on page bits to clear (usually PG_writeback or
PG_dirty), in non-killable sleeps for the most part.

Maybe the fix here is as simple as changing that?

At the risk of kicking off another O_PONIES discussion: Add an
open(O_TIMEOUT) flag that would let the kernel know that the
application is prepared to handle timeouts from operations such as
read(), write() and fsync(), then add an ioctl() or syscall to allow
said application to set the timeout value.

I was thinking on very similar lines, though I'd use 'fcntl()' if
possible because it would be a per-"file description" option.
This would be a function of the page cache, and a filesystem wouldn't
need to know about it at all.  Once enable, 'read', 'write', or 'fsync'
would return EWOULDBLOCK rather than waiting indefinitely.
It might be nice if 'select' could then be used on page-cache file
descriptors, but I think that is much harder.  Support O_TIMEOUT would
be a practical first step - if someone agreed to actually try to use it.

Yeah, that does seem like it might be worth exploring. 

That said, I think there's something even simpler we can do to make
things better for a lot of cases, and it may even help pave the way for
the proposal above.

Looking closer and remembering more, I think the main problem area when
the pages are stuck in writeback is the wait_on_page_writeback call in
places like wait_for_stable_page and __filemap_fdatawait_range.

I can't see wait_for_stable_page() being very relevant.  That only
blocks on backing devices which have requested stable pages.
raid5 sometimes does that.  Some scsi/sata devices can somehow.
And rbd (part of ceph) sometimes does.  I don't think NFS ever will.
wait_for_stable_page() doesn't currently return an error, so getting to
abort in SIGKILL would be a lot of work.

filemap_fdatawait_range() is much easier.

diff --git a/mm/filemap.c b/mm/filemap.c
index b772a33ef640..2773f6dde1da 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c

@@ -401,7 +401,9 @@ static int __filemap_fdatawait_range(struct address_space *mapping,
 			if (page->index > end)
 				continue;
 
-			wait_on_page_writeback(page);
+			if (PageWriteback(page))
+				if (wait_on_page_bit_killable(page, PG_writeback))
+					err = -ERESTARTSYS;
 			if (TestClearPageError(page))
 				ret = -EIO;
 		}

That isn't a complete solution. There is code in f2fs which doesn't
check the return value and probably should.  And gfs2 calls
	mapping_set_error(mapping, error);
with the return value, with we probably don't want in the ERESTARTSYS case.
There are some usages in btrfs that I'd need to double-check too.

But it looks to be manageable. 

Thanks,
NeilBrown

That uses an uninterruptible sleep and it's common to see applications
stuck there in these situations. They're unkillable too so your only
recourse is to hard reset the box when you can't reestablish
connectivity.

I think it might be good to consider making some of those sleeps
TASK_KILLABLE. For instance, both of the above callers of those
functions are int return functions. It may be possible to return
ERESTARTSYS when the task catches a signal.

-- 
Jeff Layton [off-list ref]