Re: [PATCH 40/44] 9p: convert to advancing variant of iov_iter_get_pages_alloc()

[Christian Schoenebeck <linux_oss@crudebyte.com>]

On Mittwoch, 22. Juni 2022 06:15:48 CEST Al Viro wrote:
> that one is somewhat clumsier than usual and needs serious testing.
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Hi Al,

it took me a bit to find the patch that introduces
iov_iter_get_pages_alloc2(), but this patch itself looks fine:

Reviewed-by: Christian Schoenebeck <linux_oss@crudebyte.com>

Please give me some days for thorough testing. We recently had 9p broken (with
cache=loose) for half a year, so I would like to avoid repetition.

Best regards,
Christian Schoenebeck

> ---
>  net/9p/client.c       | 39 +++++++++++++++++++++++----------------
>  net/9p/protocol.c     |  3 +--
>  net/9p/trans_virtio.c |  3 ++-
>  3 files changed, 26 insertions(+), 19 deletions(-)
> 
> diff --git a/net/9p/client.c b/net/9p/client.c
> index d403085b9ef5..cb4324211561 100644
> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -1491,7 +1491,7 @@ p9_client_read_once(struct p9_fid *fid, u64 offset,
> struct iov_iter *to, struct p9_client *clnt = fid->clnt;
>  	struct p9_req_t *req;
>  	int count = iov_iter_count(to);
> -	int rsize, non_zc = 0;
> +	int rsize, received, non_zc = 0;
>  	char *dataptr;
> 
>  	*err = 0;
> @@ -1520,36 +1520,40 @@ p9_client_read_once(struct p9_fid *fid, u64 offset,
> struct iov_iter *to, }
>  	if (IS_ERR(req)) {
>  		*err = PTR_ERR(req);
> +		if (!non_zc)
> +			iov_iter_revert(to, count - iov_iter_count(to));
>  		return 0;
>  	}
> 
>  	*err = p9pdu_readf(&req->rc, clnt->proto_version,
> -			   "D", &count, &dataptr);
> +			   "D", &received, &dataptr);
>  	if (*err) {
> +		if (!non_zc)
> +			iov_iter_revert(to, count - iov_iter_count(to));
>  		trace_9p_protocol_dump(clnt, &req->rc);
>  		p9_tag_remove(clnt, req);
>  		return 0;
>  	}
> -	if (rsize < count) {
> -		pr_err("bogus RREAD count (%d > %d)\n", count, rsize);
> -		count = rsize;
> +	if (rsize < received) {
> +		pr_err("bogus RREAD count (%d > %d)\n", received, rsize);
> +		received = rsize;
>  	}
> 
>  	p9_debug(P9_DEBUG_9P, "<<< RREAD count %d\n", count);
> 
>  	if (non_zc) {
> -		int n = copy_to_iter(dataptr, count, to);
> +		int n = copy_to_iter(dataptr, received, to);
> 
> -		if (n != count) {
> +		if (n != received) {
>  			*err = -EFAULT;
>  			p9_tag_remove(clnt, req);
>  			return n;
>  		}
>  	} else {
> -		iov_iter_advance(to, count);
> +		iov_iter_revert(to, count - received - iov_iter_count(to));
>  	}
>  	p9_tag_remove(clnt, req);
> -	return count;
> +	return received;
>  }
>  EXPORT_SYMBOL(p9_client_read_once);
> 
> @@ -1567,6 +1571,7 @@ p9_client_write(struct p9_fid *fid, u64 offset, struct
> iov_iter *from, int *err) while (iov_iter_count(from)) {
>  		int count = iov_iter_count(from);
>  		int rsize = fid->iounit;
> +		int written;
> 
>  		if (!rsize || rsize > clnt->msize - P9_IOHDRSZ)
>  			rsize = clnt->msize - P9_IOHDRSZ;
> @@ -1584,27 +1589,29 @@ p9_client_write(struct p9_fid *fid, u64 offset,
> struct iov_iter *from, int *err) offset, rsize, from);
>  		}
>  		if (IS_ERR(req)) {
> +			iov_iter_revert(from, count - iov_iter_count(from));
>  			*err = PTR_ERR(req);
>  			break;
>  		}
> 
> -		*err = p9pdu_readf(&req->rc, clnt->proto_version, "d", &count);
> +		*err = p9pdu_readf(&req->rc, clnt->proto_version, "d", &written);
>  		if (*err) {
> +			iov_iter_revert(from, count - iov_iter_count(from));
>  			trace_9p_protocol_dump(clnt, &req->rc);
>  			p9_tag_remove(clnt, req);
>  			break;
>  		}
> -		if (rsize < count) {
> -			pr_err("bogus RWRITE count (%d > %d)\n", count, rsize);
> -			count = rsize;
> +		if (rsize < written) {
> +			pr_err("bogus RWRITE count (%d > %d)\n", written, rsize);
> +			written = rsize;
>  		}
> 
>  		p9_debug(P9_DEBUG_9P, "<<< RWRITE count %d\n", count);
> 
>  		p9_tag_remove(clnt, req);
> -		iov_iter_advance(from, count);
> -		total += count;
> -		offset += count;
> +		iov_iter_revert(from, count - written - iov_iter_count(from));
> +		total += written;
> +		offset += written;
>  	}
>  	return total;
>  }
> diff --git a/net/9p/protocol.c b/net/9p/protocol.c
> index 3754c33e2974..83694c631989 100644
> --- a/net/9p/protocol.c
> +++ b/net/9p/protocol.c
> @@ -63,9 +63,8 @@ static size_t
>  pdu_write_u(struct p9_fcall *pdu, struct iov_iter *from, size_t size)
>  {
>  	size_t len = min(pdu->capacity - pdu->size, size);
> -	struct iov_iter i = *from;
> 
> -	if (!copy_from_iter_full(&pdu->sdata[pdu->size], len, &i))
> +	if (!copy_from_iter_full(&pdu->sdata[pdu->size], len, from))
>  		len = 0;
> 
>  	pdu->size += len;
> diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
> index 2a210c2f8e40..1977d33475fe 100644
> --- a/net/9p/trans_virtio.c
> +++ b/net/9p/trans_virtio.c
> @@ -331,7 +331,7 @@ static int p9_get_mapped_pages(struct virtio_chan *chan,
> if (err == -ERESTARTSYS)
>  				return err;
>  		}
> -		n = iov_iter_get_pages_alloc(data, pages, count, offs);
> +		n = iov_iter_get_pages_alloc2(data, pages, count, offs);
>  		if (n < 0)
>  			return n;
>  		*need_drop = 1;
> @@ -373,6 +373,7 @@ static int p9_get_mapped_pages(struct virtio_chan *chan,
> (*pages)[index] = kmap_to_page(p);
>  			p += PAGE_SIZE;
>  		}
> +		iov_iter_advance(data, len);
>  		return len;
>  	}
>  }