From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lb0-f172.google.com ([209.85.217.172]:34049 "EHLO mail-lb0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758530AbcCCWZK (ORCPT ); Thu, 3 Mar 2016 17:25:10 -0500 Received: by mail-lb0-f172.google.com with SMTP id cf7so24577026lbb.1 for ; Thu, 03 Mar 2016 14:25:09 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: <20160122200442.GF17997@ZenIV.linux.org.uk> <20160123001202.GJ17997@ZenIV.linux.org.uk> <20160123012808.GK17997@ZenIV.linux.org.uk> <20160123191055.GN17997@ZenIV.linux.org.uk> <20160123214006.GO17997@ZenIV.linux.org.uk> <20160123224632.GQ17997@ZenIV.linux.org.uk> Date: Thu, 3 Mar 2016 17:25:08 -0500 Message-ID: Subject: Re: write() semantics (Re: Orangefs ABI documentation) From: Mike Marshall To: Linus Torvalds Cc: Al Viro , linux-fsdevel , Mike Marshall Content-Type: text/plain; charset=UTF-8 Sender: linux-fsdevel-owner@vger.kernel.org List-ID: Here is what I have come up with to try and make our return to interrupted writes more acceptable... in my tests it seems to work. My test involved running the client-core with a tiny IO buffer size (4k) and a C program with a large write buffer(32M). That way there was plenty of time for me to fire off the C program and hit Ctrl-C while the write was chugging along. The return value from do_readv_writev always matches the size of the file written by the aborted C program in my tests. I changed the C program around so that sometimes I ran it with (O_CREAT | O_RDWR) on open and sometimes with (O_CREAT | O_RDWR | O_APPEND) and it seemed to do the right thing. I didn't try to set up a signal handler so that the signal wasn't fatal to the process, I guess that would be a test to actually see and verify the correct short return code to write... Do you all think this looks like it should work in principle? BTW: in the distant past someone else attempted to solve this problem the "nfs intr" way - we have an intr mount option, and that's why there's all that sweating over whether or not stuff is "interruptible" in waitqueue.c... I'm not sure if our intr mount option is relevant anymore given the way the op handling code has evolved... diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c index 6f2e0f7..4349c9d 100644 --- a/fs/orangefs/file.c +++ b/fs/orangefs/file.c @@ -180,21 +180,54 @@ populate_shared_memory: } if (ret < 0) { - /* - * don't write an error to syslog on signaled operation - * termination unless we've got debugging turned on, as - * this can happen regularly (i.e. ctrl-c) - */ - if (ret == -EINTR) + if (ret == -EINTR) { + /* + * We can't return EINTR if any data was written, + * it's not POSIX. It is minimally acceptable + * to give a partial write, the way NFS does. + * + * It would be optimal to return all or nothing, + * but if a userspace write is bigger than + * an IO buffer, and the interrupt occurs + * between buffer writes, that would not be + * possible. + */ + switch (new_op->op_state - OP_VFS_STATE_GIVEN_UP) { + /* + * If the op was waiting when the interrupt + * occurred, then the client-core did not + * trigger the write. + */ + case OP_VFS_STATE_WAITING: + ret = 0; + break; + /* + * If the op was in progress when the interrupt + * occurred, then the client-core was able to + * trigger the write. + */ + case OP_VFS_STATE_INPROGR: + ret = total_size; + break; + default: + gossip_err("%s: unexpected op state :%d:.\n", + __func__, + new_op->op_state); + ret = 0; + break; + } gossip_debug(GOSSIP_FILE_DEBUG, - "%s: returning error %ld\n", __func__, - (long)ret); - else + "%s: got EINTR, state:%d: %p\n", + __func__, + new_op->op_state, + new_op); + } else { gossip_err("%s: error in %s handle %pU, returning %zd\n", __func__, type == ORANGEFS_IO_READ ? "read from" : "write to", handle, ret); + } if (orangefs_cancel_op_in_progress(new_op)) return ret; On Sat, Jan 23, 2016 at 6:35 PM, Linus Torvalds wrote: > On Sat, Jan 23, 2016 at 2:46 PM, Al Viro wrote: >> >> What should we get? -EINTR, despite having written some data? > > No, that's not acceptable. > > Either all or nothing (which is POSIX) or the NFS 'intr' mount > behavior (partial write return, -EINTR only when nothing was written > at all). And, like NFS, a mount option might be a good thing. > > And of course, for the usual reasons, fatal signals are special in > that for them we generally say "screw posix, nobody sees the return > value anyway", but even there the filesystem might as well still > return the partial return value (just to not introduce yet another > special case). > > In fact, I think that with our "fatal signals interrupt" behavior, > nobody should likely use the "intr" mount option on NFS. Even if the > semantics may be "better", there are likely simply just too many > programs that don't check the return value of "write()" at all, much > less handle partial writes correctly. > > (And yes, our "screw posix" behavior wrt fatal signals is strictly > wrong even _despite_ the fact that nobody sees the return value - > other processes can still obviously see that the whole write wasn't > done. But blocking on a fatal signal is _so_ annoying that it's one of > those things where we just say "posix was wrong on this one, and if we > squint a bit we look _almost_ like we're compliant"). > > Linus