On Wed, 16 Aug 2023 at 22:35, David Howells <dhowells@xxxxxxxxxx> wrote: > > I'm not sure that buys us anything. It would then require every call to > iov_iter_is_bvec()[*] to check for two values instead of one Well, that part is trivially fixable, and we should do that anyway for other reasons. See the attached patch. > The issue is that ITER_xyz changes the iteration function - but we don't > actually want to do that; rather, we need to change the step function. Yeah, that may be the fundamental issue. But making the ITER_xyz flags be bit masks would help - partly exactly because it makes it so trivial yo say "for this set of ITER_xyz, do this". This patch only does that for the 'user_backed' thing, which was a similar case. Hmm? Linus
drivers/infiniband/hw/hfi1/file_ops.c | 2 +- drivers/infiniband/hw/qib/qib_file_ops.c | 2 +- include/linux/uio.h | 36 +++++++++++++++----------------- lib/iov_iter.c | 1 - sound/core/pcm_native.c | 4 ++-- 5 files changed, 21 insertions(+), 24 deletions(-) diff --git a/drivers/infiniband/hw/hfi1/file_ops.c b/drivers/infiniband/hw/hfi1/file_ops.c index a5ab22cedd41..788fc249234f 100644 --- a/drivers/infiniband/hw/hfi1/file_ops.c +++ b/drivers/infiniband/hw/hfi1/file_ops.c @@ -267,7 +267,7 @@ static ssize_t hfi1_write_iter(struct kiocb *kiocb, struct iov_iter *from) if (!HFI1_CAP_IS_KSET(SDMA)) return -EINVAL; - if (!from->user_backed) + if (!user_backed_iter(from)) return -EINVAL; idx = srcu_read_lock(&fd->pq_srcu); pq = srcu_dereference(fd->pq, &fd->pq_srcu); diff --git a/drivers/infiniband/hw/qib/qib_file_ops.c b/drivers/infiniband/hw/qib/qib_file_ops.c index ef85bc8d9384..09a6d9121b3d 100644 --- a/drivers/infiniband/hw/qib/qib_file_ops.c +++ b/drivers/infiniband/hw/qib/qib_file_ops.c @@ -2244,7 +2244,7 @@ static ssize_t qib_write_iter(struct kiocb *iocb, struct iov_iter *from) struct qib_ctxtdata *rcd = ctxt_fp(iocb->ki_filp); struct qib_user_sdma_queue *pq = fp->pq; - if (!from->user_backed || !from->nr_segs || !pq) + if (!user_backed_iter(from) || !from->nr_segs || !pq) return -EINVAL; return qib_user_sdma_writev(rcd, pq, iter_iov(from), from->nr_segs); diff --git a/include/linux/uio.h b/include/linux/uio.h index ff81e5ccaef2..230da97a42d5 100644 --- a/include/linux/uio.h +++ b/include/linux/uio.h @@ -21,12 +21,12 @@ struct kvec { enum iter_type { /* iter types */ - ITER_IOVEC, - ITER_KVEC, - ITER_BVEC, - ITER_XARRAY, - ITER_DISCARD, - ITER_UBUF, + ITER_IOVEC = 1, + ITER_UBUF = 2, + ITER_KVEC = 4, + ITER_BVEC = 8, + ITER_XARRAY = 16, + ITER_DISCARD = 32, }; #define ITER_SOURCE 1 // == WRITE @@ -39,11 +39,10 @@ struct iov_iter_state { }; struct iov_iter { - u8 iter_type; - bool copy_mc; - bool nofault; + u8 iter_type:6, + copy_mc:1, + nofault:1; bool data_source; - bool user_backed; union { size_t iov_offset; int last_offset; @@ -85,7 +84,7 @@ struct iov_iter { static inline const struct iovec *iter_iov(const struct iov_iter *iter) { - if (iter->iter_type == ITER_UBUF) + if (iter->iter_type & ITER_UBUF) return (const struct iovec *) &iter->__ubuf_iovec; return iter->__iov; } @@ -108,32 +107,32 @@ static inline void iov_iter_save_state(struct iov_iter *iter, static inline bool iter_is_ubuf(const struct iov_iter *i) { - return iov_iter_type(i) == ITER_UBUF; + return iov_iter_type(i) & ITER_UBUF; } static inline bool iter_is_iovec(const struct iov_iter *i) { - return iov_iter_type(i) == ITER_IOVEC; + return iov_iter_type(i) & ITER_IOVEC; } static inline bool iov_iter_is_kvec(const struct iov_iter *i) { - return iov_iter_type(i) == ITER_KVEC; + return iov_iter_type(i) & ITER_KVEC; } static inline bool iov_iter_is_bvec(const struct iov_iter *i) { - return iov_iter_type(i) == ITER_BVEC; + return iov_iter_type(i) & ITER_BVEC; } static inline bool iov_iter_is_discard(const struct iov_iter *i) { - return iov_iter_type(i) == ITER_DISCARD; + return iov_iter_type(i) & ITER_DISCARD; } static inline bool iov_iter_is_xarray(const struct iov_iter *i) { - return iov_iter_type(i) == ITER_XARRAY; + return iov_iter_type(i) & ITER_XARRAY; } static inline unsigned char iov_iter_rw(const struct iov_iter *i) @@ -143,7 +142,7 @@ static inline unsigned char iov_iter_rw(const struct iov_iter *i) static inline bool user_backed_iter(const struct iov_iter *i) { - return i->user_backed; + return i->iter_type & (ITER_IOVEC | ITER_UBUF); } /* @@ -376,7 +375,6 @@ static inline void iov_iter_ubuf(struct iov_iter *i, unsigned int direction, *i = (struct iov_iter) { .iter_type = ITER_UBUF, .copy_mc = false, - .user_backed = true, .data_source = direction, .ubuf = buf, .count = count, diff --git a/lib/iov_iter.c b/lib/iov_iter.c index e4dc809d1075..857e661d1554 100644 --- a/lib/iov_iter.c +++ b/lib/iov_iter.c @@ -290,7 +290,6 @@ void iov_iter_init(struct iov_iter *i, unsigned int direction, .iter_type = ITER_IOVEC, .copy_mc = false, .nofault = false, - .user_backed = true, .data_source = direction, .__iov = iov, .nr_segs = nr_segs, diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 95fc56e403b1..642dceeb80ee 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -3527,7 +3527,7 @@ static ssize_t snd_pcm_readv(struct kiocb *iocb, struct iov_iter *to) if (runtime->state == SNDRV_PCM_STATE_OPEN || runtime->state == SNDRV_PCM_STATE_DISCONNECTED) return -EBADFD; - if (!to->user_backed) + if (!user_backed_iter(to)) return -EINVAL; if (to->nr_segs > 1024 || to->nr_segs != runtime->channels) return -EINVAL; @@ -3567,7 +3567,7 @@ static ssize_t snd_pcm_writev(struct kiocb *iocb, struct iov_iter *from) if (runtime->state == SNDRV_PCM_STATE_OPEN || runtime->state == SNDRV_PCM_STATE_DISCONNECTED) return -EBADFD; - if (!from->user_backed) + if (!user_backed_iter(from)) return -EINVAL; if (from->nr_segs > 128 || from->nr_segs != runtime->channels || !frame_aligned(runtime, iov->iov_len))