* [PATCH 03/19] block: add bio_set_polled() helper
From: Jens Axboe @ 2019-02-09 21:13 UTC (permalink / raw)
To: linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro, Jens Axboe
In-Reply-To: <20190209211346.26060-1-axboe@kernel.dk>
For the upcoming async polled IO, we can't sleep allocating requests.
If we do, then we introduce a deadlock where the submitter already
has async polled IO in-flight, but can't wait for them to complete
since polled requests must be active found and reaped.
Utilize the helper in the blockdev DIRECT_IO code.
Reviewed-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
fs/block_dev.c | 4 ++--
include/linux/bio.h | 14 ++++++++++++++
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index f18d076a2596..392e2bfb636f 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -247,7 +247,7 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct iov_iter *iter,
task_io_account_write(ret);
}
if (iocb->ki_flags & IOCB_HIPRI)
- bio.bi_opf |= REQ_HIPRI;
+ bio_set_polled(&bio, iocb);
qc = submit_bio(&bio);
for (;;) {
@@ -415,7 +415,7 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages)
nr_pages = iov_iter_npages(iter, BIO_MAX_PAGES);
if (!nr_pages) {
if (iocb->ki_flags & IOCB_HIPRI)
- bio->bi_opf |= REQ_HIPRI;
+ bio_set_polled(bio, iocb);
qc = submit_bio(bio);
WRITE_ONCE(iocb->ki_cookie, qc);
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 7380b094dcca..f6f0a2b3cbc8 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -823,5 +823,19 @@ static inline int bio_integrity_add_page(struct bio *bio, struct page *page,
#endif /* CONFIG_BLK_DEV_INTEGRITY */
+/*
+ * Mark a bio as polled. Note that for async polled IO, the caller must
+ * expect -EWOULDBLOCK if we cannot allocate a request (or other resources).
+ * We cannot block waiting for requests on polled IO, as those completions
+ * must be found by the caller. This is different than IRQ driven IO, where
+ * it's safe to wait for IO to complete.
+ */
+static inline void bio_set_polled(struct bio *bio, struct kiocb *kiocb)
+{
+ bio->bi_opf |= REQ_HIPRI;
+ if (!is_sync_kiocb(kiocb))
+ bio->bi_opf |= REQ_NOWAIT;
+}
+
#endif /* CONFIG_BLOCK */
#endif /* __LINUX_BIO_H */
--
2.17.1
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* [PATCH 02/19] block: wire up block device iopoll method
From: Jens Axboe @ 2019-02-09 21:13 UTC (permalink / raw)
To: linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro, Jens Axboe
In-Reply-To: <20190209211346.26060-1-axboe@kernel.dk>
From: Christoph Hellwig <hch@lst.de>
Just call blk_poll on the iocb cookie, we can derive the block device
from the inode trivially.
Reviewed-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
fs/block_dev.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 58a4c1217fa8..f18d076a2596 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -293,6 +293,14 @@ struct blkdev_dio {
static struct bio_set blkdev_dio_pool;
+static int blkdev_iopoll(struct kiocb *kiocb, bool wait)
+{
+ struct block_device *bdev = I_BDEV(kiocb->ki_filp->f_mapping->host);
+ struct request_queue *q = bdev_get_queue(bdev);
+
+ return blk_poll(q, READ_ONCE(kiocb->ki_cookie), wait);
+}
+
static void blkdev_bio_end_io(struct bio *bio)
{
struct blkdev_dio *dio = bio->bi_private;
@@ -410,6 +418,7 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages)
bio->bi_opf |= REQ_HIPRI;
qc = submit_bio(bio);
+ WRITE_ONCE(iocb->ki_cookie, qc);
break;
}
@@ -2076,6 +2085,7 @@ const struct file_operations def_blk_fops = {
.llseek = block_llseek,
.read_iter = blkdev_read_iter,
.write_iter = blkdev_write_iter,
+ .iopoll = blkdev_iopoll,
.mmap = generic_file_mmap,
.fsync = blkdev_fsync,
.unlocked_ioctl = block_ioctl,
--
2.17.1
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* [PATCH 01/19] fs: add an iopoll method to struct file_operations
From: Jens Axboe @ 2019-02-09 21:13 UTC (permalink / raw)
To: linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro, Jens Axboe
In-Reply-To: <20190209211346.26060-1-axboe@kernel.dk>
From: Christoph Hellwig <hch@lst.de>
This new methods is used to explicitly poll for I/O completion for an
iocb. It must be called for any iocb submitted asynchronously (that
is with a non-null ki_complete) which has the IOCB_HIPRI flag set.
The method is assisted by a new ki_cookie field in struct iocb to store
the polling cookie.
Reviewed-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
Documentation/filesystems/vfs.txt | 3 +++
include/linux/fs.h | 2 ++
2 files changed, 5 insertions(+)
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index 8dc8e9c2913f..761c6fd24a53 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -857,6 +857,7 @@ struct file_operations {
ssize_t (*write) (struct file *, const char __user *, size_t, loff_t *);
ssize_t (*read_iter) (struct kiocb *, struct iov_iter *);
ssize_t (*write_iter) (struct kiocb *, struct iov_iter *);
+ int (*iopoll)(struct kiocb *kiocb, bool spin);
int (*iterate) (struct file *, struct dir_context *);
int (*iterate_shared) (struct file *, struct dir_context *);
__poll_t (*poll) (struct file *, struct poll_table_struct *);
@@ -902,6 +903,8 @@ otherwise noted.
write_iter: possibly asynchronous write with iov_iter as source
+ iopoll: called when aio wants to poll for completions on HIPRI iocbs
+
iterate: called when the VFS needs to read the directory contents
iterate_shared: called when the VFS needs to read the directory contents
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 29d8e2cfed0e..dedcc2e9265c 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -310,6 +310,7 @@ struct kiocb {
int ki_flags;
u16 ki_hint;
u16 ki_ioprio; /* See linux/ioprio.h */
+ unsigned int ki_cookie; /* for ->iopoll */
} __randomize_layout;
static inline bool is_sync_kiocb(struct kiocb *kiocb)
@@ -1787,6 +1788,7 @@ struct file_operations {
ssize_t (*write) (struct file *, const char __user *, size_t, loff_t *);
ssize_t (*read_iter) (struct kiocb *, struct iov_iter *);
ssize_t (*write_iter) (struct kiocb *, struct iov_iter *);
+ int (*iopoll)(struct kiocb *kiocb, bool spin);
int (*iterate) (struct file *, struct dir_context *);
int (*iterate_shared) (struct file *, struct dir_context *);
__poll_t (*poll) (struct file *, struct poll_table_struct *);
--
2.17.1
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* [PATCHSET v14] io_uring IO interface
From: Jens Axboe @ 2019-02-09 21:13 UTC (permalink / raw)
To: linux-aio, linux-block, linux-api; +Cc: hch, jmoyer, avi, jannh, viro
Another day, another spin. This is v14, and it fixes a few silly
issues with fsync, mainly, but also various little fixes and tweaks
all over the map.
The liburing git repo has a full set of man pages for this, though they
could probably still use a bit of polish. I'd also like to see a
io_uring(7) man page to describe the overall design of the project,
expect that in the not-so-distant future. You can clone that here:
git://git.kernel.dk/liburing
Patches are against 5.0-rc5, and can also be found in my io_uring branch
here:
git://git.kernel.dk/linux-block io_uring
Changes since v13:
- Use READ/WRITE_ONCE anywhere we touch the shared data
- Cache if we need to account/unaccount memory
- Fix fsync file grabbing
- Fix oops with fsync and deferred async list
- Fix final fput() of fixed files AFTER adding to socket
- Use correct iterator to free fixed user bufs
- up/down_read() for mmap_sem when mapping buffers
- completion -> complete in Kconfig help entry
- Ensure malicious app can't trick NOP into leaking a file
- Add a few comments, address a few review comments
Documentation/filesystems/vfs.txt | 3 +
arch/x86/entry/syscalls/syscall_32.tbl | 3 +
arch/x86/entry/syscalls/syscall_64.tbl | 3 +
block/bio.c | 59 +-
fs/Makefile | 1 +
fs/block_dev.c | 19 +-
fs/file.c | 15 +-
fs/file_table.c | 9 +-
fs/gfs2/file.c | 2 +
fs/io_uring.c | 2853 ++++++++++++++++++++++++
fs/iomap.c | 48 +-
fs/xfs/xfs_file.c | 1 +
include/linux/bio.h | 14 +
include/linux/blk_types.h | 1 +
include/linux/file.h | 2 +
include/linux/fs.h | 15 +-
include/linux/iomap.h | 1 +
include/linux/sched/user.h | 2 +-
include/linux/syscalls.h | 8 +
include/net/af_unix.h | 1 +
include/uapi/asm-generic/unistd.h | 8 +-
include/uapi/linux/io_uring.h | 142 ++
init/Kconfig | 9 +
kernel/sys_ni.c | 3 +
net/Makefile | 2 +-
net/unix/Kconfig | 5 +
net/unix/Makefile | 2 +
net/unix/af_unix.c | 63 +-
net/unix/garbage.c | 68 +-
net/unix/scm.c | 151 ++
net/unix/scm.h | 10 +
31 files changed, 3354 insertions(+), 169 deletions(-)
--
Jens Axboe
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 12/19] io_uring: add support for pre-mapped user IO buffers
From: Jens Axboe @ 2019-02-09 16:50 UTC (permalink / raw)
To: Jann Horn
Cc: linux-aio, linux-block, Linux API, hch, jmoyer, Avi Kivity,
Al Viro
In-Reply-To: <2ef04b12-6a4b-af25-cbf8-9dde79b0ec1e@kernel.dk>
On 2/8/19 4:38 PM, Jens Axboe wrote:
>> Technically, this accounting is probably a bit off; I think if you
>> pass in a vector of 4K areas from 1G hugepages, you're going to pin
>> factor 0x40000 more memory than you think you're pinning.
>> (get_user_pages() counts references against the head page of a
>> compound page; nothing in the kernel can tell afterwards which part of
>> the hugepage you're using.) I'm not sure how much of a problem that
>> is, but it should probably at least be documented. Unless I'm just
>> missing something?
>
> No I think you are right, it doesn't account for the hugepage size if
> you pass in huge pages. I'll fix that up.
I made a note of it in the man page, I don't think this is unreasonable
(or unexpected) behavior.
--
Jens Axboe
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 15/19] io_uring: add submission polling
From: Hannes Reinecke @ 2019-02-09 9:53 UTC (permalink / raw)
To: Jens Axboe, linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro
In-Reply-To: <20190208173423.27014-16-axboe@kernel.dk>
On 2/8/19 6:34 PM, Jens Axboe wrote:
> This enables an application to do IO, without ever entering the kernel.
> By using the SQ ring to fill in new sqes and watching for completions
> on the CQ ring, we can submit and reap IOs without doing a single system
> call. The kernel side thread will poll for new submissions, and in case
> of HIPRI/polled IO, it'll also poll for completions.
>
> By default, we allow 1 second of active spinning. This can by changed
> by passing in a different grace period at io_uring_register(2) time.
> If the thread exceeds this idle time without having any work to do, it
> will set:
>
> sq_ring->flags |= IORING_SQ_NEED_WAKEUP.
>
> The application will have to call io_uring_enter() to start things back
> up again. If IO is kept busy, that will never be needed. Basically an
> application that has this feature enabled will guard it's
> io_uring_enter(2) call with:
>
> read_barrier();
> if (*sq_ring->flags & IORING_SQ_NEED_WAKEUP)
> io_uring_enter(fd, 0, 0, IORING_ENTER_SQ_WAKEUP);
>
> instead of calling it unconditionally.
>
> It's mandatory to use fixed files with this feature. Failure to do so
> will result in the application getting an -EBADF CQ entry when
> submitting IO.
>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
> fs/io_uring.c | 249 +++++++++++++++++++++++++++++++++-
> include/uapi/linux/io_uring.h | 12 +-
> 2 files changed, 253 insertions(+), 8 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 14/19] io_uring: add file set registration
From: Hannes Reinecke @ 2019-02-09 9:50 UTC (permalink / raw)
To: Jens Axboe, linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro
In-Reply-To: <20190208173423.27014-15-axboe@kernel.dk>
On 2/8/19 6:34 PM, Jens Axboe wrote:
> We normally have to fget/fput for each IO we do on a file. Even with
> the batching we do, the cost of the atomic inc/dec of the file usage
> count adds up.
>
> This adds IORING_REGISTER_FILES, and IORING_UNREGISTER_FILES opcodes
> for the io_uring_register(2) system call. The arguments passed in must
> be an array of __s32 holding file descriptors, and nr_args should hold
> the number of file descriptors the application wishes to pin for the
> duration of the io_uring instance (or until IORING_UNREGISTER_FILES is
> called).
>
> When used, the application must set IOSQE_FIXED_FILE in the sqe->flags
> member. Then, instead of setting sqe->fd to the real fd, it sets sqe->fd
> to the index in the array passed in to IORING_REGISTER_FILES.
>
> Files are automatically unregistered when the io_uring instance is torn
> down. An application need only unregister if it wishes to register a new
> set of fds.
>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
> fs/io_uring.c | 256 ++++++++++++++++++++++++++++++----
> include/uapi/linux/io_uring.h | 9 +-
> 2 files changed, 235 insertions(+), 30 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 13/19] net: split out functions related to registering inflight socket files
From: Hannes Reinecke @ 2019-02-09 9:49 UTC (permalink / raw)
To: Jens Axboe, linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro, netdev, David S . Miller
In-Reply-To: <20190208173423.27014-14-axboe@kernel.dk>
On 2/8/19 6:34 PM, Jens Axboe wrote:
> We need this functionality for the io_uring file registration, but
> we cannot rely on it since CONFIG_UNIX can be modular. Move the helpers
> to a separate file, that's always builtin to the kernel if CONFIG_UNIX is
> m/y.
>
> No functional changes in this patch, just moving code around.
>
> Cc: netdev@vger.kernel.org
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
> include/net/af_unix.h | 1 +
> net/unix/Kconfig | 5 ++
> net/unix/Makefile | 2 +
> net/unix/af_unix.c | 63 +-----------------
> net/unix/garbage.c | 71 +-------------------
> net/unix/scm.c | 146 ++++++++++++++++++++++++++++++++++++++++++
> net/unix/scm.h | 10 +++
> 7 files changed, 168 insertions(+), 130 deletions(-)
> create mode 100644 net/unix/scm.c
> create mode 100644 net/unix/scm.h
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 12/19] io_uring: add support for pre-mapped user IO buffers
From: Hannes Reinecke @ 2019-02-09 9:48 UTC (permalink / raw)
To: Jens Axboe, linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro
In-Reply-To: <20190208173423.27014-13-axboe@kernel.dk>
On 2/8/19 6:34 PM, Jens Axboe wrote:
> If we have fixed user buffers, we can map them into the kernel when we
> setup the io_uring. That avoids the need to do get_user_pages() for
> each and every IO.
>
> To utilize this feature, the application must call io_uring_register()
> after having setup an io_uring instance, passing in
> IORING_REGISTER_BUFFERS as the opcode. The argument must be a pointer to
> an iovec array, and the nr_args should contain how many iovecs the
> application wishes to map.
>
> If successful, these buffers are now mapped into the kernel, eligible
> for IO. To use these fixed buffers, the application must use the
> IORING_OP_READ_FIXED and IORING_OP_WRITE_FIXED opcodes, and then
> set sqe->index to the desired buffer index. sqe->addr..sqe->addr+seq->len
> must point to somewhere inside the indexed buffer.
>
> The application may register buffers throughout the lifetime of the
> io_uring instance. It can call io_uring_register() with
> IORING_UNREGISTER_BUFFERS as the opcode to unregister the current set of
> buffers, and then register a new set. The application need not
> unregister buffers explicitly before shutting down the io_uring
> instance.
>
> It's perfectly valid to setup a larger buffer, and then sometimes only
> use parts of it for an IO. As long as the range is within the originally
> mapped region, it will work just fine.
>
> For now, buffers must not be file backed. If file backed buffers are
> passed in, the registration will fail with -1/EOPNOTSUPP. This
> restriction may be relaxed in the future.
>
> RLIMIT_MEMLOCK is used to check how much memory we can pin. A somewhat
> arbitrary 1G per buffer size is also imposed.
>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
> arch/x86/entry/syscalls/syscall_32.tbl | 1 +
> arch/x86/entry/syscalls/syscall_64.tbl | 1 +
> fs/io_uring.c | 356 ++++++++++++++++++++++++-
> include/linux/sched/user.h | 2 +-
> include/linux/syscalls.h | 2 +
> include/uapi/asm-generic/unistd.h | 4 +-
> include/uapi/linux/io_uring.h | 13 +-
> kernel/sys_ni.c | 1 +
> 8 files changed, 363 insertions(+), 17 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 11/19] block: implement bio helper to add iter bvec pages to bio
From: Hannes Reinecke @ 2019-02-09 9:45 UTC (permalink / raw)
To: Jens Axboe, linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro
In-Reply-To: <20190208173423.27014-12-axboe@kernel.dk>
On 2/8/19 6:34 PM, Jens Axboe wrote:
> For an ITER_BVEC, we can just iterate the iov and add the pages
> to the bio directly. This requires that the caller doesn't releases
> the pages on IO completion, we add a BIO_NO_PAGE_REF flag for that.
>
> The current two callers of bio_iov_iter_get_pages() are updated to
> check if they need to release pages on completion. This makes them
> work with bvecs that contain kernel mapped pages already.
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
> block/bio.c | 59 ++++++++++++++++++++++++++++++++-------
> fs/block_dev.c | 5 ++--
> fs/iomap.c | 5 ++--
> include/linux/blk_types.h | 1 +
> 4 files changed, 56 insertions(+), 14 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 10/19] io_uring: batch io_kiocb allocation
From: Hannes Reinecke @ 2019-02-09 9:43 UTC (permalink / raw)
To: Jens Axboe, linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro
In-Reply-To: <20190208173423.27014-11-axboe@kernel.dk>
On 2/8/19 6:34 PM, Jens Axboe wrote:
> Similarly to how we use the state->ios_left to know how many references
> to get to a file, we can use it to allocate the io_kiocb's we need in
> bulk.
>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
> fs/io_uring.c | 45 ++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 38 insertions(+), 7 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 09/19] io_uring: use fget/fput_many() for file references
From: Hannes Reinecke @ 2019-02-09 9:42 UTC (permalink / raw)
To: Jens Axboe, linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro
In-Reply-To: <20190208173423.27014-10-axboe@kernel.dk>
On 2/8/19 6:34 PM, Jens Axboe wrote:
> Add a separate io_submit_state structure, to cache some of the things
> we need for IO submission.
>
> One such example is file reference batching. io_submit_state. We get as
> many references as the number of sqes we are submitting, and drop
> unused ones if we end up switching files. The assumption here is that
> we're usually only dealing with one fd, and if there are multiple,
> hopefuly they are at least somewhat ordered. Could trivially be extended
> to cover multiple fds, if needed.
>
> On the completion side we do the same thing, except this is trivially
> done just locally in io_iopoll_reap().
>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
> fs/io_uring.c | 142 ++++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 121 insertions(+), 21 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 08/19] fs: add fget_many() and fput_many()
From: Hannes Reinecke @ 2019-02-09 9:41 UTC (permalink / raw)
To: Jens Axboe, linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro
In-Reply-To: <20190208173423.27014-9-axboe@kernel.dk>
On 2/8/19 6:34 PM, Jens Axboe wrote:
> Some uses cases repeatedly get and put references to the same file, but
> the only exposed interface is doing these one at the time. As each of
> these entail an atomic inc or dec on a shared structure, that cost can
> add up.
>
> Add fget_many(), which works just like fget(), except it takes an
> argument for how many references to get on the file. Ditto fput_many(),
> which can drop an arbitrary number of references to a file.
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
> fs/file.c | 15 ++++++++++-----
> fs/file_table.c | 9 +++++++--
> include/linux/file.h | 2 ++
> include/linux/fs.h | 4 +++-
> 4 files changed, 22 insertions(+), 8 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 07/19] io_uring: support for IO polling
From: Hannes Reinecke @ 2019-02-09 9:39 UTC (permalink / raw)
To: Jens Axboe, linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro
In-Reply-To: <20190208173423.27014-8-axboe@kernel.dk>
On 2/8/19 6:34 PM, Jens Axboe wrote:
> Add support for a polled io_uring instance. When a read or write is
> submitted to a polled io_uring, the application must poll for
> completions on the CQ ring through io_uring_enter(2). Polled IO may not
> generate IRQ completions, hence they need to be actively found by the
> application itself.
>
> To use polling, io_uring_setup() must be used with the
> IORING_SETUP_IOPOLL flag being set. It is illegal to mix and match
> polled and non-polled IO on an io_uring.
>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
> fs/io_uring.c | 274 ++++++++++++++++++++++++++++++++--
> include/uapi/linux/io_uring.h | 5 +
> 2 files changed, 270 insertions(+), 9 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 06/19] io_uring: add fsync support
From: Hannes Reinecke @ 2019-02-09 9:37 UTC (permalink / raw)
To: Jens Axboe, linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro
In-Reply-To: <20190208173423.27014-7-axboe@kernel.dk>
On 2/8/19 6:34 PM, Jens Axboe wrote:
> From: Christoph Hellwig <hch@lst.de>
>
> Add a new fsync opcode, which either syncs a range if one is passed,
> or the whole file if the offset and length fields are both cleared
> to zero. A flag is provided to use fdatasync semantics, that is only
> force out metadata which is required to retrieve the file data, but
> not others like metadata.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
> fs/io_uring.c | 40 +++++++++++++++++++++++++++++++++++
> include/uapi/linux/io_uring.h | 8 ++++++-
> 2 files changed, 47 insertions(+), 1 deletion(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 05/19] Add io_uring IO interface
From: Hannes Reinecke @ 2019-02-09 9:35 UTC (permalink / raw)
To: Jens Axboe, linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro
In-Reply-To: <20190208173423.27014-6-axboe@kernel.dk>
On 2/8/19 6:34 PM, Jens Axboe wrote:
> The submission queue (SQ) and completion queue (CQ) rings are shared
> between the application and the kernel. This eliminates the need to
> copy data back and forth to submit and complete IO.
>
> IO submissions use the io_uring_sqe data structure, and completions
> are generated in the form of io_uring_cqe data structures. The SQ
> ring is an index into the io_uring_sqe array, which makes it possible
> to submit a batch of IOs without them being contiguous in the ring.
> The CQ ring is always contiguous, as completion events are inherently
> unordered, and hence any io_uring_cqe entry can point back to an
> arbitrary submission.
>
> Two new system calls are added for this:
>
> io_uring_setup(entries, params)
> Sets up an io_uring instance for doing async IO. On success,
> returns a file descriptor that the application can mmap to
> gain access to the SQ ring, CQ ring, and io_uring_sqes.
>
> io_uring_enter(fd, to_submit, min_complete, flags, sigset, sigsetsize)
> Initiates IO against the rings mapped to this fd, or waits for
> them to complete, or both. The behavior is controlled by the
> parameters passed in. If 'to_submit' is non-zero, then we'll
> try and submit new IO. If IORING_ENTER_GETEVENTS is set, the
> kernel will wait for 'min_complete' events, if they aren't
> already available. It's valid to set IORING_ENTER_GETEVENTS
> and 'min_complete' == 0 at the same time, this allows the
> kernel to return already completed events without waiting
> for them. This is useful only for polling, as for IRQ
> driven IO, the application can just check the CQ ring
> without entering the kernel.
>
> With this setup, it's possible to do async IO with a single system
> call. Future developments will enable polled IO with this interface,
> and polled submission as well. The latter will enable an application
> to do IO without doing ANY system calls at all.
>
> For IRQ driven IO, an application only needs to enter the kernel for
> completions if it wants to wait for them to occur.
>
> Each io_uring is backed by a workqueue, to support buffered async IO
> as well. We will only punt to an async context if the command would
> need to wait for IO on the device side. Any data that can be accessed
> directly in the page cache is done inline. This avoids the slowness
> issue of usual threadpools, since cached data is accessed as quickly
> as a sync interface.
>
> Sample application: http://git.kernel.dk/cgit/fio/plain/t/io_uring.c
>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
> arch/x86/entry/syscalls/syscall_32.tbl | 2 +
> arch/x86/entry/syscalls/syscall_64.tbl | 2 +
> fs/Makefile | 1 +
> fs/io_uring.c | 1175 ++++++++++++++++++++++++
> include/linux/fs.h | 9 +
> include/linux/syscalls.h | 6 +
> include/uapi/asm-generic/unistd.h | 6 +-
> include/uapi/linux/io_uring.h | 95 ++
> init/Kconfig | 9 +
> kernel/sys_ni.c | 2 +
> net/unix/garbage.c | 3 +
> 11 files changed, 1309 insertions(+), 1 deletion(-)
> create mode 100644 fs/io_uring.c
> create mode 100644 include/uapi/linux/io_uring.h
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 04/19] iomap: wire up the iopoll method
From: Hannes Reinecke @ 2019-02-09 9:25 UTC (permalink / raw)
To: Jens Axboe, linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro
In-Reply-To: <20190208173423.27014-5-axboe@kernel.dk>
On 2/8/19 6:34 PM, Jens Axboe wrote:
> From: Christoph Hellwig <hch@lst.de>
>
> Store the request queue the last bio was submitted to in the iocb
> private data in addition to the cookie so that we find the right block
> device. Also refactor the common direct I/O bio submission code into a
> nice little helper.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> Modified to use bio_set_polled().
>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
> fs/gfs2/file.c | 2 ++
> fs/iomap.c | 43 ++++++++++++++++++++++++++++---------------
> fs/xfs/xfs_file.c | 1 +
> include/linux/iomap.h | 1 +
> 4 files changed, 32 insertions(+), 15 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 03/19] block: add bio_set_polled() helper
From: Hannes Reinecke @ 2019-02-09 9:24 UTC (permalink / raw)
To: Jens Axboe, linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro
In-Reply-To: <20190208173423.27014-4-axboe@kernel.dk>
On 2/8/19 6:34 PM, Jens Axboe wrote:
> For the upcoming async polled IO, we can't sleep allocating requests.
> If we do, then we introduce a deadlock where the submitter already
> has async polled IO in-flight, but can't wait for them to complete
> since polled requests must be active found and reaped.
>
> Utilize the helper in the blockdev DIRECT_IO code.
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
> fs/block_dev.c | 4 ++--
> include/linux/bio.h | 14 ++++++++++++++
> 2 files changed, 16 insertions(+), 2 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 02/19] block: wire up block device iopoll method
From: Hannes Reinecke @ 2019-02-09 9:22 UTC (permalink / raw)
To: Jens Axboe, linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro
In-Reply-To: <20190208173423.27014-3-axboe@kernel.dk>
On 2/8/19 6:34 PM, Jens Axboe wrote:
> From: Christoph Hellwig <hch@lst.de>
>
> Just call blk_poll on the iocb cookie, we can derive the block device
> from the inode trivially.
>
> Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
> fs/block_dev.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 01/19] fs: add an iopoll method to struct file_operations
From: Hannes Reinecke @ 2019-02-09 9:20 UTC (permalink / raw)
To: Jens Axboe, linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro
In-Reply-To: <20190208173423.27014-2-axboe@kernel.dk>
On 2/8/19 6:34 PM, Jens Axboe wrote:
> From: Christoph Hellwig <hch@lst.de>
>
> This new methods is used to explicitly poll for I/O completion for an
> iocb. It must be called for any iocb submitted asynchronously (that
> is with a non-null ki_complete) which has the IOCB_HIPRI flag set.
>
> The method is assisted by a new ki_cookie field in struct iocb to store
> the polling cookie.
>
> Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
> Documentation/filesystems/vfs.txt | 3 +++
> include/linux/fs.h | 2 ++
> 2 files changed, 5 insertions(+)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 05/19] Add io_uring IO interface
From: Jens Axboe @ 2019-02-09 4:15 UTC (permalink / raw)
To: Jann Horn
Cc: linux-aio, linux-block, Linux API, hch, jmoyer, Avi Kivity,
Al Viro
In-Reply-To: <CAG48ez2Qc9XOApLRb5fnNiOjxaURO8vjZ-EHX7g25gje3weZ6A@mail.gmail.com>
On 2/8/19 3:12 PM, Jann Horn wrote:
> On Fri, Feb 8, 2019 at 6:34 PM Jens Axboe <axboe@kernel.dk> wrote:
>> The submission queue (SQ) and completion queue (CQ) rings are shared
>> between the application and the kernel. This eliminates the need to
>> copy data back and forth to submit and complete IO.
>>
>> IO submissions use the io_uring_sqe data structure, and completions
>> are generated in the form of io_uring_cqe data structures. The SQ
>> ring is an index into the io_uring_sqe array, which makes it possible
>> to submit a batch of IOs without them being contiguous in the ring.
>> The CQ ring is always contiguous, as completion events are inherently
>> unordered, and hence any io_uring_cqe entry can point back to an
>> arbitrary submission.
>>
>> Two new system calls are added for this:
>>
>> io_uring_setup(entries, params)
>> Sets up an io_uring instance for doing async IO. On success,
>> returns a file descriptor that the application can mmap to
>> gain access to the SQ ring, CQ ring, and io_uring_sqes.
>>
>> io_uring_enter(fd, to_submit, min_complete, flags, sigset, sigsetsize)
>> Initiates IO against the rings mapped to this fd, or waits for
>> them to complete, or both. The behavior is controlled by the
>> parameters passed in. If 'to_submit' is non-zero, then we'll
>> try and submit new IO. If IORING_ENTER_GETEVENTS is set, the
>> kernel will wait for 'min_complete' events, if they aren't
>> already available. It's valid to set IORING_ENTER_GETEVENTS
>> and 'min_complete' == 0 at the same time, this allows the
>> kernel to return already completed events without waiting
>> for them. This is useful only for polling, as for IRQ
>> driven IO, the application can just check the CQ ring
>> without entering the kernel.
>>
>> With this setup, it's possible to do async IO with a single system
>> call. Future developments will enable polled IO with this interface,
>> and polled submission as well. The latter will enable an application
>> to do IO without doing ANY system calls at all.
>>
>> For IRQ driven IO, an application only needs to enter the kernel for
>> completions if it wants to wait for them to occur.
>>
>> Each io_uring is backed by a workqueue, to support buffered async IO
>> as well. We will only punt to an async context if the command would
>> need to wait for IO on the device side. Any data that can be accessed
>> directly in the page cache is done inline. This avoids the slowness
>> issue of usual threadpools, since cached data is accessed as quickly
>> as a sync interface.
> [...]
>> +static void io_commit_cqring(struct io_ring_ctx *ctx)
>> +{
>> + struct io_cq_ring *ring = ctx->cq_ring;
>> +
>> + if (ctx->cached_cq_tail != ring->r.tail) {
>
> I know that this is very unlikely to actually matter, but both because
> I don't feel fuzzy about relying on compiler internals regarding when
> the compiler might decide to generate dangerous double-reads (if
> switch() can blow up, why shouldn't the compiler be able to make if()
> blow up if it wants to, too?), and because I would like it to be as
> clear as possible to the reader which memory is shared with userspace,
> can we please have READ_ONCE() on *every* shared memory read, not just
> the ones in places that look like they might plausibly blow up
> otherwise? Sorry, shared memory is a bit of a pet peeve of mine.
Sure, I've done that now.
>> + /* order cqe stores with ring update */
>> + smp_wmb();
>> + WRITE_ONCE(ring->r.tail, ctx->cached_cq_tail);
>> + /* write side barrier of tail update, app has read side */
>> + smp_wmb();
>> +
>> + if (wq_has_sleeper(&ctx->cq_wait)) {
>> + wake_up_interruptible(&ctx->cq_wait);
>> + kill_fasync(&ctx->cq_fasync, SIGIO, POLL_IN);
>> + }
>> + }
>> +}
> [...]
>> +static void io_cqring_fill_event(struct io_ring_ctx *ctx, u64 ki_user_data,
>> + long res, unsigned ev_flags)
>> +{
>> + struct io_uring_cqe *cqe;
>> +
>> + /*
>> + * If we can't get a cq entry, userspace overflowed the
>> + * submission (by quite a lot). Increment the overflow count in
>> + * the ring.
>> + */
>> + cqe = io_get_cqring(ctx);
>> + if (cqe) {
>> + cqe->user_data = ki_user_data;
>> + cqe->res = res;
>> + cqe->flags = ev_flags;
>
> Please use WRITE_ONCE() for stores like these.
Done
>> + } else
>> + ctx->cq_ring->overflow++;
>> +}
> [...]
>> +static int __io_submit_sqe(struct io_ring_ctx *ctx, struct io_kiocb *req,
>> + const struct sqe_submit *s, bool force_nonblock)
>> +{
>> + ssize_t ret;
>> + int opcode;
>> +
>> + if (unlikely(s->index >= ctx->sq_entries))
>> + return -EINVAL;
>> + req->user_data = READ_ONCE(s->sqe->user_data);
>> +
>> + opcode = READ_ONCE(s->sqe->opcode);
>
> There might be a sneaky bug here. Consider the following scenario:
>
> 1. request gets submitted from io_sq_wq_submit_work() with opcode
> IORING_OP_READV, io_read() is invoked
> 2. io_read() looks up the file, taking a reference to it
> 3. call_read_iter() returns -EAGAIN
> 4. io_read() returns -EAGAIN without dropping its reference on the
> file (because it expects that it'll be called again)
> 5. __io_submit_sqe() returns -EAGAIN
> 6. io_sq_wq_submit_work() loops back and retries __io_submit_sqe()
> 7. __io_submit_sqe() reads opcode again, this time it's IORING_OP_NOP
> 8. io_nop() gets called
> 9. io_nop() uses io_free_req() to delete the request without dropping
> its reference on the file
>
> So that's a file reference leak, I think?
Hmm yes, that could happen with a malicious app.
For non-file using opcodes, I think we should just error the sqe if we
have req->rw.ki_filp set. That shouldn't happen unless the app is doing
something funky. I'll fix this.
>> + switch (opcode) {
>> + case IORING_OP_NOP:
>> + ret = io_nop(req, req->user_data);
>> + break;
>> + case IORING_OP_READV:
>> + ret = io_read(req, s, force_nonblock);
>> + break;
>> + case IORING_OP_WRITEV:
>> + ret = io_write(req, s, force_nonblock);
>> + break;
>> + default:
>> + ret = -EINVAL;
>> + break;
>> + }
>> +
>> + return ret;
>> +}
> [...]
>> +static int io_submit_sqe(struct io_ring_ctx *ctx, const struct sqe_submit *s)
>> +{
>> + struct io_kiocb *req;
>> + ssize_t ret;
>> +
>> + /* enforce forwards compatibility on users */
>> + if (unlikely(s->sqe->flags))
>> + return -EINVAL;
>> +
>> + req = io_get_req(ctx);
>> + if (unlikely(!req))
>> + return -EAGAIN;
>> +
>> + req->rw.ki_filp = NULL;
>> +
>> + ret = __io_submit_sqe(ctx, req, s, true);
>> + if (ret == -EAGAIN) {
>> + memcpy(&req->submit, s, sizeof(*s));
>> + INIT_WORK(&req->work, io_sq_wq_submit_work);
>> + queue_work(ctx->sqo_wq, &req->work);
>> + ret = 0;
>> + }
>> + if (ret)
>> + io_free_req(req);
>> +
>> + return ret;
>> +}
>> +
>> +static void io_commit_sqring(struct io_ring_ctx *ctx)
>> +{
>> + struct io_sq_ring *ring = ctx->sq_ring;
>> +
>> + if (ctx->cached_sq_head != ring->r.head) {
>> + WRITE_ONCE(ring->r.head, ctx->cached_sq_head);
>> + /* write side barrier of head update, app has read side */
>> + smp_wmb();
>
> Can you elaborate on what this memory barrier is doing? Don't you need
> some sort of memory barrier *before* the WRITE_ONCE(), to ensure that
> nobody sees the updated head before you're done reading the submission
> queue entry? Or is that barrier elsewhere?
The matching read barrier is in the application, it must do that before
reading ->head for the SQ ring.
For the other barrier, since the ring->r.head now has a READ_ONCE(),
that should be all we need to ensure that loads are done.
>> + }
>> +}
>> +
>> +/*
>> + * Undo last io_get_sqring()
>> + */
>> +static void io_drop_sqring(struct io_ring_ctx *ctx)
>> +{
>> + ctx->cached_sq_head--;
>> +}
> [...]
>> +static void io_unaccount_mem(struct user_struct *user, unsigned long nr_pages)
>> +{
>> + if (capable(CAP_IPC_LOCK))
>> + return;
>
> Hrm... what happens if root creates a uring, drops CAP_IPC_LOCK, and
> then destroys the uring? Will the pages get subtracted from
> ->locked_vm even though they were never added to it, causing a
> wraparound?
>
> You might want to make sure that ctx->user is set if and only if the
> creator didn't have CAP_IPC_LOCK, and then just do a `user == NULL`
> check instead of a `capable(...)` check. Or you could do what BPF is
> doing (AFAICS) and not treat root specially - root can just bump the
> rlimit if necessary.
That won't work since we use ->user for other items later on. But I can
store whether we need it or not, I'll do that.
>
>> + atomic_long_sub(nr_pages, &user->locked_vm);
>> +}
>> +
>> +static int io_account_mem(struct user_struct *user, unsigned long nr_pages)
>> +{
>> + unsigned long page_limit, cur_pages, new_pages;
>> +
>> + if (capable(CAP_IPC_LOCK))
>> + return 0;
>> +
>> + /* Don't allow more pages than we can safely lock */
>> + page_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>> +
>> + do {
>> + cur_pages = atomic_long_read(&user->locked_vm);
>> + new_pages = cur_pages + nr_pages;
>> + if (new_pages > page_limit)
>> + return -ENOMEM;
>> + } while (atomic_long_cmpxchg(&user->locked_vm, cur_pages,
>> + new_pages) != cur_pages);
>> +
>> + return 0;
>> +}
> [...]
>> +config IO_URING
>> + bool "Enable IO uring support" if EXPERT
>> + select ANON_INODES
>> + default y
>> + help
>> + This option enables support for the io_uring interface, enabling
>> + applications to submit and completion IO through submission and
>> + completion rings that are shared between the kernel and application.
>
> Nit: I can't parse this part: "enabling applications to submit and
> completion IO"
completion -> complete
Fixed it up.
--
Jens Axboe
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 14/19] io_uring: add file set registration
From: Jens Axboe @ 2019-02-09 0:16 UTC (permalink / raw)
To: Jann Horn
Cc: linux-aio, linux-block, Linux API, hch, jmoyer, Avi Kivity,
Al Viro
In-Reply-To: <CAG48ez2Fp10zQ4Y-x4RJh3Cu=vABu9w8ZrAJThvkh6pr74ZiQw@mail.gmail.com>
On 2/8/19 1:26 PM, Jann Horn wrote:
> On Fri, Feb 8, 2019 at 6:35 PM Jens Axboe <axboe@kernel.dk> wrote:
>> We normally have to fget/fput for each IO we do on a file. Even with
>> the batching we do, the cost of the atomic inc/dec of the file usage
>> count adds up.
>>
>> This adds IORING_REGISTER_FILES, and IORING_UNREGISTER_FILES opcodes
>> for the io_uring_register(2) system call. The arguments passed in must
>> be an array of __s32 holding file descriptors, and nr_args should hold
>> the number of file descriptors the application wishes to pin for the
>> duration of the io_uring instance (or until IORING_UNREGISTER_FILES is
>> called).
>>
>> When used, the application must set IOSQE_FIXED_FILE in the sqe->flags
>> member. Then, instead of setting sqe->fd to the real fd, it sets sqe->fd
>> to the index in the array passed in to IORING_REGISTER_FILES.
>>
>> Files are automatically unregistered when the io_uring instance is torn
>> down. An application need only unregister if it wishes to register a new
>> set of fds.
>
> I think the overall concept here is still broken: You're giving the
> user_files to the GC, and I think the GC can drop their refcounts, but
> I don't see you actually getting feedback from the GC anywhere that
> would let the GC break your references? E.g. in io_prep_rw() you grab
> file pointers from ctx->user_files after simply checking
> ctx->nr_user_files, and there is no path from the GC that touches
> those fields. As far as I can tell, the GC is just going to go through
> unix_destruct_scm() and drop references on your files, causing
> use-after-free.
>
> But the unix GC is complicated, and maybe I'm just missing something...
Only when the skb is released, which is either done when the io_uring
is torn down (and then definitely safe), or if the socket is released,
which is again also at a safe time.
>> +static void __io_sqe_files_unregister(struct io_ring_ctx *ctx)
>> +{
>> +#if defined(CONFIG_UNIX)
>> + if (ctx->ring_sock) {
>> + struct sock *sock = ctx->ring_sock->sk;
>> + struct sk_buff *skb;
>> +
>> + while ((skb = skb_dequeue(&sock->sk_receive_queue)) != NULL)
>> + kfree_skb(skb);
>> + }
>> +#else
>> + int i;
>> +
>> + for (i = 0; i < ctx->nr_user_files; i++)
>> + fput(ctx->user_files[i]);
>> +#endif
>> +}
>> +
>> +static int io_sqe_files_unregister(struct io_ring_ctx *ctx)
>> +{
>> + if (!ctx->user_files)
>> + return -ENXIO;
>> +
>> + __io_sqe_files_unregister(ctx);
>> + kfree(ctx->user_files);
>> + ctx->user_files = NULL;
>> + return 0;
>> +}
>> +
>> +#if defined(CONFIG_UNIX)
>> +static int __io_sqe_files_scm(struct io_ring_ctx *ctx, int nr, int offset)
>> +{
>> + struct scm_fp_list *fpl;
>> + struct sk_buff *skb;
>> + int i;
>> +
>> + fpl = kzalloc(sizeof(*fpl), GFP_KERNEL);
>> + if (!fpl)
>> + return -ENOMEM;
>> +
>> + skb = alloc_skb(0, GFP_KERNEL);
>> + if (!skb) {
>> + kfree(fpl);
>> + return -ENOMEM;
>> + }
>> +
>> + skb->sk = ctx->ring_sock->sk;
>> + skb->destructor = unix_destruct_scm;
>> +
>> + fpl->user = get_uid(ctx->user);
>> + for (i = 0; i < nr; i++) {
>> + fpl->fp[i] = get_file(ctx->user_files[i + offset]);
>> + unix_inflight(fpl->user, fpl->fp[i]);
>> + fput(fpl->fp[i]);
>
> This pattern is almost always superfluous. You increment the file's
> refcount, maybe insert the file into a list (essentially), and drop
> the file's refcount back down. You're already holding a stable
> reference, and you're not temporarily lending that to anyone else.
Actually, this is me messing up. The fput() should be done AFTER
adding to the socket. I'll fix that.
--
Jens Axboe
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 12/19] io_uring: add support for pre-mapped user IO buffers
From: Jens Axboe @ 2019-02-08 23:38 UTC (permalink / raw)
To: Jann Horn
Cc: linux-aio, linux-block, Linux API, hch, jmoyer, Avi Kivity,
Al Viro
In-Reply-To: <CAG48ez0uH_muHobzpwFzdyBWCcWDxbnfpuxABntph6Te+tYxGg@mail.gmail.com>
On 2/8/19 3:54 PM, Jann Horn wrote:
> On Fri, Feb 8, 2019 at 6:35 PM Jens Axboe <axboe@kernel.dk> wrote:
>> If we have fixed user buffers, we can map them into the kernel when we
>> setup the io_uring. That avoids the need to do get_user_pages() for
>> each and every IO.
>>
>> To utilize this feature, the application must call io_uring_register()
>> after having setup an io_uring instance, passing in
>> IORING_REGISTER_BUFFERS as the opcode. The argument must be a pointer to
>> an iovec array, and the nr_args should contain how many iovecs the
>> application wishes to map.
>>
>> If successful, these buffers are now mapped into the kernel, eligible
>> for IO. To use these fixed buffers, the application must use the
>> IORING_OP_READ_FIXED and IORING_OP_WRITE_FIXED opcodes, and then
>> set sqe->index to the desired buffer index. sqe->addr..sqe->addr+seq->len
>> must point to somewhere inside the indexed buffer.
>>
>> The application may register buffers throughout the lifetime of the
>> io_uring instance. It can call io_uring_register() with
>> IORING_UNREGISTER_BUFFERS as the opcode to unregister the current set of
>> buffers, and then register a new set. The application need not
>> unregister buffers explicitly before shutting down the io_uring
>> instance.
>>
>> It's perfectly valid to setup a larger buffer, and then sometimes only
>> use parts of it for an IO. As long as the range is within the originally
>> mapped region, it will work just fine.
>>
>> For now, buffers must not be file backed. If file backed buffers are
>> passed in, the registration will fail with -1/EOPNOTSUPP. This
>> restriction may be relaxed in the future.
>>
>> RLIMIT_MEMLOCK is used to check how much memory we can pin. A somewhat
>> arbitrary 1G per buffer size is also imposed.
> [...]
>> static int io_import_iovec(struct io_ring_ctx *ctx, int rw,
>> const struct sqe_submit *s, struct iovec **iovec,
>> struct iov_iter *iter)
>> @@ -711,6 +763,15 @@ static int io_import_iovec(struct io_ring_ctx *ctx, int rw,
>> const struct io_uring_sqe *sqe = s->sqe;
>> void __user *buf = u64_to_user_ptr(READ_ONCE(sqe->addr));
>> size_t sqe_len = READ_ONCE(sqe->len);
>> + u8 opcode;
>
> (You could add a comment here if you want, something like "We're
> reading ->opcode for the second time, but the first read doesn't care
> whether it's _FIXED or not, so it doesn't matter whether ->opcode
> changes concurrently. The first read does care about whether it is a
> READ or a WRITE, so we don't trust this read for that purpose and
> instead let the caller pass in the read/write flag.")
Sure, I can add that.
>> +static int io_sqe_buffer_unregister(struct io_ring_ctx *ctx)
>> +{
>> + int i, j;
>> +
>> + if (!ctx->user_bufs)
>> + return -ENXIO;
>> +
>> + for (i = 0; i < ctx->sq_entries; i++) {
>
> ->sq_entries? Shouldn't this be ->nr_user_bufs?
It should! I swear I already fixed that, odd. Maybe that was somewhere
else...
>> + struct io_mapped_ubuf *imu = &ctx->user_bufs[i];
>> +
>> + for (j = 0; j < imu->nr_bvecs; j++)
>> + put_page(imu->bvec[j].bv_page);
>> +
>> + io_unaccount_mem(ctx->user, imu->nr_bvecs);
>> + kfree(imu->bvec);
>> + imu->nr_bvecs = 0;
>> + }
>> +
>> + kfree(ctx->user_bufs);
>> + ctx->user_bufs = NULL;
>
> (It isn't really necessary, but you could set nr_user_bufs=0 here.)
Doesn't hurt to be defensive.
>> + return 0;
>> +}
> [...]
>> +static int io_sqe_buffer_register(struct io_ring_ctx *ctx, void __user *arg,
>> + unsigned nr_args)
>> +{
>> + struct vm_area_struct **vmas = NULL;
>> + struct page **pages = NULL;
>> + int i, j, got_pages = 0;
>> + int ret = -EINVAL;
>> +
>> + if (ctx->user_bufs)
>> + return -EBUSY;
>> + if (!nr_args || nr_args > UIO_MAXIOV)
>> + return -EINVAL;
>> +
>> + ctx->user_bufs = kcalloc(nr_args, sizeof(struct io_mapped_ubuf),
>> + GFP_KERNEL);
>> + if (!ctx->user_bufs)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < nr_args; i++) {
>> + struct io_mapped_ubuf *imu = &ctx->user_bufs[i];
>> + unsigned long off, start, end, ubuf;
>> + int pret, nr_pages;
>> + struct iovec iov;
>> + size_t size;
>> +
>> + ret = io_copy_iov(ctx, &iov, arg, i);
>> + if (ret)
>> + break;
>> +
>> + /*
>> + * Don't impose further limits on the size and buffer
>> + * constraints here, we'll -EINVAL later when IO is
>> + * submitted if they are wrong.
>> + */
>> + ret = -EFAULT;
>> + if (!iov.iov_base || !iov.iov_len)
>> + goto err;
>> +
>> + /* arbitrary limit, but we need something */
>> + if (iov.iov_len > SZ_1G)
>> + goto err;
>> +
>> + ubuf = (unsigned long) iov.iov_base;
>> + end = (ubuf + iov.iov_len + PAGE_SIZE - 1) >> PAGE_SHIFT;
>> + start = ubuf >> PAGE_SHIFT;
>> + nr_pages = end - start;
>> +
>> + ret = io_account_mem(ctx->user, nr_pages);
>
> Technically, this accounting is probably a bit off; I think if you
> pass in a vector of 4K areas from 1G hugepages, you're going to pin
> factor 0x40000 more memory than you think you're pinning.
> (get_user_pages() counts references against the head page of a
> compound page; nothing in the kernel can tell afterwards which part of
> the hugepage you're using.) I'm not sure how much of a problem that
> is, but it should probably at least be documented. Unless I'm just
> missing something?
No I think you are right, it doesn't account for the hugepage size if
you pass in huge pages. I'll fix that up.
>> + if (ret)
>> + goto err;
>> +
>> + if (!pages || nr_pages > got_pages) {
>> + kfree(vmas);
>> + kfree(pages);
>> + pages = kmalloc_array(nr_pages, sizeof(struct page *),
>> + GFP_KERNEL);
>> + vmas = kmalloc_array(nr_pages,
>> + sizeof(struct vma_area_struct *),
>> + GFP_KERNEL);
>> + if (!pages || !vmas) {
>> + ret = -ENOMEM;
>> + io_unaccount_mem(ctx->user, nr_pages);
>> + goto err;
>> + }
>> + got_pages = nr_pages;
>> + }
>> +
>> + imu->bvec = kmalloc_array(nr_pages, sizeof(struct bio_vec),
>> + GFP_KERNEL);
>> + if (!imu->bvec) {
>> + io_unaccount_mem(ctx->user, nr_pages);
>> + goto err;
>> + }
>> +
>> + down_write(¤t->mm->mmap_sem);
>
> Weren't you planning to make this down_read()?
I think I accidentally messed that up when going back to not using
FOLL_ANON. Fixed (again), thanks.
>> + pret = get_user_pages_longterm(ubuf, nr_pages, FOLL_WRITE,
>> + pages, vmas);
>> + if (pret == nr_pages) {
>> + /* don't support file backed memory */
>> + for (j = 0; j < nr_pages; j++) {
>> + struct vm_area_struct *vma = vmas[j];
>> +
>> + if (vma->vm_file &&
>> + !is_file_hugepages(vma->vm_file)) {
>> + ret = -EOPNOTSUPP;
>> + break;
>> + }
>> + }
>> + } else {
>> + ret = pret < 0 ? pret : -EFAULT;
>> + }
>> + up_write(¤t->mm->mmap_sem);
> [...]
>> +}
> [...]
>> diff --git a/include/linux/sched/user.h b/include/linux/sched/user.h
>> index 39ad98c09c58..c7b5f86b91a1 100644
>> --- a/include/linux/sched/user.h
>> +++ b/include/linux/sched/user.h
>> @@ -40,7 +40,7 @@ struct user_struct {
>> kuid_t uid;
>>
>> #if defined(CONFIG_PERF_EVENTS) || defined(CONFIG_BPF_SYSCALL) || \
>> - defined(CONFIG_NET)
>> + defined(CONFIG_NET) || defined(CONFIG_IO_URING)
>> atomic_long_t locked_vm;
>> #endif
>
> You're already using locked_vm in patch 5, right? I think that means
> that from patch 5 up to this patch, some kernel configs will fail to
> build.
Good point, I need to do this earlier now.
--
Jens Axboe
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 06/19] io_uring: add fsync support
From: Jens Axboe @ 2019-02-08 23:31 UTC (permalink / raw)
To: Jann Horn
Cc: linux-aio, linux-block, Linux API, hch, jmoyer, Avi Kivity,
Al Viro
In-Reply-To: <CAG48ez1zUf9GqZyoD3XEEYyeg2zrBSAO5uKT3MKHOPMk6eujDQ@mail.gmail.com>
On 2/8/19 3:36 PM, Jann Horn wrote:
> On Fri, Feb 8, 2019 at 6:34 PM Jens Axboe <axboe@kernel.dk> wrote:
>> From: Christoph Hellwig <hch@lst.de>
>>
>> Add a new fsync opcode, which either syncs a range if one is passed,
>> or the whole file if the offset and length fields are both cleared
>> to zero. A flag is provided to use fdatasync semantics, that is only
>> force out metadata which is required to retrieve the file data, but
>> not others like metadata.
>>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>> ---
> [...]
>> +static int io_fsync(struct io_kiocb *req, const struct io_uring_sqe *sqe,
>> + bool force_nonblock)
>> +{
>> + struct io_ring_ctx *ctx = req->ctx;
>> + loff_t sqe_off = READ_ONCE(sqe->off);
>> + loff_t sqe_len = READ_ONCE(sqe->len);
>> + loff_t end = sqe_off + sqe_len;
>> + unsigned fsync_flags;
>> + struct file *file;
>> + int ret, fd;
>> +
>> + /* fsync always requires a blocking context */
>> + if (force_nonblock)
>> + return -EAGAIN;
>> +
>> + if (unlikely(sqe->addr || sqe->ioprio))
>> + return -EINVAL;
>> +
>> + fsync_flags = READ_ONCE(sqe->fsync_flags);
>> + if (unlikely(fsync_flags & ~IORING_FSYNC_DATASYNC))
>> + return -EINVAL;
>> +
>> + fd = READ_ONCE(sqe->fd);
>> + file = fget(fd);
>
> This always runs on the workqueue, right? Is it possible to call
> fget() on a workqueue?
Oops yes, I've now split that into a io_prep_fsync() and io_fsync()
part so that it works correctly with this new method. Also added a
liburing test for this.
--
Jens Axboe
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 12/19] io_uring: add support for pre-mapped user IO buffers
From: Jann Horn @ 2019-02-08 22:54 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-aio, linux-block, Linux API, hch, jmoyer, Avi Kivity,
Al Viro
In-Reply-To: <20190208173423.27014-13-axboe@kernel.dk>
On Fri, Feb 8, 2019 at 6:35 PM Jens Axboe <axboe@kernel.dk> wrote:
> If we have fixed user buffers, we can map them into the kernel when we
> setup the io_uring. That avoids the need to do get_user_pages() for
> each and every IO.
>
> To utilize this feature, the application must call io_uring_register()
> after having setup an io_uring instance, passing in
> IORING_REGISTER_BUFFERS as the opcode. The argument must be a pointer to
> an iovec array, and the nr_args should contain how many iovecs the
> application wishes to map.
>
> If successful, these buffers are now mapped into the kernel, eligible
> for IO. To use these fixed buffers, the application must use the
> IORING_OP_READ_FIXED and IORING_OP_WRITE_FIXED opcodes, and then
> set sqe->index to the desired buffer index. sqe->addr..sqe->addr+seq->len
> must point to somewhere inside the indexed buffer.
>
> The application may register buffers throughout the lifetime of the
> io_uring instance. It can call io_uring_register() with
> IORING_UNREGISTER_BUFFERS as the opcode to unregister the current set of
> buffers, and then register a new set. The application need not
> unregister buffers explicitly before shutting down the io_uring
> instance.
>
> It's perfectly valid to setup a larger buffer, and then sometimes only
> use parts of it for an IO. As long as the range is within the originally
> mapped region, it will work just fine.
>
> For now, buffers must not be file backed. If file backed buffers are
> passed in, the registration will fail with -1/EOPNOTSUPP. This
> restriction may be relaxed in the future.
>
> RLIMIT_MEMLOCK is used to check how much memory we can pin. A somewhat
> arbitrary 1G per buffer size is also imposed.
[...]
> static int io_import_iovec(struct io_ring_ctx *ctx, int rw,
> const struct sqe_submit *s, struct iovec **iovec,
> struct iov_iter *iter)
> @@ -711,6 +763,15 @@ static int io_import_iovec(struct io_ring_ctx *ctx, int rw,
> const struct io_uring_sqe *sqe = s->sqe;
> void __user *buf = u64_to_user_ptr(READ_ONCE(sqe->addr));
> size_t sqe_len = READ_ONCE(sqe->len);
> + u8 opcode;
(You could add a comment here if you want, something like "We're
reading ->opcode for the second time, but the first read doesn't care
whether it's _FIXED or not, so it doesn't matter whether ->opcode
changes concurrently. The first read does care about whether it is a
READ or a WRITE, so we don't trust this read for that purpose and
instead let the caller pass in the read/write flag.")
> + opcode = READ_ONCE(sqe->opcode);
> + if (opcode == IORING_OP_READ_FIXED ||
> + opcode == IORING_OP_WRITE_FIXED) {
> + ssize_t ret = io_import_fixed(ctx, rw, sqe, iter);
> + *iovec = NULL;
> + return ret;
> + }
>
> if (!s->has_user)
> return EFAULT;
[...]
> @@ -1242,6 +1334,187 @@ static unsigned long ring_pages(unsigned sq_entries, unsigned cq_entries)
> return (bytes + PAGE_SIZE - 1) / PAGE_SIZE;
> }
>
> +static int io_sqe_buffer_unregister(struct io_ring_ctx *ctx)
> +{
> + int i, j;
> +
> + if (!ctx->user_bufs)
> + return -ENXIO;
> +
> + for (i = 0; i < ctx->sq_entries; i++) {
->sq_entries? Shouldn't this be ->nr_user_bufs?
> + struct io_mapped_ubuf *imu = &ctx->user_bufs[i];
> +
> + for (j = 0; j < imu->nr_bvecs; j++)
> + put_page(imu->bvec[j].bv_page);
> +
> + io_unaccount_mem(ctx->user, imu->nr_bvecs);
> + kfree(imu->bvec);
> + imu->nr_bvecs = 0;
> + }
> +
> + kfree(ctx->user_bufs);
> + ctx->user_bufs = NULL;
(It isn't really necessary, but you could set nr_user_bufs=0 here.)
> + return 0;
> +}
[...]
> +static int io_sqe_buffer_register(struct io_ring_ctx *ctx, void __user *arg,
> + unsigned nr_args)
> +{
> + struct vm_area_struct **vmas = NULL;
> + struct page **pages = NULL;
> + int i, j, got_pages = 0;
> + int ret = -EINVAL;
> +
> + if (ctx->user_bufs)
> + return -EBUSY;
> + if (!nr_args || nr_args > UIO_MAXIOV)
> + return -EINVAL;
> +
> + ctx->user_bufs = kcalloc(nr_args, sizeof(struct io_mapped_ubuf),
> + GFP_KERNEL);
> + if (!ctx->user_bufs)
> + return -ENOMEM;
> +
> + for (i = 0; i < nr_args; i++) {
> + struct io_mapped_ubuf *imu = &ctx->user_bufs[i];
> + unsigned long off, start, end, ubuf;
> + int pret, nr_pages;
> + struct iovec iov;
> + size_t size;
> +
> + ret = io_copy_iov(ctx, &iov, arg, i);
> + if (ret)
> + break;
> +
> + /*
> + * Don't impose further limits on the size and buffer
> + * constraints here, we'll -EINVAL later when IO is
> + * submitted if they are wrong.
> + */
> + ret = -EFAULT;
> + if (!iov.iov_base || !iov.iov_len)
> + goto err;
> +
> + /* arbitrary limit, but we need something */
> + if (iov.iov_len > SZ_1G)
> + goto err;
> +
> + ubuf = (unsigned long) iov.iov_base;
> + end = (ubuf + iov.iov_len + PAGE_SIZE - 1) >> PAGE_SHIFT;
> + start = ubuf >> PAGE_SHIFT;
> + nr_pages = end - start;
> +
> + ret = io_account_mem(ctx->user, nr_pages);
Technically, this accounting is probably a bit off; I think if you
pass in a vector of 4K areas from 1G hugepages, you're going to pin
factor 0x40000 more memory than you think you're pinning.
(get_user_pages() counts references against the head page of a
compound page; nothing in the kernel can tell afterwards which part of
the hugepage you're using.) I'm not sure how much of a problem that
is, but it should probably at least be documented. Unless I'm just
missing something?
> + if (ret)
> + goto err;
> +
> + if (!pages || nr_pages > got_pages) {
> + kfree(vmas);
> + kfree(pages);
> + pages = kmalloc_array(nr_pages, sizeof(struct page *),
> + GFP_KERNEL);
> + vmas = kmalloc_array(nr_pages,
> + sizeof(struct vma_area_struct *),
> + GFP_KERNEL);
> + if (!pages || !vmas) {
> + ret = -ENOMEM;
> + io_unaccount_mem(ctx->user, nr_pages);
> + goto err;
> + }
> + got_pages = nr_pages;
> + }
> +
> + imu->bvec = kmalloc_array(nr_pages, sizeof(struct bio_vec),
> + GFP_KERNEL);
> + if (!imu->bvec) {
> + io_unaccount_mem(ctx->user, nr_pages);
> + goto err;
> + }
> +
> + down_write(¤t->mm->mmap_sem);
Weren't you planning to make this down_read()?
> + pret = get_user_pages_longterm(ubuf, nr_pages, FOLL_WRITE,
> + pages, vmas);
> + if (pret == nr_pages) {
> + /* don't support file backed memory */
> + for (j = 0; j < nr_pages; j++) {
> + struct vm_area_struct *vma = vmas[j];
> +
> + if (vma->vm_file &&
> + !is_file_hugepages(vma->vm_file)) {
> + ret = -EOPNOTSUPP;
> + break;
> + }
> + }
> + } else {
> + ret = pret < 0 ? pret : -EFAULT;
> + }
> + up_write(¤t->mm->mmap_sem);
[...]
> +}
[...]
> diff --git a/include/linux/sched/user.h b/include/linux/sched/user.h
> index 39ad98c09c58..c7b5f86b91a1 100644
> --- a/include/linux/sched/user.h
> +++ b/include/linux/sched/user.h
> @@ -40,7 +40,7 @@ struct user_struct {
> kuid_t uid;
>
> #if defined(CONFIG_PERF_EVENTS) || defined(CONFIG_BPF_SYSCALL) || \
> - defined(CONFIG_NET)
> + defined(CONFIG_NET) || defined(CONFIG_IO_URING)
> atomic_long_t locked_vm;
> #endif
You're already using locked_vm in patch 5, right? I think that means
that from patch 5 up to this patch, some kernel configs will fail to
build.
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox