Date: Sat, 10 Dec 2016 01:50:59 +0000
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: v9fs-developer@lists.sourceforge.net, ericvh@gmail.com,
        rminnich@sandia.gov, lucho@ionkov.net, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/5] 9p: introduce async read requests
Message-ID: <20161210015059.GD1555@ZenIV.linux.org.uk>
References: <alpine.DEB.2.10.1612081249000.22778@sstabellini-ThinkPad-X260>
 <1481230746-16741-1-git-send-email-sstabellini@kernel.org>
 <1481230746-16741-4-git-send-email-sstabellini@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1481230746-16741-4-git-send-email-sstabellini@kernel.org>
User-Agent: Mutt/1.7.1 (2016-10-04)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1159
Lines: 35

On Thu, Dec 08, 2016 at 12:59:05PM -0800, Stefano Stabellini wrote:


> +		} else {
> +			req = p9_client_get_req(clnt, P9_TREAD, "dqd", fid->fid, offset, rsize);
> +			if (IS_ERR(req)) {
> +				*err = PTR_ERR(req);
> +				break;
> +			}
> +			req->rsize = iov_iter_get_pages_alloc(to, &req->pagevec, 
> +					(size_t)rsize, &req->offset);
> +			req->kiocb = iocb;
> +			for (i = 0; i < req->rsize; i += PAGE_SIZE)
> +				page_cache_get_speculative(req->pagevec[i/PAGE_SIZE]);
> +			req->callback = p9_client_read_complete;
> +
> +			*err = clnt->trans_mod->request(clnt, req);
> +			if (*err < 0) {
> +				clnt->status = Disconnected;
> +				release_pages(req->pagevec,
> +						(req->rsize + PAGE_SIZE - 1) / PAGE_SIZE,
> +						true);
> +				kvfree(req->pagevec);
> +				p9_free_req(clnt, req);
> +				break;
> +			}
> +
> +			*err = -EIOCBQUEUED;

IDGI.  AFAICS, your code will result in shitloads of short reads - every
time when you give it a multi-iovec array, only the first one will be
issued and the rest won't be even looked at.  Sure, it is technically
legal, but I very much doubt that aio users will be happy with that.

What am I missing here?