Re: [PATCH v3 2/2] iov_iter: Don't deal with iter->copy_mc in memcpy_from_iter_mc()
From: Linus Torvalds
Date: Thu Aug 17 2023 - 00:20:16 EST
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))