* [PATCH 01/11] block: ensure bio_alloc_map_data() deals with ITER_UBUF correctly
2023-03-29 18:40 [PATCHSET v6 0/11] Turn single segment imports into ITER_UBUF Jens Axboe
@ 2023-03-29 18:40 ` Jens Axboe
2023-03-29 18:40 ` [PATCH 02/11] iov_iter: add iter_iovec() helper Jens Axboe
` (10 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Jens Axboe @ 2023-03-29 18:40 UTC (permalink / raw)
To: linux-fsdevel; +Cc: torvalds, brauner, viro, Jens Axboe
This helper blindly copies the iovec, even if we don't have one.
Make this case a bit smarter by only doing so if we have an iovec
array to copy.
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
block/blk-map.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/block/blk-map.c b/block/blk-map.c
index 9137d16cecdc..3bfcad64d67c 100644
--- a/block/blk-map.c
+++ b/block/blk-map.c
@@ -29,10 +29,11 @@ static struct bio_map_data *bio_alloc_map_data(struct iov_iter *data,
bmd = kmalloc(struct_size(bmd, iov, data->nr_segs), gfp_mask);
if (!bmd)
return NULL;
- memcpy(bmd->iov, data->iov, sizeof(struct iovec) * data->nr_segs);
bmd->iter = *data;
- if (iter_is_iovec(data))
+ if (iter_is_iovec(data)) {
+ memcpy(bmd->iov, data->iov, sizeof(struct iovec) * data->nr_segs);
bmd->iter.iov = bmd->iov;
+ }
return bmd;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 02/11] iov_iter: add iter_iovec() helper
2023-03-29 18:40 [PATCHSET v6 0/11] Turn single segment imports into ITER_UBUF Jens Axboe
2023-03-29 18:40 ` [PATCH 01/11] block: ensure bio_alloc_map_data() deals with ITER_UBUF correctly Jens Axboe
@ 2023-03-29 18:40 ` Jens Axboe
2023-03-29 18:40 ` [PATCH 03/11] iov_iter: add iter_iov_addr() and iter_iov_len() helpers Jens Axboe
` (9 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Jens Axboe @ 2023-03-29 18:40 UTC (permalink / raw)
To: linux-fsdevel; +Cc: torvalds, brauner, viro, Jens Axboe
This returns a pointer to the current iovec entry in the iterator. Only
useful with ITER_IOVEC right now, but it prepares us to treat ITER_UBUF
and ITER_IOVEC identically for the first segment.
Rename struct iov_iter->iov to iov_iter->__iov to find any potentially
troublesome spots, and also to prevent anyone from adding new code that
accesses iter->iov directly.
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
block/blk-map.c | 4 +-
drivers/infiniband/hw/hfi1/file_ops.c | 3 +-
drivers/infiniband/hw/qib/qib_file_ops.c | 2 +-
drivers/net/tun.c | 3 +-
drivers/vhost/scsi.c | 2 +-
fs/btrfs/file.c | 11 +++--
fs/fuse/file.c | 2 +-
include/linux/uio.h | 9 ++--
io_uring/net.c | 4 +-
io_uring/rw.c | 8 ++--
lib/iov_iter.c | 56 +++++++++++++-----------
sound/core/pcm_native.c | 22 ++++++----
12 files changed, 73 insertions(+), 53 deletions(-)
diff --git a/block/blk-map.c b/block/blk-map.c
index 3bfcad64d67c..04c55f1c492e 100644
--- a/block/blk-map.c
+++ b/block/blk-map.c
@@ -31,8 +31,8 @@ static struct bio_map_data *bio_alloc_map_data(struct iov_iter *data,
return NULL;
bmd->iter = *data;
if (iter_is_iovec(data)) {
- memcpy(bmd->iov, data->iov, sizeof(struct iovec) * data->nr_segs);
- bmd->iter.iov = bmd->iov;
+ memcpy(bmd->iov, iter_iov(data), sizeof(struct iovec) * data->nr_segs);
+ bmd->iter.__iov = bmd->iov;
}
return bmd;
}
diff --git a/drivers/infiniband/hw/hfi1/file_ops.c b/drivers/infiniband/hw/hfi1/file_ops.c
index b1d6ca7e9708..3065db9d6bb9 100644
--- a/drivers/infiniband/hw/hfi1/file_ops.c
+++ b/drivers/infiniband/hw/hfi1/file_ops.c
@@ -287,11 +287,12 @@ static ssize_t hfi1_write_iter(struct kiocb *kiocb, struct iov_iter *from)
}
while (dim) {
+ const struct iovec *iov = iter_iov(from);
int ret;
unsigned long count = 0;
ret = hfi1_user_sdma_process_request(
- fd, (struct iovec *)(from->iov + done),
+ fd, (struct iovec *)(iov + done),
dim, &count);
if (ret) {
reqs = ret;
diff --git a/drivers/infiniband/hw/qib/qib_file_ops.c b/drivers/infiniband/hw/qib/qib_file_ops.c
index 80fe92a21f96..4cee39337866 100644
--- a/drivers/infiniband/hw/qib/qib_file_ops.c
+++ b/drivers/infiniband/hw/qib/qib_file_ops.c
@@ -2248,7 +2248,7 @@ static ssize_t qib_write_iter(struct kiocb *iocb, struct iov_iter *from)
if (!iter_is_iovec(from) || !from->nr_segs || !pq)
return -EINVAL;
- return qib_user_sdma_writev(rcd, pq, from->iov, from->nr_segs);
+ return qib_user_sdma_writev(rcd, pq, iter_iov(from), from->nr_segs);
}
static struct class *qib_class;
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index ad653b32b2f0..5df1eba7b30a 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1486,7 +1486,8 @@ static struct sk_buff *tun_napi_alloc_frags(struct tun_file *tfile,
skb->truesize += skb->data_len;
for (i = 1; i < it->nr_segs; i++) {
- size_t fragsz = it->iov[i].iov_len;
+ const struct iovec *iov = iter_iov(it);
+ size_t fragsz = iov->iov_len;
struct page *page;
void *frag;
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index b244e7c0f514..042caea64007 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -671,7 +671,7 @@ vhost_scsi_calc_sgls(struct iov_iter *iter, size_t bytes, int max_sgls)
{
int sgl_count = 0;
- if (!iter || !iter->iov) {
+ if (!iter || !iter_iov(iter)) {
pr_err("%s: iter->iov is NULL, but expected bytes: %zu"
" present\n", __func__, bytes);
return -EINVAL;
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 5cc5a1faaef5..f649647392e0 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -3730,10 +3730,15 @@ static int check_direct_read(struct btrfs_fs_info *fs_info,
if (!iter_is_iovec(iter))
return 0;
- for (seg = 0; seg < iter->nr_segs; seg++)
- for (i = seg + 1; i < iter->nr_segs; i++)
- if (iter->iov[seg].iov_base == iter->iov[i].iov_base)
+ for (seg = 0; seg < iter->nr_segs; seg++) {
+ for (i = seg + 1; i < iter->nr_segs; i++) {
+ const struct iovec *iov1 = iter_iov(iter) + seg;
+ const struct iovec *iov2 = iter_iov(iter) + i;
+
+ if (iov1->iov_base == iov2->iov_base)
return -EINVAL;
+ }
+ }
return 0;
}
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index de37a3a06a71..89d97f6188e0 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1419,7 +1419,7 @@ static ssize_t fuse_cache_write_iter(struct kiocb *iocb, struct iov_iter *from)
static inline unsigned long fuse_get_user_addr(const struct iov_iter *ii)
{
- return (unsigned long)ii->iov->iov_base + ii->iov_offset;
+ return (unsigned long)iter_iov(ii)->iov_base + ii->iov_offset;
}
static inline size_t fuse_get_frag_size(const struct iov_iter *ii,
diff --git a/include/linux/uio.h b/include/linux/uio.h
index 27e3fd942960..4218624b7f78 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -51,7 +51,8 @@ struct iov_iter {
};
size_t count;
union {
- const struct iovec *iov;
+ /* use iter_iov() to get the current vec */
+ const struct iovec *__iov;
const struct kvec *kvec;
const struct bio_vec *bvec;
struct xarray *xarray;
@@ -68,6 +69,8 @@ struct iov_iter {
};
};
+#define iter_iov(iter) (iter)->__iov
+
static inline enum iter_type iov_iter_type(const struct iov_iter *i)
{
return i->iter_type;
@@ -146,9 +149,9 @@ static inline size_t iov_length(const struct iovec *iov, unsigned long nr_segs)
static inline struct iovec iov_iter_iovec(const struct iov_iter *iter)
{
return (struct iovec) {
- .iov_base = iter->iov->iov_base + iter->iov_offset,
+ .iov_base = iter_iov(iter)->iov_base + iter->iov_offset,
.iov_len = min(iter->count,
- iter->iov->iov_len - iter->iov_offset),
+ iter_iov(iter)->iov_len - iter->iov_offset),
};
}
diff --git a/io_uring/net.c b/io_uring/net.c
index 4040cf093318..89e839013837 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -184,8 +184,8 @@ static int io_setup_async_msg(struct io_kiocb *req,
async_msg->msg.msg_name = &async_msg->addr;
/* if were using fast_iov, set it to the new one */
if (iter_is_iovec(&kmsg->msg.msg_iter) && !kmsg->free_iov) {
- size_t fast_idx = kmsg->msg.msg_iter.iov - kmsg->fast_iov;
- async_msg->msg.msg_iter.iov = &async_msg->fast_iov[fast_idx];
+ size_t fast_idx = iter_iov(&kmsg->msg.msg_iter) - kmsg->fast_iov;
+ async_msg->msg.msg_iter.__iov = &async_msg->fast_iov[fast_idx];
}
return -EAGAIN;
diff --git a/io_uring/rw.c b/io_uring/rw.c
index 4c233910e200..7573a34ea42a 100644
--- a/io_uring/rw.c
+++ b/io_uring/rw.c
@@ -503,10 +503,10 @@ static void io_req_map_rw(struct io_kiocb *req, const struct iovec *iovec,
if (!iovec) {
unsigned iov_off = 0;
- io->s.iter.iov = io->s.fast_iov;
- if (iter->iov != fast_iov) {
- iov_off = iter->iov - fast_iov;
- io->s.iter.iov += iov_off;
+ io->s.iter.__iov = io->s.fast_iov;
+ if (iter->__iov != fast_iov) {
+ iov_off = iter_iov(iter) - fast_iov;
+ io->s.iter.__iov += iov_off;
}
if (io->s.fast_iov != fast_iov)
memcpy(io->s.fast_iov + iov_off, fast_iov + iov_off,
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 274014e4eafe..87488c4aad3f 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -126,13 +126,13 @@ __out: \
iterate_buf(i, n, base, len, off, \
i->ubuf, (I)) \
} else if (likely(iter_is_iovec(i))) { \
- const struct iovec *iov = i->iov; \
+ const struct iovec *iov = iter_iov(i); \
void __user *base; \
size_t len; \
iterate_iovec(i, n, base, len, off, \
iov, (I)) \
- i->nr_segs -= iov - i->iov; \
- i->iov = iov; \
+ i->nr_segs -= iov - iter_iov(i); \
+ i->__iov = iov; \
} else if (iov_iter_is_bvec(i)) { \
const struct bio_vec *bvec = i->bvec; \
void *base; \
@@ -355,7 +355,7 @@ size_t fault_in_iov_iter_readable(const struct iov_iter *i, size_t size)
size_t skip;
size -= count;
- for (p = i->iov, skip = i->iov_offset; count; p++, skip = 0) {
+ for (p = iter_iov(i), skip = i->iov_offset; count; p++, skip = 0) {
size_t len = min(count, p->iov_len - skip);
size_t ret;
@@ -398,7 +398,7 @@ size_t fault_in_iov_iter_writeable(const struct iov_iter *i, size_t size)
size_t skip;
size -= count;
- for (p = i->iov, skip = i->iov_offset; count; p++, skip = 0) {
+ for (p = iter_iov(i), skip = i->iov_offset; count; p++, skip = 0) {
size_t len = min(count, p->iov_len - skip);
size_t ret;
@@ -425,7 +425,7 @@ void iov_iter_init(struct iov_iter *i, unsigned int direction,
.nofault = false,
.user_backed = true,
.data_source = direction,
- .iov = iov,
+ .__iov = iov,
.nr_segs = nr_segs,
.iov_offset = 0,
.count = count
@@ -876,14 +876,14 @@ static void iov_iter_iovec_advance(struct iov_iter *i, size_t size)
i->count -= size;
size += i->iov_offset; // from beginning of current segment
- for (iov = i->iov, end = iov + i->nr_segs; iov < end; iov++) {
+ for (iov = iter_iov(i), end = iov + i->nr_segs; iov < end; iov++) {
if (likely(size < iov->iov_len))
break;
size -= iov->iov_len;
}
i->iov_offset = size;
- i->nr_segs -= iov - i->iov;
- i->iov = iov;
+ i->nr_segs -= iov - iter_iov(i);
+ i->__iov = iov;
}
void iov_iter_advance(struct iov_iter *i, size_t size)
@@ -958,12 +958,12 @@ void iov_iter_revert(struct iov_iter *i, size_t unroll)
unroll -= n;
}
} else { /* same logics for iovec and kvec */
- const struct iovec *iov = i->iov;
+ const struct iovec *iov = iter_iov(i);
while (1) {
size_t n = (--iov)->iov_len;
i->nr_segs++;
if (unroll <= n) {
- i->iov = iov;
+ i->__iov = iov;
i->iov_offset = n - unroll;
return;
}
@@ -980,7 +980,7 @@ size_t iov_iter_single_seg_count(const struct iov_iter *i)
{
if (i->nr_segs > 1) {
if (likely(iter_is_iovec(i) || iov_iter_is_kvec(i)))
- return min(i->count, i->iov->iov_len - i->iov_offset);
+ return min(i->count, iter_iov(i)->iov_len - i->iov_offset);
if (iov_iter_is_bvec(i))
return min(i->count, i->bvec->bv_len - i->iov_offset);
}
@@ -1095,13 +1095,14 @@ static bool iov_iter_aligned_iovec(const struct iov_iter *i, unsigned addr_mask,
unsigned k;
for (k = 0; k < i->nr_segs; k++, skip = 0) {
- size_t len = i->iov[k].iov_len - skip;
+ const struct iovec *iov = iter_iov(i) + k;
+ size_t len = iov->iov_len - skip;
if (len > size)
len = size;
if (len & len_mask)
return false;
- if ((unsigned long)(i->iov[k].iov_base + skip) & addr_mask)
+ if ((unsigned long)(iov->iov_base + skip) & addr_mask)
return false;
size -= len;
@@ -1194,9 +1195,10 @@ static unsigned long iov_iter_alignment_iovec(const struct iov_iter *i)
unsigned k;
for (k = 0; k < i->nr_segs; k++, skip = 0) {
- size_t len = i->iov[k].iov_len - skip;
+ const struct iovec *iov = iter_iov(i) + k;
+ size_t len = iov->iov_len - skip;
if (len) {
- res |= (unsigned long)i->iov[k].iov_base + skip;
+ res |= (unsigned long)iov->iov_base + skip;
if (len > size)
len = size;
res |= len;
@@ -1273,14 +1275,15 @@ unsigned long iov_iter_gap_alignment(const struct iov_iter *i)
return ~0U;
for (k = 0; k < i->nr_segs; k++) {
- if (i->iov[k].iov_len) {
- unsigned long base = (unsigned long)i->iov[k].iov_base;
+ const struct iovec *iov = iter_iov(i) + k;
+ if (iov->iov_len) {
+ unsigned long base = (unsigned long)iov->iov_base;
if (v) // if not the first one
res |= base | v; // this start | previous end
- v = base + i->iov[k].iov_len;
- if (size <= i->iov[k].iov_len)
+ v = base + iov->iov_len;
+ if (size <= iov->iov_len)
break;
- size -= i->iov[k].iov_len;
+ size -= iov->iov_len;
}
}
return res;
@@ -1396,13 +1399,14 @@ static unsigned long first_iovec_segment(const struct iov_iter *i, size_t *size)
return (unsigned long)i->ubuf + i->iov_offset;
for (k = 0, skip = i->iov_offset; k < i->nr_segs; k++, skip = 0) {
- size_t len = i->iov[k].iov_len - skip;
+ const struct iovec *iov = iter_iov(i) + k;
+ size_t len = iov->iov_len - skip;
if (unlikely(!len))
continue;
if (*size > len)
*size = len;
- return (unsigned long)i->iov[k].iov_base + skip;
+ return (unsigned long)iov->iov_base + skip;
}
BUG(); // if it had been empty, we wouldn't get called
}
@@ -1614,7 +1618,7 @@ static int iov_npages(const struct iov_iter *i, int maxpages)
const struct iovec *p;
int npages = 0;
- for (p = i->iov; size; skip = 0, p++) {
+ for (p = iter_iov(i); size; skip = 0, p++) {
unsigned offs = offset_in_page(p->iov_base + skip);
size_t len = min(p->iov_len - skip, size);
@@ -1691,7 +1695,7 @@ const void *dup_iter(struct iov_iter *new, struct iov_iter *old, gfp_t flags)
flags);
else if (iov_iter_is_kvec(new) || iter_is_iovec(new))
/* iovec and kvec have identical layout */
- return new->iov = kmemdup(new->iov,
+ return new->__iov = kmemdup(new->__iov,
new->nr_segs * sizeof(struct iovec),
flags);
return NULL;
@@ -1918,7 +1922,7 @@ void iov_iter_restore(struct iov_iter *i, struct iov_iter_state *state)
if (iov_iter_is_bvec(i))
i->bvec -= state->nr_segs - i->nr_segs;
else
- i->iov -= state->nr_segs - i->nr_segs;
+ i->__iov -= state->nr_segs - i->nr_segs;
i->nr_segs = state->nr_segs;
}
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 331380c2438b..8bb97ee6720d 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -3521,6 +3521,7 @@ static ssize_t snd_pcm_readv(struct kiocb *iocb, struct iov_iter *to)
unsigned long i;
void __user **bufs;
snd_pcm_uframes_t frames;
+ const struct iovec *iov = iter_iov(to);
pcm_file = iocb->ki_filp->private_data;
substream = pcm_file->substream;
@@ -3534,14 +3535,16 @@ static ssize_t snd_pcm_readv(struct kiocb *iocb, struct iov_iter *to)
return -EINVAL;
if (to->nr_segs > 1024 || to->nr_segs != runtime->channels)
return -EINVAL;
- if (!frame_aligned(runtime, to->iov->iov_len))
+ if (!frame_aligned(runtime, iov->iov_len))
return -EINVAL;
- frames = bytes_to_samples(runtime, to->iov->iov_len);
+ frames = bytes_to_samples(runtime, iov->iov_len);
bufs = kmalloc_array(to->nr_segs, sizeof(void *), GFP_KERNEL);
if (bufs == NULL)
return -ENOMEM;
- for (i = 0; i < to->nr_segs; ++i)
- bufs[i] = to->iov[i].iov_base;
+ for (i = 0; i < to->nr_segs; ++i) {
+ bufs[i] = iov->iov_base;
+ iov++;
+ }
result = snd_pcm_lib_readv(substream, bufs, frames);
if (result > 0)
result = frames_to_bytes(runtime, result);
@@ -3558,6 +3561,7 @@ static ssize_t snd_pcm_writev(struct kiocb *iocb, struct iov_iter *from)
unsigned long i;
void __user **bufs;
snd_pcm_uframes_t frames;
+ const struct iovec *iov = iter_iov(from);
pcm_file = iocb->ki_filp->private_data;
substream = pcm_file->substream;
@@ -3570,14 +3574,16 @@ static ssize_t snd_pcm_writev(struct kiocb *iocb, struct iov_iter *from)
if (!iter_is_iovec(from))
return -EINVAL;
if (from->nr_segs > 128 || from->nr_segs != runtime->channels ||
- !frame_aligned(runtime, from->iov->iov_len))
+ !frame_aligned(runtime, iov->iov_len))
return -EINVAL;
- frames = bytes_to_samples(runtime, from->iov->iov_len);
+ frames = bytes_to_samples(runtime, iov->iov_len);
bufs = kmalloc_array(from->nr_segs, sizeof(void *), GFP_KERNEL);
if (bufs == NULL)
return -ENOMEM;
- for (i = 0; i < from->nr_segs; ++i)
- bufs[i] = from->iov[i].iov_base;
+ for (i = 0; i < from->nr_segs; ++i) {
+ bufs[i] = iov->iov_base;
+ iov++;
+ }
result = snd_pcm_lib_writev(substream, bufs, frames);
if (result > 0)
result = frames_to_bytes(runtime, result);
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 03/11] iov_iter: add iter_iov_addr() and iter_iov_len() helpers
2023-03-29 18:40 [PATCHSET v6 0/11] Turn single segment imports into ITER_UBUF Jens Axboe
2023-03-29 18:40 ` [PATCH 01/11] block: ensure bio_alloc_map_data() deals with ITER_UBUF correctly Jens Axboe
2023-03-29 18:40 ` [PATCH 02/11] iov_iter: add iter_iovec() helper Jens Axboe
@ 2023-03-29 18:40 ` Jens Axboe
2023-03-29 18:40 ` [PATCH 04/11] iov_iter: remove iov_iter_iovec() Jens Axboe
` (8 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Jens Axboe @ 2023-03-29 18:40 UTC (permalink / raw)
To: linux-fsdevel; +Cc: torvalds, brauner, viro, Jens Axboe
These just return the address and length of the current iovec segment
in the iterator. Convert existing iov_iter_iovec() users to use them
instead of getting a copy of the current vec.
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
fs/read_write.c | 11 +++++------
include/linux/uio.h | 2 ++
io_uring/rw.c | 27 +++++++++++++--------------
mm/madvise.c | 9 ++++-----
4 files changed, 24 insertions(+), 25 deletions(-)
diff --git a/fs/read_write.c b/fs/read_write.c
index 7a2ff6157eda..a21ba3be7dbe 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -749,15 +749,14 @@ static ssize_t do_loop_readv_writev(struct file *filp, struct iov_iter *iter,
return -EOPNOTSUPP;
while (iov_iter_count(iter)) {
- struct iovec iovec = iov_iter_iovec(iter);
ssize_t nr;
if (type == READ) {
- nr = filp->f_op->read(filp, iovec.iov_base,
- iovec.iov_len, ppos);
+ nr = filp->f_op->read(filp, iter_iov_addr(iter),
+ iter_iov_len(iter), ppos);
} else {
- nr = filp->f_op->write(filp, iovec.iov_base,
- iovec.iov_len, ppos);
+ nr = filp->f_op->write(filp, iter_iov_addr(iter),
+ iter_iov_len(iter), ppos);
}
if (nr < 0) {
@@ -766,7 +765,7 @@ static ssize_t do_loop_readv_writev(struct file *filp, struct iov_iter *iter,
break;
}
ret += nr;
- if (nr != iovec.iov_len)
+ if (nr != iter_iov_len(iter))
break;
iov_iter_advance(iter, nr);
}
diff --git a/include/linux/uio.h b/include/linux/uio.h
index 4218624b7f78..b7fce87b720e 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -70,6 +70,8 @@ struct iov_iter {
};
#define iter_iov(iter) (iter)->__iov
+#define iter_iov_addr(iter) (iter_iov(iter)->iov_base + (iter)->iov_offset)
+#define iter_iov_len(iter) (iter_iov(iter)->iov_len - (iter)->iov_offset)
static inline enum iter_type iov_iter_type(const struct iov_iter *i)
{
diff --git a/io_uring/rw.c b/io_uring/rw.c
index 7573a34ea42a..f33ba6f28247 100644
--- a/io_uring/rw.c
+++ b/io_uring/rw.c
@@ -447,26 +447,25 @@ static ssize_t loop_rw_iter(int ddir, struct io_rw *rw, struct iov_iter *iter)
ppos = io_kiocb_ppos(kiocb);
while (iov_iter_count(iter)) {
- struct iovec iovec;
+ void __user *addr;
+ size_t len;
ssize_t nr;
if (iter_is_ubuf(iter)) {
- iovec.iov_base = iter->ubuf + iter->iov_offset;
- iovec.iov_len = iov_iter_count(iter);
+ addr = iter->ubuf + iter->iov_offset;
+ len = iov_iter_count(iter);
} else if (!iov_iter_is_bvec(iter)) {
- iovec = iov_iter_iovec(iter);
+ addr = iter_iov_addr(iter);
+ len = iter_iov_len(iter);
} else {
- iovec.iov_base = u64_to_user_ptr(rw->addr);
- iovec.iov_len = rw->len;
+ addr = u64_to_user_ptr(rw->addr);
+ len = rw->len;
}
- if (ddir == READ) {
- nr = file->f_op->read(file, iovec.iov_base,
- iovec.iov_len, ppos);
- } else {
- nr = file->f_op->write(file, iovec.iov_base,
- iovec.iov_len, ppos);
- }
+ if (ddir == READ)
+ nr = file->f_op->read(file, addr, len, ppos);
+ else
+ nr = file->f_op->write(file, addr, len, ppos);
if (nr < 0) {
if (!ret)
@@ -482,7 +481,7 @@ static ssize_t loop_rw_iter(int ddir, struct io_rw *rw, struct iov_iter *iter)
if (!rw->len)
break;
}
- if (nr != iovec.iov_len)
+ if (nr != len)
break;
}
diff --git a/mm/madvise.c b/mm/madvise.c
index 340125d08c03..9f389c5304d2 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -1456,7 +1456,7 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, const struct iovec __user *, vec,
size_t, vlen, int, behavior, unsigned int, flags)
{
ssize_t ret;
- struct iovec iovstack[UIO_FASTIOV], iovec;
+ struct iovec iovstack[UIO_FASTIOV];
struct iovec *iov = iovstack;
struct iov_iter iter;
struct task_struct *task;
@@ -1503,12 +1503,11 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, const struct iovec __user *, vec,
total_len = iov_iter_count(&iter);
while (iov_iter_count(&iter)) {
- iovec = iov_iter_iovec(&iter);
- ret = do_madvise(mm, (unsigned long)iovec.iov_base,
- iovec.iov_len, behavior);
+ ret = do_madvise(mm, (unsigned long)iter_iov_addr(&iter),
+ iter_iov_len(&iter), behavior);
if (ret < 0)
break;
- iov_iter_advance(&iter, iovec.iov_len);
+ iov_iter_advance(&iter, iter_iov_len(&iter));
}
ret = (total_len - iov_iter_count(&iter)) ? : ret;
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 04/11] iov_iter: remove iov_iter_iovec()
2023-03-29 18:40 [PATCHSET v6 0/11] Turn single segment imports into ITER_UBUF Jens Axboe
` (2 preceding siblings ...)
2023-03-29 18:40 ` [PATCH 03/11] iov_iter: add iter_iov_addr() and iter_iov_len() helpers Jens Axboe
@ 2023-03-29 18:40 ` Jens Axboe
2023-03-29 18:40 ` [PATCH 05/11] iov_iter: set nr_segs = 1 for ITER_UBUF Jens Axboe
` (7 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Jens Axboe @ 2023-03-29 18:40 UTC (permalink / raw)
To: linux-fsdevel; +Cc: torvalds, brauner, viro, Jens Axboe
No more users are left of this function.
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
include/linux/uio.h | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/include/linux/uio.h b/include/linux/uio.h
index b7fce87b720e..7f585ceedcb2 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -148,15 +148,6 @@ static inline size_t iov_length(const struct iovec *iov, unsigned long nr_segs)
return ret;
}
-static inline struct iovec iov_iter_iovec(const struct iov_iter *iter)
-{
- return (struct iovec) {
- .iov_base = iter_iov(iter)->iov_base + iter->iov_offset,
- .iov_len = min(iter->count,
- iter_iov(iter)->iov_len - iter->iov_offset),
- };
-}
-
size_t copy_page_from_iter_atomic(struct page *page, unsigned offset,
size_t bytes, struct iov_iter *i);
void iov_iter_advance(struct iov_iter *i, size_t bytes);
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 05/11] iov_iter: set nr_segs = 1 for ITER_UBUF
2023-03-29 18:40 [PATCHSET v6 0/11] Turn single segment imports into ITER_UBUF Jens Axboe
` (3 preceding siblings ...)
2023-03-29 18:40 ` [PATCH 04/11] iov_iter: remove iov_iter_iovec() Jens Axboe
@ 2023-03-29 18:40 ` Jens Axboe
2023-03-29 18:40 ` [PATCH 06/11] iov_iter: overlay struct iovec and ubuf/len Jens Axboe
` (6 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Jens Axboe @ 2023-03-29 18:40 UTC (permalink / raw)
To: linux-fsdevel; +Cc: torvalds, brauner, viro, Jens Axboe
To avoid needing to check if a given user backed iov_iter is of type
ITER_IOVEC or ITER_UBUF, set the number of segments for the ITER_UBUF
case to 1 as we're carrying a single segment.
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
include/linux/uio.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/uio.h b/include/linux/uio.h
index 7f585ceedcb2..5dbd2dcab35c 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -355,7 +355,8 @@ static inline void iov_iter_ubuf(struct iov_iter *i, unsigned int direction,
.user_backed = true,
.data_source = direction,
.ubuf = buf,
- .count = count
+ .count = count,
+ .nr_segs = 1
};
}
/* Flags for iov_iter_get/extract_pages*() */
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 06/11] iov_iter: overlay struct iovec and ubuf/len
2023-03-29 18:40 [PATCHSET v6 0/11] Turn single segment imports into ITER_UBUF Jens Axboe
` (4 preceding siblings ...)
2023-03-29 18:40 ` [PATCH 05/11] iov_iter: set nr_segs = 1 for ITER_UBUF Jens Axboe
@ 2023-03-29 18:40 ` Jens Axboe
2023-03-29 19:30 ` Linus Torvalds
2023-03-29 18:40 ` [PATCH 07/11] IB/hfi1: check for user backed iterator, not specific iterator type Jens Axboe
` (5 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Jens Axboe @ 2023-03-29 18:40 UTC (permalink / raw)
To: linux-fsdevel; +Cc: torvalds, brauner, viro, Jens Axboe
Add an internal struct iovec that we can return as a pointer, with the
fields of the iovec overlapping with the ITER_UBUF ubuf and length
fields.
Then we can have iter_iov() check for the appropriate type, and return
&iter->__ubuf_iovec for ITER_UBUF and iter->__iov for ITER_IOVEC and
things will magically work out for a single segment request regardless
of either type.
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
include/linux/uio.h | 39 ++++++++++++++++++++++++++++++---------
1 file changed, 30 insertions(+), 9 deletions(-)
diff --git a/include/linux/uio.h b/include/linux/uio.h
index 5dbd2dcab35c..361688b86291 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -49,15 +49,30 @@ struct iov_iter {
size_t iov_offset;
int last_offset;
};
- size_t count;
+ /*
+ * Hack alert: overlay ubuf_iovec with iovec + count, so
+ * that the members resolve correctly regardless of the type
+ * of iterator used. This means that you can use:
+ *
+ * &iter->ubuf or iter->iov
+ *
+ * interchangably for the user_backed cases, hence simplifying
+ * some of the cases that need to deal with both.
+ */
union {
- /* use iter_iov() to get the current vec */
- const struct iovec *__iov;
- const struct kvec *kvec;
- const struct bio_vec *bvec;
- struct xarray *xarray;
- struct pipe_inode_info *pipe;
- void __user *ubuf;
+ struct iovec __ubuf_iovec;
+ struct {
+ union {
+ /* use iter_iov() to get the current vec */
+ const struct iovec *__iov;
+ const struct kvec *kvec;
+ const struct bio_vec *bvec;
+ struct xarray *xarray;
+ struct pipe_inode_info *pipe;
+ void __user *ubuf;
+ };
+ size_t count;
+ };
};
union {
unsigned long nr_segs;
@@ -69,7 +84,13 @@ struct iov_iter {
};
};
-#define iter_iov(iter) (iter)->__iov
+static inline const struct iovec *iter_iov(const struct iov_iter *iter)
+{
+ if (iter->iter_type == ITER_UBUF)
+ return (const struct iovec *) &iter->__ubuf_iovec;
+ return iter->__iov;
+}
+
#define iter_iov_addr(iter) (iter_iov(iter)->iov_base + (iter)->iov_offset)
#define iter_iov_len(iter) (iter_iov(iter)->iov_len - (iter)->iov_offset)
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 06/11] iov_iter: overlay struct iovec and ubuf/len
2023-03-29 18:40 ` [PATCH 06/11] iov_iter: overlay struct iovec and ubuf/len Jens Axboe
@ 2023-03-29 19:30 ` Linus Torvalds
2023-03-29 19:38 ` Jens Axboe
0 siblings, 1 reply; 23+ messages in thread
From: Linus Torvalds @ 2023-03-29 19:30 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-fsdevel, brauner, viro
On Wed, Mar 29, 2023 at 11:41 AM Jens Axboe <axboe@kernel.dk> wrote:
>
> + struct iovec __ubuf_iovec;
I think this is the third time I say this: this should be "const struct iovec".
No other use is ever valid, and this cast:
> +static inline const struct iovec *iter_iov(const struct iov_iter *iter)
> +{
> + if (iter->iter_type == ITER_UBUF)
> + return (const struct iovec *) &iter->__ubuf_iovec;
should simply not exist.
The first rule of cast club is that casting one pointer to another is
generally a sign that something is wrong.
Casting a pointer to an integer? That's a valid way to get at the bit
representation for things like tagged pointers or for virtual address
arithmetic etc (ok, and by "valid" I mean "valid in kernel contexts -
not necessarily in all other contexts).
Casting an integer to a pointer? Same thing - some things just need to
do bit operations on what will become a pointer (allocators etc).
But casting a pointer to another one - that should basically always
raise eyebrows. You should try really hard to avoid it.
Yes, we do it in the kernel, and yes, it *can* be valid. But most of
the time it's really a bad sign.
Linus
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 06/11] iov_iter: overlay struct iovec and ubuf/len
2023-03-29 19:30 ` Linus Torvalds
@ 2023-03-29 19:38 ` Jens Axboe
2023-03-29 19:42 ` Jens Axboe
0 siblings, 1 reply; 23+ messages in thread
From: Jens Axboe @ 2023-03-29 19:38 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-fsdevel, brauner, viro
On 3/29/23 1:30 PM, Linus Torvalds wrote:
> On Wed, Mar 29, 2023 at 11:41 AM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> + struct iovec __ubuf_iovec;
>
> I think this is the third time I say this: this should be "const struct iovec".
Doh sorry, not sure why I keep missing that... But yes, it should, I'll make
the edit and actually amend it.
> No other use is ever valid, and this cast:
>
>> +static inline const struct iovec *iter_iov(const struct iov_iter *iter)
>> +{
>> + if (iter->iter_type == ITER_UBUF)
>> + return (const struct iovec *) &iter->__ubuf_iovec;
>
> should simply not exist.
Yep. Fixed both up.
--
Jens Axboe
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 06/11] iov_iter: overlay struct iovec and ubuf/len
2023-03-29 19:38 ` Jens Axboe
@ 2023-03-29 19:42 ` Jens Axboe
2023-03-29 19:49 ` Jens Axboe
0 siblings, 1 reply; 23+ messages in thread
From: Jens Axboe @ 2023-03-29 19:42 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-fsdevel, brauner, viro
On 3/29/23 1:38 PM, Jens Axboe wrote:
> On 3/29/23 1:30 PM, Linus Torvalds wrote:
>> On Wed, Mar 29, 2023 at 11:41 AM Jens Axboe <axboe@kernel.dk> wrote:
>>>
>>> + struct iovec __ubuf_iovec;
>>
>> I think this is the third time I say this: this should be "const struct iovec".
>
> Doh sorry, not sure why I keep missing that... But yes, it should, I'll make
> the edit and actually amend it.
Now I recall why that ended up like that again, during the initial fiddling
with this. If we leave it const, we get:
CC arch/arm64/kernel/asm-offsets.s
In file included from ./include/linux/socket.h:8,
from ./include/linux/compat.h:15,
from ./arch/arm64/include/asm/ftrace.h:52,
from ./include/linux/ftrace.h:23,
from arch/arm64/kernel/asm-offsets.c:12:
./include/linux/uio.h: In function ‘iov_iter_ubuf’:
./include/linux/uio.h:374:12: error: assignment of read-only location ‘*i’
374 | *i = (struct iov_iter) {
| ^
make[1]: *** [scripts/Makefile.build:114: arch/arm64/kernel/asm-offsets.s] Error 1
make: *** [Makefile:1286: prepare0] Error 2
Let me take a closer look at that...
--
Jens Axboe
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 06/11] iov_iter: overlay struct iovec and ubuf/len
2023-03-29 19:42 ` Jens Axboe
@ 2023-03-29 19:49 ` Jens Axboe
2023-03-29 19:52 ` Linus Torvalds
0 siblings, 1 reply; 23+ messages in thread
From: Jens Axboe @ 2023-03-29 19:49 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-fsdevel, brauner, viro
On 3/29/23 1:42 PM, Jens Axboe wrote:
> On 3/29/23 1:38 PM, Jens Axboe wrote:
>> On 3/29/23 1:30 PM, Linus Torvalds wrote:
>>> On Wed, Mar 29, 2023 at 11:41 AM Jens Axboe <axboe@kernel.dk> wrote:
>>>>
>>>> + struct iovec __ubuf_iovec;
>>>
>>> I think this is the third time I say this: this should be "const struct iovec".
>>
>> Doh sorry, not sure why I keep missing that... But yes, it should, I'll make
>> the edit and actually amend it.
>
> Now I recall why that ended up like that again, during the initial fiddling
> with this. If we leave it const, we get:
>
> CC arch/arm64/kernel/asm-offsets.s
> In file included from ./include/linux/socket.h:8,
> from ./include/linux/compat.h:15,
> from ./arch/arm64/include/asm/ftrace.h:52,
> from ./include/linux/ftrace.h:23,
> from arch/arm64/kernel/asm-offsets.c:12:
> ./include/linux/uio.h: In function ‘iov_iter_ubuf’:
> ./include/linux/uio.h:374:12: error: assignment of read-only location ‘*i’
> 374 | *i = (struct iov_iter) {
> | ^
> make[1]: *** [scripts/Makefile.build:114: arch/arm64/kernel/asm-offsets.s] Error 1
> make: *** [Makefile:1286: prepare0] Error 2
>
> Let me take a closer look at that...
We can get rid of these if we convert the iov_iter initializers to
just assign the members rather than the copy+zero fill. The automatic
zero fill is nice though, in terms of sanity.
--
Jens Axboe
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 06/11] iov_iter: overlay struct iovec and ubuf/len
2023-03-29 19:49 ` Jens Axboe
@ 2023-03-29 19:52 ` Linus Torvalds
2023-03-29 19:56 ` Jens Axboe
0 siblings, 1 reply; 23+ messages in thread
From: Linus Torvalds @ 2023-03-29 19:52 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-fsdevel, brauner, viro
On Wed, Mar 29, 2023 at 12:49 PM Jens Axboe <axboe@kernel.dk> wrote:
>
> We can get rid of these if we convert the iov_iter initializers to
> just assign the members rather than the copy+zero fill. The automatic
> zero fill is nice though, in terms of sanity.
The automatic zero fill is good, but I think it should be fixed by
just not making that
const struct iovec __ubuf_iovec;
member be the first member of a union.
The way union initializers work is that if they aren't named, they are
for the first member.
So I *think* the reason you get that warning is literally just because
the __ubuf_iovec member is first in that union, and moving it down to
below the other struct will just fix things.
Linus
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 06/11] iov_iter: overlay struct iovec and ubuf/len
2023-03-29 19:52 ` Linus Torvalds
@ 2023-03-29 19:56 ` Jens Axboe
2023-03-29 19:59 ` Linus Torvalds
0 siblings, 1 reply; 23+ messages in thread
From: Jens Axboe @ 2023-03-29 19:56 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-fsdevel, brauner, viro
On 3/29/23 1:52 PM, Linus Torvalds wrote:
> On Wed, Mar 29, 2023 at 12:49 PM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> We can get rid of these if we convert the iov_iter initializers to
>> just assign the members rather than the copy+zero fill. The automatic
>> zero fill is nice though, in terms of sanity.
>
> The automatic zero fill is good, but I think it should be fixed by
> just not making that
>
> const struct iovec __ubuf_iovec;
>
> member be the first member of a union.
>
> The way union initializers work is that if they aren't named, they are
> for the first member.
>
> So I *think* the reason you get that warning is literally just because
> the __ubuf_iovec member is first in that union, and moving it down to
> below the other struct will just fix things.
Nope, still fails with it moved below.
--
Jens Axboe
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 06/11] iov_iter: overlay struct iovec and ubuf/len
2023-03-29 19:56 ` Jens Axboe
@ 2023-03-29 19:59 ` Linus Torvalds
2023-03-29 20:01 ` Jens Axboe
0 siblings, 1 reply; 23+ messages in thread
From: Linus Torvalds @ 2023-03-29 19:59 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-fsdevel, brauner, viro
On Wed, Mar 29, 2023 at 12:56 PM Jens Axboe <axboe@kernel.dk> wrote:
>
> Nope, still fails with it moved below.
Ouch. I guess the 'const' cast may be the only way then. It sounds
like gcc may warn whenever it sees an assignment to any structure that
has a const member, rather than when it sees an assignment to that
particular member.
Sad.
Linus
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 06/11] iov_iter: overlay struct iovec and ubuf/len
2023-03-29 19:59 ` Linus Torvalds
@ 2023-03-29 20:01 ` Jens Axboe
0 siblings, 0 replies; 23+ messages in thread
From: Jens Axboe @ 2023-03-29 20:01 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-fsdevel, brauner, viro
On 3/29/23 1:59 PM, Linus Torvalds wrote:
> On Wed, Mar 29, 2023 at 12:56 PM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> Nope, still fails with it moved below.
>
> Ouch. I guess the 'const' cast may be the only way then. It sounds
> like gcc may warn whenever it sees an assignment to any structure that
> has a const member, rather than when it sees an assignment to that
> particular member.
>
> Sad.
Yeah, I tried a bunch of variants to trick it, including having
it in a union with another non-const iovec first. But it cannot be
tricked, so I think we're stuck with that. I'll add a comment.
--
Jens Axboe
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 07/11] IB/hfi1: check for user backed iterator, not specific iterator type
2023-03-29 18:40 [PATCHSET v6 0/11] Turn single segment imports into ITER_UBUF Jens Axboe
` (5 preceding siblings ...)
2023-03-29 18:40 ` [PATCH 06/11] iov_iter: overlay struct iovec and ubuf/len Jens Axboe
@ 2023-03-29 18:40 ` Jens Axboe
2023-03-29 18:40 ` [PATCH 08/11] IB/qib: " Jens Axboe
` (4 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Jens Axboe @ 2023-03-29 18:40 UTC (permalink / raw)
To: linux-fsdevel; +Cc: torvalds, brauner, viro, Jens Axboe
In preparation for switching single segment iterators to using ITER_UBUF,
swap the check for whether we are user backed or not. While at it, move
it outside the srcu locking area to clean up the code a bit.
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
drivers/infiniband/hw/hfi1/file_ops.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/infiniband/hw/hfi1/file_ops.c b/drivers/infiniband/hw/hfi1/file_ops.c
index 3065db9d6bb9..f3d6ce45c397 100644
--- a/drivers/infiniband/hw/hfi1/file_ops.c
+++ b/drivers/infiniband/hw/hfi1/file_ops.c
@@ -267,6 +267,8 @@ 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)
+ return -EINVAL;
idx = srcu_read_lock(&fd->pq_srcu);
pq = srcu_dereference(fd->pq, &fd->pq_srcu);
if (!cq || !pq) {
@@ -274,11 +276,6 @@ static ssize_t hfi1_write_iter(struct kiocb *kiocb, struct iov_iter *from)
return -EIO;
}
- if (!iter_is_iovec(from) || !dim) {
- srcu_read_unlock(&fd->pq_srcu, idx);
- return -EINVAL;
- }
-
trace_hfi1_sdma_request(fd->dd, fd->uctxt->ctxt, fd->subctxt, dim);
if (atomic_read(&pq->n_reqs) == pq->n_max_reqs) {
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 08/11] IB/qib: check for user backed iterator, not specific iterator type
2023-03-29 18:40 [PATCHSET v6 0/11] Turn single segment imports into ITER_UBUF Jens Axboe
` (6 preceding siblings ...)
2023-03-29 18:40 ` [PATCH 07/11] IB/hfi1: check for user backed iterator, not specific iterator type Jens Axboe
@ 2023-03-29 18:40 ` Jens Axboe
2023-03-29 18:40 ` [PATCH 09/11] ALSA: pcm: " Jens Axboe
` (3 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Jens Axboe @ 2023-03-29 18:40 UTC (permalink / raw)
To: linux-fsdevel; +Cc: torvalds, brauner, viro, Jens Axboe
In preparation for switching single segment iterators to using ITER_UBUF,
swap the check for whether we are user backed or not.
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
drivers/infiniband/hw/qib/qib_file_ops.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/qib/qib_file_ops.c b/drivers/infiniband/hw/qib/qib_file_ops.c
index 4cee39337866..815ea72ad473 100644
--- a/drivers/infiniband/hw/qib/qib_file_ops.c
+++ b/drivers/infiniband/hw/qib/qib_file_ops.c
@@ -2245,7 +2245,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 (!iter_is_iovec(from) || !from->nr_segs || !pq)
+ if (!from->user_backed || !from->nr_segs || !pq)
return -EINVAL;
return qib_user_sdma_writev(rcd, pq, iter_iov(from), from->nr_segs);
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 09/11] ALSA: pcm: check for user backed iterator, not specific iterator type
2023-03-29 18:40 [PATCHSET v6 0/11] Turn single segment imports into ITER_UBUF Jens Axboe
` (7 preceding siblings ...)
2023-03-29 18:40 ` [PATCH 08/11] IB/qib: " Jens Axboe
@ 2023-03-29 18:40 ` Jens Axboe
2023-03-29 18:40 ` [PATCH 10/11] iov_iter: convert import_single_range() to ITER_UBUF Jens Axboe
` (2 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Jens Axboe @ 2023-03-29 18:40 UTC (permalink / raw)
To: linux-fsdevel; +Cc: torvalds, brauner, viro, Jens Axboe
In preparation for switching single segment iterators to using ITER_UBUF,
swap the check for whether we are user backed or not.
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
sound/core/pcm_native.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 8bb97ee6720d..5868661d461b 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -3531,7 +3531,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 (!iter_is_iovec(to))
+ if (!to->user_backed)
return -EINVAL;
if (to->nr_segs > 1024 || to->nr_segs != runtime->channels)
return -EINVAL;
@@ -3571,7 +3571,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 (!iter_is_iovec(from))
+ if (!from->user_backed)
return -EINVAL;
if (from->nr_segs > 128 || from->nr_segs != runtime->channels ||
!frame_aligned(runtime, iov->iov_len))
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 10/11] iov_iter: convert import_single_range() to ITER_UBUF
2023-03-29 18:40 [PATCHSET v6 0/11] Turn single segment imports into ITER_UBUF Jens Axboe
` (8 preceding siblings ...)
2023-03-29 18:40 ` [PATCH 09/11] ALSA: pcm: " Jens Axboe
@ 2023-03-29 18:40 ` Jens Axboe
2023-03-29 18:40 ` [PATCH 11/11] iov_iter: import single vector iovecs as ITER_UBUF Jens Axboe
2023-03-29 19:44 ` [PATCHSET v6 0/11] Turn single segment imports into ITER_UBUF Linus Torvalds
11 siblings, 0 replies; 23+ messages in thread
From: Jens Axboe @ 2023-03-29 18:40 UTC (permalink / raw)
To: linux-fsdevel; +Cc: torvalds, brauner, viro, Jens Axboe
Since we're just importing a single vector, we don't have to turn it
into an ITER_IOVEC. Instead turn it into an ITER_UBUF, which is cheaper
to iterate.
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
lib/iov_iter.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 87488c4aad3f..f411bda1171f 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -1870,9 +1870,7 @@ int import_single_range(int rw, void __user *buf, size_t len,
if (unlikely(!access_ok(buf, len)))
return -EFAULT;
- iov->iov_base = buf;
- iov->iov_len = len;
- iov_iter_init(i, rw, iov, 1, len);
+ iov_iter_ubuf(i, rw, buf, len);
return 0;
}
EXPORT_SYMBOL(import_single_range);
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 11/11] iov_iter: import single vector iovecs as ITER_UBUF
2023-03-29 18:40 [PATCHSET v6 0/11] Turn single segment imports into ITER_UBUF Jens Axboe
` (9 preceding siblings ...)
2023-03-29 18:40 ` [PATCH 10/11] iov_iter: convert import_single_range() to ITER_UBUF Jens Axboe
@ 2023-03-29 18:40 ` Jens Axboe
2023-03-29 19:44 ` [PATCHSET v6 0/11] Turn single segment imports into ITER_UBUF Linus Torvalds
11 siblings, 0 replies; 23+ messages in thread
From: Jens Axboe @ 2023-03-29 18:40 UTC (permalink / raw)
To: linux-fsdevel; +Cc: torvalds, brauner, viro, Jens Axboe
Add a special case to __import_iovec(), which imports a single segment
iovec as an ITER_UBUF rather than an ITER_IOVEC. ITER_UBUF is cheaper
to iterate than ITER_IOVEC, and for a single segment iovec, there's no
point in using a segmented iterator.
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
lib/iov_iter.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index f411bda1171f..3e6c9bcfa612 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -1784,6 +1784,30 @@ struct iovec *iovec_from_user(const struct iovec __user *uvec,
return iov;
}
+/*
+ * Single segment iovec supplied by the user, import it as ITER_UBUF.
+ */
+static ssize_t __import_iovec_ubuf(int type, const struct iovec __user *uvec,
+ struct iovec **iovp, struct iov_iter *i,
+ bool compat)
+{
+ struct iovec *iov = *iovp;
+ ssize_t ret;
+
+ if (compat)
+ ret = copy_compat_iovec_from_user(iov, uvec, 1);
+ else
+ ret = copy_iovec_from_user(iov, uvec, 1);
+ if (unlikely(ret))
+ return ret;
+
+ ret = import_ubuf(type, iov->iov_base, iov->iov_len, i);
+ if (unlikely(ret))
+ return ret;
+ *iovp = NULL;
+ return i->count;
+}
+
ssize_t __import_iovec(int type, const struct iovec __user *uvec,
unsigned nr_segs, unsigned fast_segs, struct iovec **iovp,
struct iov_iter *i, bool compat)
@@ -1792,6 +1816,9 @@ ssize_t __import_iovec(int type, const struct iovec __user *uvec,
unsigned long seg;
struct iovec *iov;
+ if (nr_segs == 1)
+ return __import_iovec_ubuf(type, uvec, iovp, i, compat);
+
iov = iovec_from_user(uvec, nr_segs, fast_segs, *iovp, compat);
if (IS_ERR(iov)) {
*iovp = NULL;
--
2.39.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCHSET v6 0/11] Turn single segment imports into ITER_UBUF
2023-03-29 18:40 [PATCHSET v6 0/11] Turn single segment imports into ITER_UBUF Jens Axboe
` (10 preceding siblings ...)
2023-03-29 18:40 ` [PATCH 11/11] iov_iter: import single vector iovecs as ITER_UBUF Jens Axboe
@ 2023-03-29 19:44 ` Linus Torvalds
2023-03-29 19:55 ` Jens Axboe
11 siblings, 1 reply; 23+ messages in thread
From: Linus Torvalds @ 2023-03-29 19:44 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-fsdevel, brauner, viro
On Wed, Mar 29, 2023 at 11:41 AM Jens Axboe <axboe@kernel.dk> wrote:
>
> Passes testing, and verified we do the right thing for 1 and multi
> segments.
Apart from the pointer casting rant, this looks sane to me.
I feel like 02/11 has a few potential cleanups:
(a) it feels like a few too many "iter.__iov" uses remaining, but
they mostly (all?) look like assignments.
I do get the feeling that any time you assign __iov, you should also
assign "nr_segs", and it worries me a bit that I see one without the
other. Maybe room for another helper that enforces a "if you set the
__iov pointer, you must be setting nr_segs too"?
And maybe I'm just being difficult.
(b) I see at least one "iov = iter_iov(from)" that precedes a later
check for "iter_is_iovec()", which again means that *if* we add some
debug sanity test to "iter_iov()", it might trigger when it shouldn't?
The one I see is in snd_pcm_writev(), but I th ink the same thing
happens in snd_pcm_readv() but just isn't visible in the patch due to
not having the context lines.
Linus
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHSET v6 0/11] Turn single segment imports into ITER_UBUF
2023-03-29 19:44 ` [PATCHSET v6 0/11] Turn single segment imports into ITER_UBUF Linus Torvalds
@ 2023-03-29 19:55 ` Jens Axboe
0 siblings, 0 replies; 23+ messages in thread
From: Jens Axboe @ 2023-03-29 19:55 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-fsdevel, brauner, viro
On 3/29/23 1:44 PM, Linus Torvalds wrote:
> On Wed, Mar 29, 2023 at 11:41 AM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> Passes testing, and verified we do the right thing for 1 and multi
>> segments.
>
> Apart from the pointer casting rant, this looks sane to me.
>
> I feel like 02/11 has a few potential cleanups:
>
> (a) it feels like a few too many "iter.__iov" uses remaining, but
> they mostly (all?) look like assignments.
>
> I do get the feeling that any time you assign __iov, you should also
> assign "nr_segs", and it worries me a bit that I see one without the
> other. Maybe room for another helper that enforces a "if you set the
> __iov pointer, you must be setting nr_segs too"?
>
> And maybe I'm just being difficult.
No, I think that's valid, and the cover letter does touch upon that.
The thought of doing an iov assign helper has occurred to me as well.
I just wanted to get general feelings on the direction first, then
do a round of polish when prudent rather than prematurely.
> (b) I see at least one "iov = iter_iov(from)" that precedes a later
> check for "iter_is_iovec()", which again means that *if* we add some
> debug sanity test to "iter_iov()", it might trigger when it shouldn't?
>
> The one I see is in snd_pcm_writev(), but I th ink the same thing
> happens in snd_pcm_readv() but just isn't visible in the patch due to
> not having the context lines.
I think that's mostly a patch ordering issue. Should probably just
push the sound and IB patches to the front of the series.
--
Jens Axboe
^ permalink raw reply [flat|nested] 23+ messages in thread