* openat ignores changes to RLIMIT_NOFILE?
@ 2020-03-19 12:12 Dmitry Kadashev
2020-03-20 1:23 ` Jens Axboe
0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Kadashev @ 2020-03-19 12:12 UTC (permalink / raw)
To: io-uring
[-- Attachment #1: Type: text/plain, Size: 748 bytes --]
Hi,
It seems that openat calls issued via io_uring ignore changes to
RLIMIT_NOFILE. Maybe a wrong limit is checked. A short reproducer is
attached, it sets RLIMIT_NOFILE to a very low value and the sync
openat() call fails with "Too many open files", but io_uring one
succeeds. The resulting FD is completely usable, I've tried writing to
it successfully.
To be clear, originally I've encountered another side of this problem:
we increase the limit in our code, and io_uring's openat started to
fail after a while under load, while the sync calls executed on a
thread pool were working as expected. It's just easier to demo with
small limit.
Kernel 5.6-rc2, 5.6-rc6.
Hope it's the right place to report an issue like this.
Thanks.
--
Dmitry
[-- Attachment #2: test-io_uring-openat-rlimit.c --]
[-- Type: text/x-csrc, Size: 2135 bytes --]
#include <liburing.h>
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <errno.h>
#include <sys/resource.h>
#include <unistd.h>
#define DIE(...) do {\
fprintf(stderr, __VA_ARGS__);\
abort();\
} while(0);
static const int RSIZE = 2;
static const int OPEN_FLAGS = O_RDWR | O_CREAT;
static const mode_t OPEN_MODE = S_IRUSR | S_IWUSR;
void setup_rlimit()
{
struct rlimit rlim;
rlim.rlim_cur = rlim.rlim_max = 5; // 3 stdio ones, 1 for uring, 1 for dirfd
if (setrlimit(RLIMIT_NOFILE, &rlim) == -1) {
DIE("setrlimit nofile: %s\n", strerror(errno));
}
}
void open_sync(int dfd, const char* fn)
{
int fd = openat(dfd, fn, OPEN_FLAGS, OPEN_MODE);
if (fd < 0) {
fprintf(stderr, "sync open failed: %s\n", strerror(errno));
}
else {
fprintf(stderr, "sync open succeeded\n");
close(fd);
}
}
void open_io_uring(struct io_uring *ring, int dfd, const char* fn)
{
struct io_uring_sqe *sqe;
sqe = io_uring_get_sqe(ring);
if (!sqe) {
fprintf(stderr, "failed to get sqe\n");
return;
}
io_uring_prep_openat(sqe, dfd, fn, OPEN_FLAGS, OPEN_MODE);
int ret = io_uring_submit(ring);
if (ret < 0) {
fprintf(stderr, "failed to submit openat: %s\n", strerror(-ret));
return;
}
struct io_uring_cqe *cqe;
ret = io_uring_wait_cqe(ring, &cqe);
int fd = cqe->res;
io_uring_cqe_seen(ring, cqe);
if (ret < 0) {
fprintf(stderr, "wait_cqe failed: %s\n", strerror(-ret));
}
else if (fd < 0) {
fprintf(stderr, "io_uring openat failed: %s\n", strerror(-fd));
}
else {
fprintf(stderr, "io_uring openat succeeded\n");
close(fd);
}
}
int main(int argc, const char *argv[])
{
const char *mode = "io_uring";
const char *fn = "io_uring_openat_test";
setup_rlimit();
int dfd = open("/tmp", O_RDONLY | O_DIRECTORY);
if (dfd < 0) {
DIE("open /tmp: %s\n", strerror(errno));
}
struct io_uring ring;
int ret = io_uring_queue_init(RSIZE, &ring, 0);
if (ret < 0) {
DIE("failed to init io_uring: %s\n", strerror(-ret));
}
open_sync(dfd, fn);
open_io_uring(&ring, dfd, fn);
io_uring_queue_exit(&ring);
close(dfd);
return 0;
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: openat ignores changes to RLIMIT_NOFILE?
2020-03-19 12:12 openat ignores changes to RLIMIT_NOFILE? Dmitry Kadashev
@ 2020-03-20 1:23 ` Jens Axboe
2020-03-20 14:03 ` Dmitry Kadashev
0 siblings, 1 reply; 4+ messages in thread
From: Jens Axboe @ 2020-03-20 1:23 UTC (permalink / raw)
To: Dmitry Kadashev, io-uring
On 3/19/20 6:12 AM, Dmitry Kadashev wrote:
> Hi,
>
> It seems that openat calls issued via io_uring ignore changes to
> RLIMIT_NOFILE. Maybe a wrong limit is checked. A short reproducer is
> attached, it sets RLIMIT_NOFILE to a very low value and the sync
> openat() call fails with "Too many open files", but io_uring one
> succeeds. The resulting FD is completely usable, I've tried writing to
> it successfully.
>
> To be clear, originally I've encountered another side of this problem:
> we increase the limit in our code, and io_uring's openat started to
> fail after a while under load, while the sync calls executed on a
> thread pool were working as expected. It's just easier to demo with
> small limit.
>
> Kernel 5.6-rc2, 5.6-rc6.
>
> Hope it's the right place to report an issue like this.
Can you try the below patch?
diff --git a/fs/file.c b/fs/file.c
index a364e1a9b7e8..c8a4e4c86e55 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -540,9 +540,14 @@ static int alloc_fd(unsigned start, unsigned flags)
return __alloc_fd(current->files, start, rlimit(RLIMIT_NOFILE), flags);
}
+int __get_unused_fd_flags(unsigned flags, unsigned long nofile)
+{
+ return __alloc_fd(current->files, 0, nofile, flags);
+}
+
int get_unused_fd_flags(unsigned flags)
{
- return __alloc_fd(current->files, 0, rlimit(RLIMIT_NOFILE), flags);
+ return __get_unused_fd_flags(flags, rlimit(RLIMIT_NOFILE));
}
EXPORT_SYMBOL(get_unused_fd_flags);
diff --git a/fs/io_uring.c b/fs/io_uring.c
index c06082bb039a..be5705ff33b4 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -398,6 +398,7 @@ struct io_open {
struct filename *filename;
struct statx __user *buffer;
struct open_how how;
+ unsigned long nofile;
};
struct io_files_update {
@@ -2578,6 +2579,7 @@ static int io_openat_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
return ret;
}
+ req->open.nofile = rlimit(RLIMIT_NOFILE);
req->flags |= REQ_F_NEED_CLEANUP;
return 0;
}
@@ -2619,6 +2621,7 @@ static int io_openat2_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
return ret;
}
+ req->open.nofile = rlimit(RLIMIT_NOFILE);
req->flags |= REQ_F_NEED_CLEANUP;
return 0;
}
@@ -2637,7 +2640,7 @@ static int io_openat2(struct io_kiocb *req, struct io_kiocb **nxt,
if (ret)
goto err;
- ret = get_unused_fd_flags(req->open.how.flags);
+ ret = __get_unused_fd_flags(req->open.how.flags, req->open.nofile);
if (ret < 0)
goto err;
diff --git a/include/linux/file.h b/include/linux/file.h
index c6c7b24ea9f7..142d102f285e 100644
--- a/include/linux/file.h
+++ b/include/linux/file.h
@@ -85,6 +85,7 @@ extern int f_dupfd(unsigned int from, struct file *file, unsigned flags);
extern int replace_fd(unsigned fd, struct file *file, unsigned flags);
extern void set_close_on_exec(unsigned int fd, int flag);
extern bool get_close_on_exec(unsigned int fd);
+extern int __get_unused_fd_flags(unsigned flags, unsigned long nofile);
extern int get_unused_fd_flags(unsigned flags);
extern void put_unused_fd(unsigned int fd);
--
Jens Axboe
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: openat ignores changes to RLIMIT_NOFILE?
2020-03-20 1:23 ` Jens Axboe
@ 2020-03-20 14:03 ` Dmitry Kadashev
2020-03-20 14:47 ` Jens Axboe
0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Kadashev @ 2020-03-20 14:03 UTC (permalink / raw)
To: Jens Axboe; +Cc: io-uring
Hi Jens,
Yes, with the patch it works perfectly, thank you.
--
Dmitry
On Fri, Mar 20, 2020 at 8:23 AM Jens Axboe <[email protected]> wrote:
>
> On 3/19/20 6:12 AM, Dmitry Kadashev wrote:
> > Hi,
> >
> > It seems that openat calls issued via io_uring ignore changes to
> > RLIMIT_NOFILE. Maybe a wrong limit is checked. A short reproducer is
> > attached, it sets RLIMIT_NOFILE to a very low value and the sync
> > openat() call fails with "Too many open files", but io_uring one
> > succeeds. The resulting FD is completely usable, I've tried writing to
> > it successfully.
> >
> > To be clear, originally I've encountered another side of this problem:
> > we increase the limit in our code, and io_uring's openat started to
> > fail after a while under load, while the sync calls executed on a
> > thread pool were working as expected. It's just easier to demo with
> > small limit.
> >
> > Kernel 5.6-rc2, 5.6-rc6.
> >
> > Hope it's the right place to report an issue like this.
>
> Can you try the below patch?
>
>
> diff --git a/fs/file.c b/fs/file.c
> index a364e1a9b7e8..c8a4e4c86e55 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -540,9 +540,14 @@ static int alloc_fd(unsigned start, unsigned flags)
> return __alloc_fd(current->files, start, rlimit(RLIMIT_NOFILE), flags);
> }
>
> +int __get_unused_fd_flags(unsigned flags, unsigned long nofile)
> +{
> + return __alloc_fd(current->files, 0, nofile, flags);
> +}
> +
> int get_unused_fd_flags(unsigned flags)
> {
> - return __alloc_fd(current->files, 0, rlimit(RLIMIT_NOFILE), flags);
> + return __get_unused_fd_flags(flags, rlimit(RLIMIT_NOFILE));
> }
> EXPORT_SYMBOL(get_unused_fd_flags);
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index c06082bb039a..be5705ff33b4 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -398,6 +398,7 @@ struct io_open {
> struct filename *filename;
> struct statx __user *buffer;
> struct open_how how;
> + unsigned long nofile;
> };
>
> struct io_files_update {
> @@ -2578,6 +2579,7 @@ static int io_openat_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
> return ret;
> }
>
> + req->open.nofile = rlimit(RLIMIT_NOFILE);
> req->flags |= REQ_F_NEED_CLEANUP;
> return 0;
> }
> @@ -2619,6 +2621,7 @@ static int io_openat2_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
> return ret;
> }
>
> + req->open.nofile = rlimit(RLIMIT_NOFILE);
> req->flags |= REQ_F_NEED_CLEANUP;
> return 0;
> }
> @@ -2637,7 +2640,7 @@ static int io_openat2(struct io_kiocb *req, struct io_kiocb **nxt,
> if (ret)
> goto err;
>
> - ret = get_unused_fd_flags(req->open.how.flags);
> + ret = __get_unused_fd_flags(req->open.how.flags, req->open.nofile);
> if (ret < 0)
> goto err;
>
> diff --git a/include/linux/file.h b/include/linux/file.h
> index c6c7b24ea9f7..142d102f285e 100644
> --- a/include/linux/file.h
> +++ b/include/linux/file.h
> @@ -85,6 +85,7 @@ extern int f_dupfd(unsigned int from, struct file *file, unsigned flags);
> extern int replace_fd(unsigned fd, struct file *file, unsigned flags);
> extern void set_close_on_exec(unsigned int fd, int flag);
> extern bool get_close_on_exec(unsigned int fd);
> +extern int __get_unused_fd_flags(unsigned flags, unsigned long nofile);
> extern int get_unused_fd_flags(unsigned flags);
> extern void put_unused_fd(unsigned int fd);
>
> --
> Jens Axboe
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-20 14:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-19 12:12 openat ignores changes to RLIMIT_NOFILE? Dmitry Kadashev
2020-03-20 1:23 ` Jens Axboe
2020-03-20 14:03 ` Dmitry Kadashev
2020-03-20 14:47 ` Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox