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]:58654 "EHLO
        ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1755638AbeE1Ryb (ORCPT
        <rfc822;linux-fsdevel@vger.kernel.org>);
        Mon, 28 May 2018 13:54:31 -0400
Date: Mon, 28 May 2018 18:54:30 +0100
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 1/4] aio: all callers of aio_{read,write,fsync,poll}
 treat 0 and -EIOCBQUEUED the same way
Message-ID: <20180528175430.GC30522@ZenIV.linux.org.uk>
References: <20180527222730.GS30522@ZenIV.linux.org.uk>
 <20180527222853.30715-1-viro@ZenIV.linux.org.uk>
 <20180528051529.GA2806@lst.de>
 <20180528140434.GX30522@ZenIV.linux.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20180528140434.GX30522@ZenIV.linux.org.uk>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Mon, May 28, 2018 at 03:04:34PM +0100, Al Viro wrote:

> How about lifting the list removal into aio_complete_rw() and aio_poll_work(),
> with WARN_ON() left in its place in aio_complete() itself?  Look:
> aio_compelete() call chains are
> 	aio_complete_rw()
> 	aio_fsync_work()
> 	__aio_poll_complete()
> 		aio_poll_work()
> 		aio_poll_wake()
> 		aio_poll()
> 
> The call in aio_fsync_work() is guaranteed to have iocb not on cancel lists.
> The call in aio_poll_wake() *relies* upon aio_complete() not going into
> list removal.  The call in aio_poll() is also guaranteed to be not on cancel
> list - we get there only if mask != 0 and we add to ->active_reqs only if
> mask == 0.
> 
> So if we take the list removal into aio_complete_rw() and aio_poll_wake() we
> should get the right ordering - iocb gets removed from the list before fput()
> in all cases.  And aio_complete() locking footprint becomes simpler...  As
> a fringe benefit, __aio_poll_complete() becomes simply
> 	fput(req->file);
> 	aio_complete(iocb, mangle_poll(mask), 0);
> since we don't need to order fput() vs. aio_complete() anymore - the caller
> of __aio_poll_complete() has already taken care of ->ki_cancel() possibility.

Anyway, what I have in mind is in vfs.git#work.aio; on top of your fix for missing
break it's
      aio: take list removal to (some) callers of aio_complete()
      aio: all callers of aio_{read,write,fsync,poll} treat 0 and -EIOCBQUEUED the same way
      aio_read_events_ring(): make a bit more readable
      aio: shift copyin of iocb into io_submit_one()
      aio: fold do_io_submit() into callers
      aio: sanitize the limit checking in io_submit(2)
(in followups)