From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-fsdevel-owner@vger.kernel.org>
Received: from zeniv.linux.org.uk ([195.92.253.2]:58270 "EHLO
	ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753276AbcAWVkI (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Sat, 23 Jan 2016 16:40:08 -0500
Date: Sat, 23 Jan 2016 21:40:06 +0000
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Mike Marshall <hubcap@omnibond.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: Orangefs ABI documentation
Message-ID: <20160123214006.GO17997@ZenIV.linux.org.uk>
References: <20160122183720.GB17997@ZenIV.linux.org.uk>
 <CAOg9mSTCkr5_or_VSOHnNSsVqd5S=ENuGYBsJkgitqgworN9Eg@mail.gmail.com>
 <CAOg9mSRoEFvFkdJmEmFOO_ErT7X3Q51=X=HP5Li6g-dVFANOOg@mail.gmail.com>
 <20160122200442.GF17997@ZenIV.linux.org.uk>
 <CAOg9mSTPbx0231RoERbbdeyR+nDCD4wGHHFKAzxmowai8GtNgg@mail.gmail.com>
 <20160123001202.GJ17997@ZenIV.linux.org.uk>
 <20160123012808.GK17997@ZenIV.linux.org.uk>
 <CAOg9mST8rpzSs4FGB=7yiGJ2PZBqw2s=H1ozQBiWjDF232ZnhQ@mail.gmail.com>
 <20160123191055.GN17997@ZenIV.linux.org.uk>
 <CAOg9mSR667SdVvR323MOaqEs0DVGKcxr8e0tyx=Bu_=ENops0g@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAOg9mSR667SdVvR323MOaqEs0DVGKcxr8e0tyx=Bu_=ENops0g@mail.gmail.com>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Sat, Jan 23, 2016 at 02:24:51PM -0500, Mike Marshall wrote:
> OK, I'll get them momentarily...
> 
> I merged your other patches, and there was a merge
> conflict I had to work around... you're working from
> an orangefs tree that lacks one commit I had made
> last week... my linux-next tree has all your patches
> through yesterday in it now...
> 
> I am setting up "the gnarly test" (at home from a VM,
> though) that should cause a bunch of cancellations,
> I want to see if I can get
> wait_for_cancellation_downcall to ever
> flow past that "if (signal_pending(current)) {"
> block... if it does, that demonstrate where
> the comments conflict with the code, right?

Yes...  BTW, speaking of that codepath - how can the second caller of
handle_io_error() ever get !op_state_serviced(new_op)?  That failure,
after all, had been in postcopy_buffers(), so the daemon is sitting
in its write_iter() waiting until we finish copying the data out of
bufmap; it's too late for sending cancel anyway, is it not?  IOW, would
the following do the right thing?  That would've left us with only
one caller of handle_io_error()...

diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c
index c585063d..86ba1df 100644
--- a/fs/orangefs/file.c
+++ b/fs/orangefs/file.c
@@ -245,15 +245,8 @@ populate_shared_memory:
 				       buffer_index,
 				       iter,
 				       new_op->downcall.resp.io.amt_complete);
-		if (ret < 0) {
-			/*
-			 * put error codes in downcall so that handle_io_error()
-			 * preserves it properly
-			 */
-			new_op->downcall.status = ret;
-			handle_io_error();
-			goto out;
-		}
+		if (ret < 0)
+			goto done_copying;
 	}
 	gossip_debug(GOSSIP_FILE_DEBUG,
 	    "%s(%pU): Amount written as returned by the sys-io call:%d\n",
@@ -263,6 +256,7 @@ populate_shared_memory:
 
 	ret = new_op->downcall.resp.io.amt_complete;
 
+done_copying:
 	/*
 	 * tell the device file owner waiting on I/O that this read has
 	 * completed and it can return now.