Re: [git pull] iov_iter fixes
From: Al Viro <viro@zeniv.linux.org.uk>
Date: 2021-09-10 16:56:34
Also in:
linux-fsdevel
On Fri, Sep 10, 2021 at 10:06:25AM -0600, Jens Axboe wrote:
Looks something like this. Not super pretty in terms of needing a define for this, and maybe I'm missing something, but ideally we'd want it as an anonymous struct that's defined inside iov_iter. Anyway, gets the point across. Alternatively, since we're down to just a few members now, we just duplicate them in each struct... Would be split into two patches, one for the iov_state addition and the save/restore helpers, and then one switching io_uring to use them. Figured we'd need some agreement on this first...
quoted hunk ↗ jump to hunk
+#define IOV_ITER_STATE \ + size_t iov_offset; \ + size_t count; \ + union { \ + unsigned long nr_segs; \ + struct { \ + unsigned int head; \ + unsigned int start_head; \ + }; \ + loff_t xarray_start; \ + }; \ + +struct iov_iter_state { + IOV_ITER_STATE; +}; + struct iov_iter { u8 iter_type; bool data_source; - size_t iov_offset; - size_t count; union { const struct iovec *iov; const struct kvec *kvec;@@ -40,12 +54,10 @@ struct iov_iter { struct pipe_inode_info *pipe; }; union { - unsigned long nr_segs; + struct iov_iter_state state; struct { - unsigned int head; - unsigned int start_head; + IOV_ITER_STATE; }; - loff_t xarray_start; }; size_t truncated; };
No. This is impossible to read *and* wrong for flavours other than iovec anyway. Rules: count is flavour-independent iovec: iov, nr_segs, iov_offset. nr_segs + iov is constant kvec: kvec, nr_segs, iov_offset. nr_segs + kvec is constant bvec: bvec, nr_segs, iov_offset. nr_segs + bvec is constant xarray: xarray, xarray_start, iov_offset. xarray and xarray_start are constant. pipe: pipe, head, start_head, iov_offset. pipe and start_head are constant, iov_offset can be derived from the rest. discard: nothing. What's more, for pipe (output-only) the situation is much trickier and there this "reset + advance" won't work at all. Simply not applicable. What's the point of all those contortions, anyway? You only need it for iovec case; don't mix doing that and turning it into flavour-independent primitive. Especially since you turn around and access the fields of that sucker (->count, that is) directly in your code. Keep it simple and readable, please. We'll sort the sane flavour-independent API later. And get rid of ->truncate, while we are at it.