* Re: [PATCH 13/18] io_uring: add file set registration
From: Jens Axboe @ 2019-02-12 17:33 UTC (permalink / raw)
To: Alan Jenkins, linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro
In-Reply-To: <6c7ea7da-ab56-f1b8-2399-c0579b4eceec@gmail.com>
On 2/12/19 10:21 AM, Alan Jenkins wrote:
> On 12/02/2019 15:17, Jens Axboe wrote:
>> On 2/12/19 5:29 AM, Alan Jenkins wrote:
>>> On 08/02/2019 15:13, Jens Axboe wrote:
>>>> On 2/8/19 7:02 AM, Alan Jenkins wrote:
>>>>> On 08/02/2019 12:57, Jens Axboe wrote:
>>>>>> On 2/8/19 5:17 AM, Alan Jenkins wrote:
>>>>>>>> +static int io_sqe_files_scm(struct io_ring_ctx *ctx)
>>>>>>>> +{
>>>>>>>> +#if defined(CONFIG_NET)
>>>>>>>> + struct scm_fp_list *fpl = ctx->user_files;
>>>>>>>> + struct sk_buff *skb;
>>>>>>>> + int i;
>>>>>>>> +
>>>>>>>> + skb = __alloc_skb(0, GFP_KERNEL, 0, NUMA_NO_NODE);
>>>>>>>> + if (!skb)
>>>>>>>> + return -ENOMEM;
>>>>>>>> +
>>>>>>>> + skb->sk = ctx->ring_sock->sk;
>>>>>>>> + skb->destructor = unix_destruct_scm;
>>>>>>>> +
>>>>>>>> + fpl->user = get_uid(ctx->user);
>>>>>>>> + for (i = 0; i < fpl->count; i++) {
>>>>>>>> + get_file(fpl->fp[i]);
>>>>>>>> + unix_inflight(fpl->user, fpl->fp[i]);
>>>>>>>> + fput(fpl->fp[i]);
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> + UNIXCB(skb).fp = fpl;
>>>>>>>> + skb_queue_head(&ctx->ring_sock->sk->sk_receive_queue, skb);
>>>>>>> This code sounds elegant if you know about the existence of unix_gc(),
>>>>>>> but quite mysterious if you don't. (E.g. why "inflight"?) Could we
>>>>>>> have a brief comment, to comfort mortal readers on their journey?
>>>>>>>
>>>>>>> /* A message on a unix socket can hold a reference to a file. This can
>>>>>>> cause a reference cycle. So there is a garbage collector for unix
>>>>>>> sockets, which we hook into here. */
>>>>>> Yes that's a good idea, I've added a comment as to why we go through the
>>>>>> trouble of doing this socket + skb dance.
>>>>> Great, thanks.
>>>>>
>>>>>>> I think this is bypassing too_many_unix_fds() though? I understood that
>>>>>>> was intended to bound kernel memory allocation, at least in principle.
>>>>>> As the code stands above, it'll cap it at 253. I'm just now reworking it
>>>>>> to NOT be limited to the SCM max fd count, but still impose a limit of
>>>>>> 1024 on the number of registered files. This is important to cap the
>>>>>> memory allocation attempt as well.
>>>>> I saw you were limiting to SCM_MAX_FD per io_uring. On the other hand,
>>>>> there's no specific limit on the number of io_urings you can open (only
>>>>> the standard limits on fds). So this would let you allocate hundreds of
>>>>> times more files than the previous limit RLIMIT_NOFILE...
>>>> But there is, the io_uring itself is under the memlock rlimit.
>>>>
>>>>> static inline bool too_many_unix_fds(struct task_struct *p)
>>>>> {
>>>>> struct user_struct *user = current_user();
>>>>>
>>>>> if (unlikely(user->unix_inflight > task_rlimit(p, RLIMIT_NOFILE)))
>>>>> return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
>>>>> return false;
>>>>> }
>>>>>
>>>>> RLIMIT_NOFILE is technically per-task, but here it is capping
>>>>> unix_inflight per-user. So the way I look at this, the number of file
>>>>> descriptors per user is bounded by NOFILE * NPROC. Then
>>>>> user->unix_inflight can have one additional process' worth (NOFILE) of
>>>>> "inflight" files. (Plus SCM_MAX_FD slop, because too_many_fds() is only
>>>>> called once per SCM_RIGHTS).
>>>>>
>>>>> Because io_uring doesn't check too_many_unix_fds(), I think it will let
>>>>> you have about 253 (or 1024) more process' worth of open files. That
>>>>> could be big proportionally when RLIMIT_NPROC is low.
>>>>>
>>>>> I don't know if it matters. It maybe reads like an oversight though.
>>>>>
>>>>> (If it does matter, it might be cleanest to change too_many_unix_fds()
>>>>> to get rid of the "slop". Since that may be different between af_unix
>>>>> and io_uring; 253 v.s. 1024 or whatever. E.g. add a parameter for the
>>>>> number of inflight files we want to add.)
>>>> I don't think it matters. The files in the fixed file set have already
>>>> been opened by the application, so it counts towards the number of open
>>>> files that is allowed to have. I don't think we should impose further
>>>> limits on top of that.
>>> A process can open one io_uring and 199 other files. Register the 199
>>> files in the io_uring, then close their file descriptors. The main
>>> NOFILE limit only counts file descriptors. So then you can open one
>>> io_uring, 198 other files, and repeat.
>>>
>>> You're right, I had forgotten the memlock limit on io_uring. That makes
>>> it much less of a practical problem.
>>>
>>> But it raises a second point. It's not just that it lets users allocate
>>> more files. You might not want to be limited by user->unix_inflight.
>>> But you are calling unix_inflight(), which increments it! Then if
>>> unix->inflight exceeds the NOFILE limit, you will avoid seeing any
>>> errors with io_uring, but the user will not be able to send files over
>>> unix sockets.
>>>
>>> So I think this is confusing to read, and confusing to troubleshoot if
>>> the limit is ever hit.
>>>
>>> I would be happy if io_uring didn't increment user->unix_inflight. I'm
>>> not sure what the best way is to arrange that.
>> How about we just do something like the below? I think that's the saner
>> approach, rather than bypass user->unix_inflight. It's literally the
>> same thing.
>>
>>
>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>> index a4973af1c272..5196b3aa935e 100644
>> --- a/fs/io_uring.c
>> +++ b/fs/io_uring.c
>> @@ -2041,6 +2041,13 @@ static int __io_sqe_files_scm(struct io_ring_ctx *ctx, int nr, int offset)
>> struct sk_buff *skb;
>> int i;
>>
>> + if (!capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN)) {
>> + struct user_struct *user = ctx->user;
>> +
>> + if (user->unix_inflight > task_rlimit(current, RLIMIT_NOFILE))
>> + return -EMFILE;
>> + }
>> +
>> fpl = kzalloc(sizeof(*fpl), GFP_KERNEL);
>> if (!fpl)
>> return -ENOMEM;
>>
>>
>
> Welp, you gave me exactly what I asked for. So now I'd better be
> positive about it :-D.
;-)
> I hope this will be documented accurately, at least where the EMFILE
> result is explained for this syscall.
How's this:
http://git.kernel.dk/cgit/liburing/commit/?id=37e48698a09aa1e37690f8fa6dfd8da69a48ee60
> Because EMFILE is different from the errno in af_unix.c, I will add a
> wish for the existing documentation of ETOOMANYREFS in unix(7) to
> reference this.
>
> I'll stop bikeshedding there. EMFILE sounds ok. strerror() calls
> ETOOMANYREFS "Too many references: cannot splice"; it doesn't seem to be
> particularly helpful or well-known.
Agree
--
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 13/18] io_uring: add file set registration
From: Alan Jenkins @ 2019-02-12 17:21 UTC (permalink / raw)
To: Jens Axboe, linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro
In-Reply-To: <1f1dc8b8-ad8f-16d1-c688-0ab1ce2df378@kernel.dk>
On 12/02/2019 15:17, Jens Axboe wrote:
> On 2/12/19 5:29 AM, Alan Jenkins wrote:
>> On 08/02/2019 15:13, Jens Axboe wrote:
>>> On 2/8/19 7:02 AM, Alan Jenkins wrote:
>>>> On 08/02/2019 12:57, Jens Axboe wrote:
>>>>> On 2/8/19 5:17 AM, Alan Jenkins wrote:
>>>>>>> +static int io_sqe_files_scm(struct io_ring_ctx *ctx)
>>>>>>> +{
>>>>>>> +#if defined(CONFIG_NET)
>>>>>>> + struct scm_fp_list *fpl = ctx->user_files;
>>>>>>> + struct sk_buff *skb;
>>>>>>> + int i;
>>>>>>> +
>>>>>>> + skb = __alloc_skb(0, GFP_KERNEL, 0, NUMA_NO_NODE);
>>>>>>> + if (!skb)
>>>>>>> + return -ENOMEM;
>>>>>>> +
>>>>>>> + skb->sk = ctx->ring_sock->sk;
>>>>>>> + skb->destructor = unix_destruct_scm;
>>>>>>> +
>>>>>>> + fpl->user = get_uid(ctx->user);
>>>>>>> + for (i = 0; i < fpl->count; i++) {
>>>>>>> + get_file(fpl->fp[i]);
>>>>>>> + unix_inflight(fpl->user, fpl->fp[i]);
>>>>>>> + fput(fpl->fp[i]);
>>>>>>> + }
>>>>>>> +
>>>>>>> + UNIXCB(skb).fp = fpl;
>>>>>>> + skb_queue_head(&ctx->ring_sock->sk->sk_receive_queue, skb);
>>>>>> This code sounds elegant if you know about the existence of unix_gc(),
>>>>>> but quite mysterious if you don't. (E.g. why "inflight"?) Could we
>>>>>> have a brief comment, to comfort mortal readers on their journey?
>>>>>>
>>>>>> /* A message on a unix socket can hold a reference to a file. This can
>>>>>> cause a reference cycle. So there is a garbage collector for unix
>>>>>> sockets, which we hook into here. */
>>>>> Yes that's a good idea, I've added a comment as to why we go through the
>>>>> trouble of doing this socket + skb dance.
>>>> Great, thanks.
>>>>
>>>>>> I think this is bypassing too_many_unix_fds() though? I understood that
>>>>>> was intended to bound kernel memory allocation, at least in principle.
>>>>> As the code stands above, it'll cap it at 253. I'm just now reworking it
>>>>> to NOT be limited to the SCM max fd count, but still impose a limit of
>>>>> 1024 on the number of registered files. This is important to cap the
>>>>> memory allocation attempt as well.
>>>> I saw you were limiting to SCM_MAX_FD per io_uring. On the other hand,
>>>> there's no specific limit on the number of io_urings you can open (only
>>>> the standard limits on fds). So this would let you allocate hundreds of
>>>> times more files than the previous limit RLIMIT_NOFILE...
>>> But there is, the io_uring itself is under the memlock rlimit.
>>>
>>>> static inline bool too_many_unix_fds(struct task_struct *p)
>>>> {
>>>> struct user_struct *user = current_user();
>>>>
>>>> if (unlikely(user->unix_inflight > task_rlimit(p, RLIMIT_NOFILE)))
>>>> return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
>>>> return false;
>>>> }
>>>>
>>>> RLIMIT_NOFILE is technically per-task, but here it is capping
>>>> unix_inflight per-user. So the way I look at this, the number of file
>>>> descriptors per user is bounded by NOFILE * NPROC. Then
>>>> user->unix_inflight can have one additional process' worth (NOFILE) of
>>>> "inflight" files. (Plus SCM_MAX_FD slop, because too_many_fds() is only
>>>> called once per SCM_RIGHTS).
>>>>
>>>> Because io_uring doesn't check too_many_unix_fds(), I think it will let
>>>> you have about 253 (or 1024) more process' worth of open files. That
>>>> could be big proportionally when RLIMIT_NPROC is low.
>>>>
>>>> I don't know if it matters. It maybe reads like an oversight though.
>>>>
>>>> (If it does matter, it might be cleanest to change too_many_unix_fds()
>>>> to get rid of the "slop". Since that may be different between af_unix
>>>> and io_uring; 253 v.s. 1024 or whatever. E.g. add a parameter for the
>>>> number of inflight files we want to add.)
>>> I don't think it matters. The files in the fixed file set have already
>>> been opened by the application, so it counts towards the number of open
>>> files that is allowed to have. I don't think we should impose further
>>> limits on top of that.
>> A process can open one io_uring and 199 other files. Register the 199
>> files in the io_uring, then close their file descriptors. The main
>> NOFILE limit only counts file descriptors. So then you can open one
>> io_uring, 198 other files, and repeat.
>>
>> You're right, I had forgotten the memlock limit on io_uring. That makes
>> it much less of a practical problem.
>>
>> But it raises a second point. It's not just that it lets users allocate
>> more files. You might not want to be limited by user->unix_inflight.
>> But you are calling unix_inflight(), which increments it! Then if
>> unix->inflight exceeds the NOFILE limit, you will avoid seeing any
>> errors with io_uring, but the user will not be able to send files over
>> unix sockets.
>>
>> So I think this is confusing to read, and confusing to troubleshoot if
>> the limit is ever hit.
>>
>> I would be happy if io_uring didn't increment user->unix_inflight. I'm
>> not sure what the best way is to arrange that.
> How about we just do something like the below? I think that's the saner
> approach, rather than bypass user->unix_inflight. It's literally the
> same thing.
>
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index a4973af1c272..5196b3aa935e 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -2041,6 +2041,13 @@ static int __io_sqe_files_scm(struct io_ring_ctx *ctx, int nr, int offset)
> struct sk_buff *skb;
> int i;
>
> + if (!capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN)) {
> + struct user_struct *user = ctx->user;
> +
> + if (user->unix_inflight > task_rlimit(current, RLIMIT_NOFILE))
> + return -EMFILE;
> + }
> +
> fpl = kzalloc(sizeof(*fpl), GFP_KERNEL);
> if (!fpl)
> return -ENOMEM;
>
>
Welp, you gave me exactly what I asked for. So now I'd better be
positive about it :-D.
I hope this will be documented accurately, at least where the EMFILE
result is explained for this syscall.
Because EMFILE is different from the errno in af_unix.c, I will add a
wish for the existing documentation of ETOOMANYREFS in unix(7) to
reference this.
I'll stop bikeshedding there. EMFILE sounds ok. strerror() calls
ETOOMANYREFS "Too many references: cannot splice"; it doesn't seem to be
particularly helpful or well-known.
Thanks
Alan
--
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 2/3] mm/filemap: initiate readahead even if IOCB_NOWAIT is set for the I/O
From: Jiri Kosina @ 2019-02-12 15:48 UTC (permalink / raw)
To: Dave Chinner
Cc: Michal Hocko, Vlastimil Babka, Andrew Morton, Linus Torvalds,
linux-kernel, linux-mm, linux-api, Peter Zijlstra, Greg KH,
Jann Horn, Dominique Martinet, Andy Lutomirski, Kevin Easton,
Matthew Wilcox, Cyril Hrubis, Tejun Heo, Kirill A . Shutemov,
Daniel Gruss, linux-fsdevel
In-Reply-To: <20190201014446.GU6173@dastard>
On Fri, 1 Feb 2019, Dave Chinner wrote:
> So, I'll invite the incoherent, incandescent O_DIRECT rage flames of
> Linus to be unleashed again and point out the /other reference/ to
> IOCB_NOWAIT in mm/filemap.c. That is, in generic_file_read_iter(),
> in the *generic O_DIRECT read path*:
>
> if (iocb->ki_flags & IOCB_DIRECT) {
> .....
> if (iocb->ki_flags & IOCB_NOWAIT) {
> if (filemap_range_has_page(mapping, iocb->ki_pos,
> iocb->ki_pos + count - 1))
> return -EAGAIN;
> } else {
> .....
OK, thanks Dave, this is a good point I've missed in this mail before
(probabably as I focused only on the aspect of disagreement what NONBLOCK
actually means :) ). I will look into fixing it for next iteration.
> It's effectively useless as a workaround because you can avoid the
> readahead IO being issued relatively easily:
>
> void page_cache_sync_readahead(struct address_space *mapping,
> struct file_ra_state *ra, struct file *filp,
> pgoff_t offset, unsigned long req_size)
> {
> /* no read-ahead */
> if (!ra->ra_pages)
> return;
>
> if (blk_cgroup_congested())
> return;
> ....
>
> IOWs, we just have to issue enough IO to congest the block device (or,
> even easier, a rate-limited cgroup), and we can still use RWF_NOWAIT
> to probe the page cache. Or if we can convince ra->ra_pages to be
> zero (e.g. it's on bdi device with no readahead configured because
> it's real fast) then it doesn't work there, either.
It's though questionable whether the noise level here wouldn't be too high
already for any sidechannel to work reliably. So I'd suggest to operate
under the assumption that it would be too noisy, unless anyone is able to
prove otherwise.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 13/18] io_uring: add file set registration
From: Jens Axboe @ 2019-02-12 15:17 UTC (permalink / raw)
To: Alan Jenkins, linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro
In-Reply-To: <c2ef6663-3e15-d071-56e3-4afba7b2d778@gmail.com>
On 2/12/19 5:29 AM, Alan Jenkins wrote:
> On 08/02/2019 15:13, Jens Axboe wrote:
>> On 2/8/19 7:02 AM, Alan Jenkins wrote:
>>> On 08/02/2019 12:57, Jens Axboe wrote:
>>>> On 2/8/19 5:17 AM, Alan Jenkins wrote:
>>>>>> +static int io_sqe_files_scm(struct io_ring_ctx *ctx)
>>>>>> +{
>>>>>> +#if defined(CONFIG_NET)
>>>>>> + struct scm_fp_list *fpl = ctx->user_files;
>>>>>> + struct sk_buff *skb;
>>>>>> + int i;
>>>>>> +
>>>>>> + skb = __alloc_skb(0, GFP_KERNEL, 0, NUMA_NO_NODE);
>>>>>> + if (!skb)
>>>>>> + return -ENOMEM;
>>>>>> +
>>>>>> + skb->sk = ctx->ring_sock->sk;
>>>>>> + skb->destructor = unix_destruct_scm;
>>>>>> +
>>>>>> + fpl->user = get_uid(ctx->user);
>>>>>> + for (i = 0; i < fpl->count; i++) {
>>>>>> + get_file(fpl->fp[i]);
>>>>>> + unix_inflight(fpl->user, fpl->fp[i]);
>>>>>> + fput(fpl->fp[i]);
>>>>>> + }
>>>>>> +
>>>>>> + UNIXCB(skb).fp = fpl;
>>>>>> + skb_queue_head(&ctx->ring_sock->sk->sk_receive_queue, skb);
>>>>> This code sounds elegant if you know about the existence of unix_gc(),
>>>>> but quite mysterious if you don't. (E.g. why "inflight"?) Could we
>>>>> have a brief comment, to comfort mortal readers on their journey?
>>>>>
>>>>> /* A message on a unix socket can hold a reference to a file. This can
>>>>> cause a reference cycle. So there is a garbage collector for unix
>>>>> sockets, which we hook into here. */
>>>> Yes that's a good idea, I've added a comment as to why we go through the
>>>> trouble of doing this socket + skb dance.
>>> Great, thanks.
>>>
>>>>> I think this is bypassing too_many_unix_fds() though? I understood that
>>>>> was intended to bound kernel memory allocation, at least in principle.
>>>> As the code stands above, it'll cap it at 253. I'm just now reworking it
>>>> to NOT be limited to the SCM max fd count, but still impose a limit of
>>>> 1024 on the number of registered files. This is important to cap the
>>>> memory allocation attempt as well.
>>> I saw you were limiting to SCM_MAX_FD per io_uring. On the other hand,
>>> there's no specific limit on the number of io_urings you can open (only
>>> the standard limits on fds). So this would let you allocate hundreds of
>>> times more files than the previous limit RLIMIT_NOFILE...
>> But there is, the io_uring itself is under the memlock rlimit.
>>
>>> static inline bool too_many_unix_fds(struct task_struct *p)
>>> {
>>> struct user_struct *user = current_user();
>>>
>>> if (unlikely(user->unix_inflight > task_rlimit(p, RLIMIT_NOFILE)))
>>> return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
>>> return false;
>>> }
>>>
>>> RLIMIT_NOFILE is technically per-task, but here it is capping
>>> unix_inflight per-user. So the way I look at this, the number of file
>>> descriptors per user is bounded by NOFILE * NPROC. Then
>>> user->unix_inflight can have one additional process' worth (NOFILE) of
>>> "inflight" files. (Plus SCM_MAX_FD slop, because too_many_fds() is only
>>> called once per SCM_RIGHTS).
>>>
>>> Because io_uring doesn't check too_many_unix_fds(), I think it will let
>>> you have about 253 (or 1024) more process' worth of open files. That
>>> could be big proportionally when RLIMIT_NPROC is low.
>>>
>>> I don't know if it matters. It maybe reads like an oversight though.
>>>
>>> (If it does matter, it might be cleanest to change too_many_unix_fds()
>>> to get rid of the "slop". Since that may be different between af_unix
>>> and io_uring; 253 v.s. 1024 or whatever. E.g. add a parameter for the
>>> number of inflight files we want to add.)
>> I don't think it matters. The files in the fixed file set have already
>> been opened by the application, so it counts towards the number of open
>> files that is allowed to have. I don't think we should impose further
>> limits on top of that.
>
> A process can open one io_uring and 199 other files. Register the 199
> files in the io_uring, then close their file descriptors. The main
> NOFILE limit only counts file descriptors. So then you can open one
> io_uring, 198 other files, and repeat.
>
> You're right, I had forgotten the memlock limit on io_uring. That makes
> it much less of a practical problem.
>
> But it raises a second point. It's not just that it lets users allocate
> more files. You might not want to be limited by user->unix_inflight.
> But you are calling unix_inflight(), which increments it! Then if
> unix->inflight exceeds the NOFILE limit, you will avoid seeing any
> errors with io_uring, but the user will not be able to send files over
> unix sockets.
>
> So I think this is confusing to read, and confusing to troubleshoot if
> the limit is ever hit.
>
> I would be happy if io_uring didn't increment user->unix_inflight. I'm
> not sure what the best way is to arrange that.
How about we just do something like the below? I think that's the saner
approach, rather than bypass user->unix_inflight. It's literally the
same thing.
diff --git a/fs/io_uring.c b/fs/io_uring.c
index a4973af1c272..5196b3aa935e 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2041,6 +2041,13 @@ static int __io_sqe_files_scm(struct io_ring_ctx *ctx, int nr, int offset)
struct sk_buff *skb;
int i;
+ if (!capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN)) {
+ struct user_struct *user = ctx->user;
+
+ if (user->unix_inflight > task_rlimit(current, RLIMIT_NOFILE))
+ return -EMFILE;
+ }
+
fpl = kzalloc(sizeof(*fpl), GFP_KERNEL);
if (!fpl)
return -ENOMEM;
--
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 related
* Re: [PATCH 3/3] mm/mincore: provide mapped status when cached status is not allowed
From: Michal Hocko @ 2019-02-12 14:01 UTC (permalink / raw)
To: Jiri Kosina
Cc: Vlastimil Babka, Andrew Morton, Linus Torvalds, linux-kernel,
linux-mm, linux-api, Peter Zijlstra, Greg KH, Jann Horn,
Dominique Martinet, Andy Lutomirski, Dave Chinner, Kevin Easton,
Matthew Wilcox, Cyril Hrubis, Tejun Heo, Kirill A . Shutemov,
Daniel Gruss, Josh Snyder
In-Reply-To: <nycvar.YFH.7.76.1902121405440.11598@cbobk.fhfr.pm>
On Tue 12-02-19 14:09:03, Jiri Kosina wrote:
> On Tue, 12 Feb 2019, Michal Hocko wrote:
>
> > I would go with patch 1 for 5.1. Patches 2 still sounds controversial or
> > incomplete to me.
>
> Is it because of the disagreement what 'non-blocking' really means, or do
> you see something else missing?
Not only. See the remark from Dave [1] that the patch in its current
form seems to be incomplete. Also FS people were not involved
properly to evaluate all the potential fallouts. Even if the only way
forward is to "cripple" IOCB_NOWAIT then the documentation should go
along with the change rather than suprise people much later when the
system behaves unexpectedly. So I _think_ this patch is not really ready
yet.
Also I haven't heard any discussion whether we can reduce the effect of
the change in a similar way we do for mincore.
> Merging patch just patch 1 withouth patch 2 is probably sort of useless
> excercise, unfortunately.
Why would that be the case. We know that mincore is the simplest way
_right now_. Closing it makes sense on its own.
[1] http://lkml.kernel.org/r/20190201014446.GU6173@dastard
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH 3/3] mm/mincore: provide mapped status when cached status is not allowed
From: Jiri Kosina @ 2019-02-12 13:09 UTC (permalink / raw)
To: Michal Hocko
Cc: Vlastimil Babka, Andrew Morton, Linus Torvalds, linux-kernel,
linux-mm, linux-api, Peter Zijlstra, Greg KH, Jann Horn,
Dominique Martinet, Andy Lutomirski, Dave Chinner, Kevin Easton,
Matthew Wilcox, Cyril Hrubis, Tejun Heo, Kirill A . Shutemov,
Daniel Gruss, Josh Snyder
In-Reply-To: <20190212063643.GL15609@dhcp22.suse.cz>
On Tue, 12 Feb 2019, Michal Hocko wrote:
> I would go with patch 1 for 5.1. Patches 2 still sounds controversial or
> incomplete to me.
Is it because of the disagreement what 'non-blocking' really means, or do
you see something else missing?
Merging patch just patch 1 withouth patch 2 is probably sort of useless
excercise, unfortunately.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 13/18] io_uring: add file set registration
From: Alan Jenkins @ 2019-02-12 12:29 UTC (permalink / raw)
To: Jens Axboe, linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro
In-Reply-To: <ad669e88-5a33-37e1-6d69-e2b182d89891@kernel.dk>
On 08/02/2019 15:13, Jens Axboe wrote:
> On 2/8/19 7:02 AM, Alan Jenkins wrote:
>> On 08/02/2019 12:57, Jens Axboe wrote:
>>> On 2/8/19 5:17 AM, Alan Jenkins wrote:
>>>>> +static int io_sqe_files_scm(struct io_ring_ctx *ctx)
>>>>> +{
>>>>> +#if defined(CONFIG_NET)
>>>>> + struct scm_fp_list *fpl = ctx->user_files;
>>>>> + struct sk_buff *skb;
>>>>> + int i;
>>>>> +
>>>>> + skb = __alloc_skb(0, GFP_KERNEL, 0, NUMA_NO_NODE);
>>>>> + if (!skb)
>>>>> + return -ENOMEM;
>>>>> +
>>>>> + skb->sk = ctx->ring_sock->sk;
>>>>> + skb->destructor = unix_destruct_scm;
>>>>> +
>>>>> + fpl->user = get_uid(ctx->user);
>>>>> + for (i = 0; i < fpl->count; i++) {
>>>>> + get_file(fpl->fp[i]);
>>>>> + unix_inflight(fpl->user, fpl->fp[i]);
>>>>> + fput(fpl->fp[i]);
>>>>> + }
>>>>> +
>>>>> + UNIXCB(skb).fp = fpl;
>>>>> + skb_queue_head(&ctx->ring_sock->sk->sk_receive_queue, skb);
>>>> This code sounds elegant if you know about the existence of unix_gc(),
>>>> but quite mysterious if you don't. (E.g. why "inflight"?) Could we
>>>> have a brief comment, to comfort mortal readers on their journey?
>>>>
>>>> /* A message on a unix socket can hold a reference to a file. This can
>>>> cause a reference cycle. So there is a garbage collector for unix
>>>> sockets, which we hook into here. */
>>> Yes that's a good idea, I've added a comment as to why we go through the
>>> trouble of doing this socket + skb dance.
>> Great, thanks.
>>
>>>> I think this is bypassing too_many_unix_fds() though? I understood that
>>>> was intended to bound kernel memory allocation, at least in principle.
>>> As the code stands above, it'll cap it at 253. I'm just now reworking it
>>> to NOT be limited to the SCM max fd count, but still impose a limit of
>>> 1024 on the number of registered files. This is important to cap the
>>> memory allocation attempt as well.
>> I saw you were limiting to SCM_MAX_FD per io_uring. On the other hand,
>> there's no specific limit on the number of io_urings you can open (only
>> the standard limits on fds). So this would let you allocate hundreds of
>> times more files than the previous limit RLIMIT_NOFILE...
> But there is, the io_uring itself is under the memlock rlimit.
>
>> static inline bool too_many_unix_fds(struct task_struct *p)
>> {
>> struct user_struct *user = current_user();
>>
>> if (unlikely(user->unix_inflight > task_rlimit(p, RLIMIT_NOFILE)))
>> return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
>> return false;
>> }
>>
>> RLIMIT_NOFILE is technically per-task, but here it is capping
>> unix_inflight per-user. So the way I look at this, the number of file
>> descriptors per user is bounded by NOFILE * NPROC. Then
>> user->unix_inflight can have one additional process' worth (NOFILE) of
>> "inflight" files. (Plus SCM_MAX_FD slop, because too_many_fds() is only
>> called once per SCM_RIGHTS).
>>
>> Because io_uring doesn't check too_many_unix_fds(), I think it will let
>> you have about 253 (or 1024) more process' worth of open files. That
>> could be big proportionally when RLIMIT_NPROC is low.
>>
>> I don't know if it matters. It maybe reads like an oversight though.
>>
>> (If it does matter, it might be cleanest to change too_many_unix_fds()
>> to get rid of the "slop". Since that may be different between af_unix
>> and io_uring; 253 v.s. 1024 or whatever. E.g. add a parameter for the
>> number of inflight files we want to add.)
> I don't think it matters. The files in the fixed file set have already
> been opened by the application, so it counts towards the number of open
> files that is allowed to have. I don't think we should impose further
> limits on top of that.
A process can open one io_uring and 199 other files. Register the 199
files in the io_uring, then close their file descriptors. The main
NOFILE limit only counts file descriptors. So then you can open one
io_uring, 198 other files, and repeat.
You're right, I had forgotten the memlock limit on io_uring. That makes
it much less of a practical problem.
But it raises a second point. It's not just that it lets users allocate
more files. You might not want to be limited by user->unix_inflight.
But you are calling unix_inflight(), which increments it! Then if
unix->inflight exceeds the NOFILE limit, you will avoid seeing any
errors with io_uring, but the user will not be able to send files over
unix sockets.
So I think this is confusing to read, and confusing to troubleshoot if
the limit is ever hit.
I would be happy if io_uring didn't increment user->unix_inflight. I'm
not sure what the best way is to arrange that.
Regards
Alan
--
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] mmap.2: describe the 5level paging hack
From: Kirill A. Shutemov @ 2019-02-12 9:41 UTC (permalink / raw)
To: Jann Horn
Cc: linux-arch, linux-man, Catalin Marinas, Dave Hansen,
Peter Zijlstra, Benjamin Herrenschmidt, Will Deacon, linuxppc-dev,
Andy Lutomirski, linux-mm, Paul Mackerras, mtk.manpages,
Michael Ellerman, Andrew Morton, linux-api, Linus Torvalds,
Thomas Gleixner, Kirill A . Shutemov, linux-arm-kernel
In-Reply-To: <20190211163653.97742-1-jannh@google.com>
On Mon, Feb 11, 2019 at 05:36:53PM +0100, Jann Horn wrote:
> The manpage is missing information about the compatibility hack for
> 5-level paging that went in in 4.14, around commit ee00f4a32a76 ("x86/mm:
> Allow userspace have mappings above 47-bit"). Add some information about
> that.
>
> While I don't think any hardware supporting this is shipping yet (?), I
> think it's useful to try to write a manpage for this API, partly to
> figure out how usable that API actually is, and partly because when this
> hardware does ship, it'd be nice if distro manpages had information about
> how to use it.
>
> Signed-off-by: Jann Horn <jannh@google.com>
Thanks for doing this.
> ---
> This patch goes on top of the patch "[PATCH] mmap.2: fix description of
> treatment of the hint" that I just sent, but I'm not sending them in a
> series because I want the first one to go in, and I think this one might
> be a bit more controversial.
>
> It would be nice if the architecture maintainers and mm folks could have
> a look at this and check that what I wrote is right - I only looked at
> the source for this, I haven't tried it.
>
> man2/mmap.2 | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/man2/mmap.2 b/man2/mmap.2
> index 8556bbfeb..977782fa8 100644
> --- a/man2/mmap.2
> +++ b/man2/mmap.2
> @@ -67,6 +67,8 @@ is NULL,
> then the kernel chooses the (page-aligned) address
> at which to create the mapping;
> this is the most portable method of creating a new mapping.
> +On Linux, in this case, the kernel may limit the maximum address that can be
> +used for allocations to a legacy limit for compatibility reasons.
> If
> .I addr
> is not NULL,
> @@ -77,6 +79,19 @@ or equal to the value specified by
> and attempt to create the mapping there.
> If another mapping already exists there, the kernel picks a new
> address, independent of the hint.
> +However, if a hint above the architecture's legacy address limit is provided
> +(on x86-64: above 0x7ffffffff000, on arm64: above 0x1000000000000, on ppc64 with
> +book3s: above 0x7fffffffffff or 0x3fffffffffff, depending on page size), the
> +kernel is permitted to allocate mappings beyond the architecture's legacy
> +address limit. The availability of such addresses is hardware-dependent.
> +Therefore, if you want to be able to use the full virtual address space of
> +hardware that supports addresses beyond the legacy range, you need to specify an
> +address above that limit; however, for security reasons, you should avoid
> +specifying a fixed valid address outside the compatibility range,
> +since that would reduce the value of userspace address space layout
> +randomization. Therefore, it is recommended to specify an address
> +.I beyond
> +the end of the userspace address space.
It probably worth recommending (void *) -1 as such address.
> .\" Before Linux 2.6.24, the address was rounded up to the next page
> .\" boundary; since 2.6.24, it is rounded down!
> The address of the new mapping is returned as the result of the call.
> --
> 2.20.1.791.gb4d0f1c61a-goog
>
--
Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH 3/3] mm/mincore: provide mapped status when cached status is not allowed
From: Michal Hocko @ 2019-02-12 6:36 UTC (permalink / raw)
To: Jiri Kosina
Cc: Vlastimil Babka, Andrew Morton, Linus Torvalds, linux-kernel,
linux-mm, linux-api, Peter Zijlstra, Greg KH, Jann Horn,
Dominique Martinet, Andy Lutomirski, Dave Chinner, Kevin Easton,
Matthew Wilcox, Cyril Hrubis, Tejun Heo, Kirill A . Shutemov,
Daniel Gruss, Josh Snyder
In-Reply-To: <nycvar.YFH.7.76.1902120440430.11598@cbobk.fhfr.pm>
On Tue 12-02-19 04:44:30, Jiri Kosina wrote:
> On Fri, 1 Feb 2019, Vlastimil Babka wrote:
>
> > >> After "mm/mincore: make mincore() more conservative" we sometimes restrict the
> > >> information about page cache residency, which we have to do without breaking
> > >> existing userspace, if possible. We thus fake the resulting values as 1, which
> > >> should be safer than faking them as 0, as there might theoretically exist code
> > >> that would try to fault in the page(s) until mincore() returns 1.
> > >>
> > >> Faking 1 however means that such code would not fault in a page even if it was
> > >> not in page cache, with unwanted performance implications. We can improve the
> > >> situation by revisting the approach of 574823bfab82 ("Change mincore() to count
> > >> "mapped" pages rather than "cached" pages") but only applying it to cases where
> > >> page cache residency check is restricted. Thus mincore() will return 0 for an
> > >> unmapped page (which may or may not be resident in a pagecache), and 1 after
> > >> the process faults it in.
> > >>
> > >> One potential downside is that mincore() will be again able to recognize when a
> > >> previously mapped page was reclaimed. While that might be useful for some
> > >> attack scenarios, it's not as crucial as recognizing that somebody else faulted
> > >> the page in, and there are also other ways to recognize reclaimed pages anyway.
> > >
> > > Is this really worth it? Do we know about any specific usecase that
> > > would benefit from this change? TBH I would rather wait for the report
> > > than add a hard to evaluate side channel.
> >
> > Well it's not that complicated IMHO. Linus said it's worth trying, so
> > let's see how he likes the result. The side channel exists anyway as
> > long as process can e.g. check if its rss shrinked, and I doubt we are
> > going to remove that possibility.
>
> So, where do we go from here?
>
> Either Linus and Andrew like the mincore() return value tweak, or this
> could be further discussed (*). But in either of the cases, I think
> patches 1 and 2 should be at least queued for 5.1.
I would go with patch 1 for 5.1. Patches 2 still sounds controversial or
incomplete to me. And patch 3, well I will leave the decision to
Andrew/Linus.
> (*) I'd personally include it as well, as I don't see how it would break
> anything, it's pretty straightforward, and brings back some sanity to
> mincore() return value.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH 3/3] mm/mincore: provide mapped status when cached status is not allowed
From: Jiri Kosina @ 2019-02-12 3:44 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Michal Hocko, Andrew Morton, Linus Torvalds, linux-kernel,
linux-mm, linux-api, Peter Zijlstra, Greg KH, Jann Horn,
Dominique Martinet, Andy Lutomirski, Dave Chinner, Kevin Easton,
Matthew Wilcox, Cyril Hrubis, Tejun Heo, Kirill A . Shutemov,
Daniel Gruss, Josh Snyder
In-Reply-To: <99ee4d3e-aeb2-0104-22be-b028938e7f88@suse.cz>
On Fri, 1 Feb 2019, Vlastimil Babka wrote:
> >> After "mm/mincore: make mincore() more conservative" we sometimes restrict the
> >> information about page cache residency, which we have to do without breaking
> >> existing userspace, if possible. We thus fake the resulting values as 1, which
> >> should be safer than faking them as 0, as there might theoretically exist code
> >> that would try to fault in the page(s) until mincore() returns 1.
> >>
> >> Faking 1 however means that such code would not fault in a page even if it was
> >> not in page cache, with unwanted performance implications. We can improve the
> >> situation by revisting the approach of 574823bfab82 ("Change mincore() to count
> >> "mapped" pages rather than "cached" pages") but only applying it to cases where
> >> page cache residency check is restricted. Thus mincore() will return 0 for an
> >> unmapped page (which may or may not be resident in a pagecache), and 1 after
> >> the process faults it in.
> >>
> >> One potential downside is that mincore() will be again able to recognize when a
> >> previously mapped page was reclaimed. While that might be useful for some
> >> attack scenarios, it's not as crucial as recognizing that somebody else faulted
> >> the page in, and there are also other ways to recognize reclaimed pages anyway.
> >
> > Is this really worth it? Do we know about any specific usecase that
> > would benefit from this change? TBH I would rather wait for the report
> > than add a hard to evaluate side channel.
>
> Well it's not that complicated IMHO. Linus said it's worth trying, so
> let's see how he likes the result. The side channel exists anyway as
> long as process can e.g. check if its rss shrinked, and I doubt we are
> going to remove that possibility.
So, where do we go from here?
Either Linus and Andrew like the mincore() return value tweak, or this
could be further discussed (*). But in either of the cases, I think
patches 1 and 2 should be at least queued for 5.1.
(*) I'd personally include it as well, as I don't see how it would break
anything, it's pretty straightforward, and brings back some sanity to
mincore() return value.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [RFC PATCH v2 11/20] fscrypt: add FS_IOC_REMOVE_ENCRYPTION_KEY ioctl
From: Dave Chinner @ 2019-02-12 0:03 UTC (permalink / raw)
To: Eric Biggers
Cc: Satya Tangirala, linux-api, linux-f2fs-devel, linux-fscrypt,
keyrings, linux-mtd, linux-crypto, linux-fsdevel, linux-ext4,
Paul Crowley
In-Reply-To: <20190211233128.GB226227@gmail.com>
On Mon, Feb 11, 2019 at 03:31:29PM -0800, Eric Biggers wrote:
> Hi Dave,
>
> On Tue, Feb 12, 2019 at 09:12:49AM +1100, Dave Chinner wrote:
> > On Mon, Feb 11, 2019 at 09:27:29AM -0800, Eric Biggers wrote:
> >
> > Indeed, this is exactly what ->drop_inode() is for.
> >
> > Take this function:
> >
> > > +static void evict_dentries_for_decrypted_inodes(struct fscrypt_master_key *mk)
> > > +{
> > > + struct fscrypt_info *ci;
> > > + struct inode *inode;
> > > + struct inode *toput_inode = NULL;
> > > +
> > > + spin_lock(&mk->mk_decrypted_inodes_lock);
> > > +
> > > + list_for_each_entry(ci, &mk->mk_decrypted_inodes, ci_master_key_link) {
> > > + inode = ci->ci_inode;
> > > + spin_lock(&inode->i_lock);
> > > + if (inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW)) {
> > > + spin_unlock(&inode->i_lock);
> > > + continue;
> > > + }
> > > + __iget(inode);
> > > + spin_unlock(&inode->i_lock);
> > > + spin_unlock(&mk->mk_decrypted_inodes_lock);
> > > +
> > > + shrink_dcache_inode(inode);
> > > + iput(toput_inode);
> > > + toput_inode = inode;
> > > +
> > > + spin_lock(&mk->mk_decrypted_inodes_lock);
> > > + }
> > > +
> > > + spin_unlock(&mk->mk_decrypted_inodes_lock);
> > > + iput(toput_inode);
> > > +}
> >
> > It takes a new reference to each decrypted inode, and then drops it
> > again after all the dentry cache references have been killed and
> > we've got a reference to the next inode in the list. Killing the
> > dentry references to the inode means it should only have in-use
> > references and the reference this function holds on it.
> >
> > If the inode is not in use then there will be only one, and so it
> > will fall into iput_final() and the ->drop_inode() function
> > determines if the inode should be evicted from the cache and
> > destroyed immediately. IOWs, implement fscrypt_drop_inode() to do
> > the right thing when the key has been destroyed, and you can get rid
> > of all this crazy inode cache walk-and-invalidate hackery.
> >
>
> Thanks for the feedback! If I understand correctly, your suggestion is:
>
> - Keep evict_dentries_for_decrypted_inodes() as-is, i.e. fscrypt would still
> evict the dentries for all inodes in ->mk_decrypted_inodes.
> (I don't see how it could work otherwise.)
>
> - However, evict_decrypted_inodes() would be removed and fscrypt would not
> directly evict the list of inodes. Instead, the filesystem's ->drop_inode()
> would be made to return 1 if the inode's master key has been removed. Thus
> each inode, if no longer in use, would end up getting evicted during the
> iput() in evict_dentries_for_decrypted_inodes().
*nod*
> I hadn't thought of this, and I think it would work; I'll try implementing it.
> It would also have the advantage that if a key is removed while an inode is
> still in-use, that inode will be evicted as soon as it's no longer in use rather
> than waiting around until another FS_IOC_REMOVE_ENCRYPTION_KEY.
*nod*
> The ioctl will need a different way to determine whether any inodes couldn't be
> evicted, but simply checking whether ->mk_decrypted_inodes ended up empty or not
> should work.
*nod*
> FWIW, originally I also considered leaving the inodes in the inode cache and
> instead only freeing ->i_crypt_info and truncating the pagecache. But I don't
> see a way to do it even with this new idea; for one, ->drop_inode() is called
> under ->i_lock. So it seems that eviction is still the way to go.
Yeah, eviction is by far the easiest way to deal with this. If it's
being frequently referenced/written, the backing buffer should be in
memory anyway and the next access simply has to re-instantiate the
inode cache from the buffer and won't need to do IO.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply
* Re: [RFC PATCH v2 11/20] fscrypt: add FS_IOC_REMOVE_ENCRYPTION_KEY ioctl
From: Eric Biggers @ 2019-02-11 23:31 UTC (permalink / raw)
To: Dave Chinner
Cc: Satya Tangirala, linux-api, linux-f2fs-devel, linux-fscrypt,
keyrings, linux-mtd, linux-crypto, linux-fsdevel, linux-ext4,
Paul Crowley
In-Reply-To: <20190211221249.GH20493@dastard>
Hi Dave,
On Tue, Feb 12, 2019 at 09:12:49AM +1100, Dave Chinner wrote:
> On Mon, Feb 11, 2019 at 09:27:29AM -0800, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> >
> > Add a new fscrypt ioctl, FS_IOC_REMOVE_ENCRYPTION_KEY. This ioctl
> > removes an encryption key that was added by FS_IOC_ADD_ENCRYPTION_KEY.
> > It wipes the secret key itself, then "locks" the encrypted files and
> > directories that had been unlocked using that key -- implemented by
> > evicting the relevant dentries and inodes from the VFS caches.
> >
> > The problem this solves is that many fscrypt users want the ability to
> > remove encryption keys, causing the corresponding encrypted directories
> > to appear "locked" (presented in ciphertext form) again. Moreover,
> > users want removing an encryption key to *really* remove it, in the
> > sense that the removed keys cannot be recovered even if kernel memory is
> > compromised, e.g. by the exploit of a kernel security vulnerability or
> > by a physical attack. This is desirable after a user logs out of the
> > system, for example. In many cases users even already assume this to be
> > the case and are surprised to hear when it's not.
> >
> > It is not sufficient to simply unlink the master key from the keyring
> > (or to revoke or invalidate it), since the actual encryption transform
> > objects are still pinned in memory by their inodes. Therefore, to
> > really remove a key we must also evict the relevant inodes.
> >
> > Currently one workaround is to run 'sync && echo 2 >
> > /proc/sys/vm/drop_caches'. But, that evicts all unused inodes in the
> > system rather than just the inodes associated with the key being
> > removed, causing severe performance problems. Moreover, it requires
> > root privileges, so regular users can't "lock" their encrypted files.
> >
> > Another workaround, used in Chromium OS kernels, is to add a new
> > VFS-level ioctl FS_IOC_DROP_CACHE which is a more restricted version of
> > drop_caches that operates on a single super_block. It does:
> >
> > shrink_dcache_sb(sb);
> > invalidate_inodes(sb, false);
> >
> > But it's still a hack. Yet, the major users of filesystem encryption
> > want this feature badly enough that they are actually using these hacks.
> >
> > To properly solve the problem, start maintaining a list of the inodes
> > which have been "unlocked" using each master key. Originally this
> > wasn't possible because the kernel didn't keep track of in-use master
> > keys at all. But, with the ->s_master_keys keyring it is now possible.
> >
> > Then, add an ioctl FS_IOC_REMOVE_ENCRYPTION_KEY. It finds the specified
> > master key in ->s_master_keys, then wipes the secret key itself, which
> > prevents any additional inodes from being unlocked with the key. Then,
> > it syncs the filesystem and evicts the inodes in the key's list. The
> > normal inode eviction code will free and wipe the per-file keys (in
> > ->i_crypt_info). Note that freeing ->i_crypt_info without evicting the
> > inodes was also considered, but would have been racy.
>
> The solution is still so gross. Exporting all the inode cache
> internal functions so you can invalidate an external list of inodes
> is, IMO, not an appropriate solution for anything.
>
> Indeed, this is exactly what ->drop_inode() is for.
>
> Take this function:
>
> > +static void evict_dentries_for_decrypted_inodes(struct fscrypt_master_key *mk)
> > +{
> > + struct fscrypt_info *ci;
> > + struct inode *inode;
> > + struct inode *toput_inode = NULL;
> > +
> > + spin_lock(&mk->mk_decrypted_inodes_lock);
> > +
> > + list_for_each_entry(ci, &mk->mk_decrypted_inodes, ci_master_key_link) {
> > + inode = ci->ci_inode;
> > + spin_lock(&inode->i_lock);
> > + if (inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW)) {
> > + spin_unlock(&inode->i_lock);
> > + continue;
> > + }
> > + __iget(inode);
> > + spin_unlock(&inode->i_lock);
> > + spin_unlock(&mk->mk_decrypted_inodes_lock);
> > +
> > + shrink_dcache_inode(inode);
> > + iput(toput_inode);
> > + toput_inode = inode;
> > +
> > + spin_lock(&mk->mk_decrypted_inodes_lock);
> > + }
> > +
> > + spin_unlock(&mk->mk_decrypted_inodes_lock);
> > + iput(toput_inode);
> > +}
>
> It takes a new reference to each decrypted inode, and then drops it
> again after all the dentry cache references have been killed and
> we've got a reference to the next inode in the list. Killing the
> dentry references to the inode means it should only have in-use
> references and the reference this function holds on it.
>
> If the inode is not in use then there will be only one, and so it
> will fall into iput_final() and the ->drop_inode() function
> determines if the inode should be evicted from the cache and
> destroyed immediately. IOWs, implement fscrypt_drop_inode() to do
> the right thing when the key has been destroyed, and you can get rid
> of all this crazy inode cache walk-and-invalidate hackery.
>
Thanks for the feedback! If I understand correctly, your suggestion is:
- Keep evict_dentries_for_decrypted_inodes() as-is, i.e. fscrypt would still
evict the dentries for all inodes in ->mk_decrypted_inodes.
(I don't see how it could work otherwise.)
- However, evict_decrypted_inodes() would be removed and fscrypt would not
directly evict the list of inodes. Instead, the filesystem's ->drop_inode()
would be made to return 1 if the inode's master key has been removed. Thus
each inode, if no longer in use, would end up getting evicted during the
iput() in evict_dentries_for_decrypted_inodes().
I hadn't thought of this, and I think it would work; I'll try implementing it.
It would also have the advantage that if a key is removed while an inode is
still in-use, that inode will be evicted as soon as it's no longer in use rather
than waiting around until another FS_IOC_REMOVE_ENCRYPTION_KEY.
The ioctl will need a different way to determine whether any inodes couldn't be
evicted, but simply checking whether ->mk_decrypted_inodes ended up empty or not
should work.
FWIW, originally I also considered leaving the inodes in the inode cache and
instead only freeing ->i_crypt_info and truncating the pagecache. But I don't
see a way to do it even with this new idea; for one, ->drop_inode() is called
under ->i_lock. So it seems that eviction is still the way to go.
- Eric
^ permalink raw reply
* Re: [RFC PATCH v2 11/20] fscrypt: add FS_IOC_REMOVE_ENCRYPTION_KEY ioctl
From: Dave Chinner @ 2019-02-11 22:12 UTC (permalink / raw)
To: Eric Biggers
Cc: Satya Tangirala, linux-api, linux-f2fs-devel, linux-fscrypt,
keyrings, linux-mtd, linux-crypto, linux-fsdevel, linux-ext4,
Paul Crowley
In-Reply-To: <20190211172738.4633-12-ebiggers@kernel.org>
On Mon, Feb 11, 2019 at 09:27:29AM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> Add a new fscrypt ioctl, FS_IOC_REMOVE_ENCRYPTION_KEY. This ioctl
> removes an encryption key that was added by FS_IOC_ADD_ENCRYPTION_KEY.
> It wipes the secret key itself, then "locks" the encrypted files and
> directories that had been unlocked using that key -- implemented by
> evicting the relevant dentries and inodes from the VFS caches.
>
> The problem this solves is that many fscrypt users want the ability to
> remove encryption keys, causing the corresponding encrypted directories
> to appear "locked" (presented in ciphertext form) again. Moreover,
> users want removing an encryption key to *really* remove it, in the
> sense that the removed keys cannot be recovered even if kernel memory is
> compromised, e.g. by the exploit of a kernel security vulnerability or
> by a physical attack. This is desirable after a user logs out of the
> system, for example. In many cases users even already assume this to be
> the case and are surprised to hear when it's not.
>
> It is not sufficient to simply unlink the master key from the keyring
> (or to revoke or invalidate it), since the actual encryption transform
> objects are still pinned in memory by their inodes. Therefore, to
> really remove a key we must also evict the relevant inodes.
>
> Currently one workaround is to run 'sync && echo 2 >
> /proc/sys/vm/drop_caches'. But, that evicts all unused inodes in the
> system rather than just the inodes associated with the key being
> removed, causing severe performance problems. Moreover, it requires
> root privileges, so regular users can't "lock" their encrypted files.
>
> Another workaround, used in Chromium OS kernels, is to add a new
> VFS-level ioctl FS_IOC_DROP_CACHE which is a more restricted version of
> drop_caches that operates on a single super_block. It does:
>
> shrink_dcache_sb(sb);
> invalidate_inodes(sb, false);
>
> But it's still a hack. Yet, the major users of filesystem encryption
> want this feature badly enough that they are actually using these hacks.
>
> To properly solve the problem, start maintaining a list of the inodes
> which have been "unlocked" using each master key. Originally this
> wasn't possible because the kernel didn't keep track of in-use master
> keys at all. But, with the ->s_master_keys keyring it is now possible.
>
> Then, add an ioctl FS_IOC_REMOVE_ENCRYPTION_KEY. It finds the specified
> master key in ->s_master_keys, then wipes the secret key itself, which
> prevents any additional inodes from being unlocked with the key. Then,
> it syncs the filesystem and evicts the inodes in the key's list. The
> normal inode eviction code will free and wipe the per-file keys (in
> ->i_crypt_info). Note that freeing ->i_crypt_info without evicting the
> inodes was also considered, but would have been racy.
The solution is still so gross. Exporting all the inode cache
internal functions so you can invalidate an external list of inodes
is, IMO, not an appropriate solution for anything.
Indeed, this is exactly what ->drop_inode() is for.
Take this function:
> +static void evict_dentries_for_decrypted_inodes(struct fscrypt_master_key *mk)
> +{
> + struct fscrypt_info *ci;
> + struct inode *inode;
> + struct inode *toput_inode = NULL;
> +
> + spin_lock(&mk->mk_decrypted_inodes_lock);
> +
> + list_for_each_entry(ci, &mk->mk_decrypted_inodes, ci_master_key_link) {
> + inode = ci->ci_inode;
> + spin_lock(&inode->i_lock);
> + if (inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW)) {
> + spin_unlock(&inode->i_lock);
> + continue;
> + }
> + __iget(inode);
> + spin_unlock(&inode->i_lock);
> + spin_unlock(&mk->mk_decrypted_inodes_lock);
> +
> + shrink_dcache_inode(inode);
> + iput(toput_inode);
> + toput_inode = inode;
> +
> + spin_lock(&mk->mk_decrypted_inodes_lock);
> + }
> +
> + spin_unlock(&mk->mk_decrypted_inodes_lock);
> + iput(toput_inode);
> +}
It takes a new reference to each decrypted inode, and then drops it
again after all the dentry cache references have been killed and
we've got a reference to the next inode in the list. Killing the
dentry references to the inode means it should only have in-use
references and the reference this function holds on it.
If the inode is not in use then there will be only one, and so it
will fall into iput_final() and the ->drop_inode() function
determines if the inode should be evicted from the cache and
destroyed immediately. IOWs, implement fscrypt_drop_inode() to do
the right thing when the key has been destroyed, and you can get rid
of all this crazy inode cache walk-and-invalidate hackery.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply
* Re: + sysctl-return-einval-if-val-violates-minmax.patch added to -mm tree
From: Luis Chamberlain @ 2019-02-11 21:41 UTC (permalink / raw)
To: Christian Brauner
Cc: Alexey Dobriyan, akpm, linux-kernel, viro, longman, linux,
keescook, joe.lawrence, ebiederm, linux-api
In-Reply-To: <20190211213249.lta2pithpf5w6w7m@brauner.io>
On Mon, Feb 11, 2019 at 10:32:50PM +0100, Christian Brauner wrote:
> On Mon, Feb 11, 2019 at 01:25:23PM -0800, Luis Chamberlain wrote:
> > On Mon, Feb 11, 2019 at 10:19:19PM +0100, Christian Brauner wrote:
> > > On Tue, Feb 12, 2019 at 12:17:16AM +0300, Alexey Dobriyan wrote:
> > > > On Mon, Feb 11, 2019 at 01:06:32PM -0800, akpm@linux-foundation.org wrote:
> > > >
> > > > > @@ -2848,8 +2848,10 @@ static int __do_proc_doulongvec_minmax(v
> > > >
> > > > > - if ((min && val < *min) || (max && val > *max))
> > > > > - continue;
> > > > > + if ((min && val < *min) || (max && val > *max)) {
> > > > > + err = -EINVAL;
> > > >
> > > > I was asked to return ERANGE in kstrto*().
> > >
> > > I think we discussed ERANGE vs EINVAL and decided EINVAL because there
> > > was precedence for other sysctls already.
> >
> > Can you do a proper audit and see?
>
> If you look at proc_get_long() right now you can see that when the
> buffer we use to parse the number is exceeded we return EINVAL. In short
> if you do right now:
>
> echo 1844674407370955161600000 > /proc/sys/fs/file-max
>
> that would exceed the buffer in proc_get_long() and you already get
> EINVAL for all such cases. If we now change this to ERANGE we would
> return:
>
> echo 18446744073709551616 > /proc/sys/fs/file-max -> ERANGE
> echo 1844674407370955161600000 > /proc/sys/fs/file-max -> EINVAL
>
> which would be very confusing. For consistency we should use EINVAL.
>
> See kernel/sysctl.c:
>
> /* We don't know if the next char is whitespace thus we may accept
> * invalid integers (e.g. 1234...a) or two integers instead of one
> * (e.g. 123...1). So lets not allow such large numbers. */
> if (len == TMPBUFLEN - 1)
> return -EINVAL;
Thanks this works for me.
Luis
^ permalink raw reply
* Re: + sysctl-return-einval-if-val-violates-minmax.patch added to -mm tree
From: Christian Brauner @ 2019-02-11 21:32 UTC (permalink / raw)
To: Luis Chamberlain
Cc: Alexey Dobriyan, akpm, linux-kernel, viro, longman, linux,
keescook, joe.lawrence, ebiederm, linux-api
In-Reply-To: <20190211212523.GU11489@garbanzo.do-not-panic.com>
On Mon, Feb 11, 2019 at 01:25:23PM -0800, Luis Chamberlain wrote:
> On Mon, Feb 11, 2019 at 10:19:19PM +0100, Christian Brauner wrote:
> > On Tue, Feb 12, 2019 at 12:17:16AM +0300, Alexey Dobriyan wrote:
> > > On Mon, Feb 11, 2019 at 01:06:32PM -0800, akpm@linux-foundation.org wrote:
> > >
> > > > @@ -2848,8 +2848,10 @@ static int __do_proc_doulongvec_minmax(v
> > >
> > > > - if ((min && val < *min) || (max && val > *max))
> > > > - continue;
> > > > + if ((min && val < *min) || (max && val > *max)) {
> > > > + err = -EINVAL;
> > >
> > > I was asked to return ERANGE in kstrto*().
> >
> > I think we discussed ERANGE vs EINVAL and decided EINVAL because there
> > was precedence for other sysctls already.
>
> Can you do a proper audit and see?
If you look at proc_get_long() right now you can see that when the
buffer we use to parse the number is exceeded we return EINVAL. In short
if you do right now:
echo 1844674407370955161600000 > /proc/sys/fs/file-max
that would exceed the buffer in proc_get_long() and you already get
EINVAL for all such cases. If we now change this to ERANGE we would
return:
echo 18446744073709551616 > /proc/sys/fs/file-max -> ERANGE
echo 1844674407370955161600000 > /proc/sys/fs/file-max -> EINVAL
which would be very confusing. For consistency we should use EINVAL.
See kernel/sysctl.c:
/* We don't know if the next char is whitespace thus we may accept
* invalid integers (e.g. 1234...a) or two integers instead of one
* (e.g. 123...1). So lets not allow such large numbers. */
if (len == TMPBUFLEN - 1)
return -EINVAL;
Christian
^ permalink raw reply
* Re: + sysctl-return-einval-if-val-violates-minmax.patch added to -mm tree
From: Luis Chamberlain @ 2019-02-11 21:25 UTC (permalink / raw)
To: Christian Brauner
Cc: Alexey Dobriyan, akpm, linux-kernel, viro, longman, linux,
keescook, joe.lawrence, ebiederm, linux-api
In-Reply-To: <20190211211917.kq2ahdkhoe36obhp@brauner.io>
On Mon, Feb 11, 2019 at 10:19:19PM +0100, Christian Brauner wrote:
> On Tue, Feb 12, 2019 at 12:17:16AM +0300, Alexey Dobriyan wrote:
> > On Mon, Feb 11, 2019 at 01:06:32PM -0800, akpm@linux-foundation.org wrote:
> >
> > > @@ -2848,8 +2848,10 @@ static int __do_proc_doulongvec_minmax(v
> >
> > > - if ((min && val < *min) || (max && val > *max))
> > > - continue;
> > > + if ((min && val < *min) || (max && val > *max)) {
> > > + err = -EINVAL;
> >
> > I was asked to return ERANGE in kstrto*().
>
> I think we discussed ERANGE vs EINVAL and decided EINVAL because there
> was precedence for other sysctls already.
Can you do a proper audit and see?
linux-api folks may care.
Luis
^ permalink raw reply
* Re: [PATCH 13/18] io_uring: add file set registration
From: Jonathan Corbet @ 2019-02-11 20:33 UTC (permalink / raw)
To: Al Viro
Cc: Jens Axboe, Jann Horn, linux-aio, linux-block, Linux API, hch,
jmoyer, avi, linux-fsdevel
In-Reply-To: <20190211173521.GI2217@ZenIV.linux.org.uk>
On Mon, 11 Feb 2019 17:35:21 +0000
Al Viro <viro@zeniv.linux.org.uk> wrote:
> If you turn that thing into a coherent text, I'd appreciate a chance
> to take a look at the result and see if anything else needs to be
> corrected...
Thanks for the updated info!
I'm likely to do a watered-down version for some disreputable web site
first; after that, I'll try to do it properly for Documentation/. You'll
certainly see the result in your inbox once it's ready to be looked at.
Thanks,
jon
--
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: [RFC PATCH v4 3/3] sysctl: return -EINVAL if val violates minmax
From: Luis Chamberlain @ 2019-02-11 19:50 UTC (permalink / raw)
To: Christian Brauner
Cc: akpm, keescook, linux-kernel, ebiederm, joe.lawrence, longman,
linux, viro, adobriyan, linux-api
In-Reply-To: <20190210203943.8227-4-christian@brauner.io>
On Sun, Feb 10, 2019 at 09:39:43PM +0100, Christian Brauner wrote:
> Currently when userspace gives us a values that overflow e.g. file-max and
> other callers of __do_proc_doulongvec_minmax() we simply
> ignore the new value and leave the current value untouched. This can be
> problematic as it gives the illusion that the limit has indeed be bumped
> when in fact it failed.
> This commit makes sure to return EINVAL when an overflow is detected.
> Please note that this is a userspace facing change.
>
Acked-by: Luis Chamberlain <mcgrof@kernel.org>
Luis
> Signed-off-by: Christian Brauner <christian@brauner.io>
> ---
> /* Changelog */
> v4:
> - patch introduced
>
> v1-v3:
> - patch not present
> ---
> kernel/sysctl.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index c4a44b7ccb8a..516bc8a2812d 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -2846,8 +2846,10 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int
> if (neg)
> continue;
> val = convmul * val / convdiv;
> - if ((min && val < *min) || (max && val > *max))
> - continue;
> + if ((min && val < *min) || (max && val > *max)) {
> + err = -EINVAL;
> + break;
> + }
> *i = val;
> } else {
> val = convdiv * (*i) / convmul;
> --
> 2.20.1
>
^ permalink raw reply
* [PATCH 19/19] io_uring: add io_uring_event cache hit information
From: Jens Axboe @ 2019-02-11 19:00 UTC (permalink / raw)
To: linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro, Jens Axboe
In-Reply-To: <20190211190049.7888-1-axboe@kernel.dk>
Add hint on whether a read was served out of the page cache, or if it
hit media. This is useful for buffered async IO, O_DIRECT reads would
never have this set (for obvious reasons).
If the read hit page cache, cqe->flags will have IOCQE_FLAG_CACHEHIT
set.
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
fs/io_uring.c | 7 ++++++-
include/uapi/linux/io_uring.h | 5 +++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index ce446f59f092..a4973af1c272 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -611,11 +611,16 @@ static void io_fput(struct io_kiocb *req)
static void io_complete_rw(struct kiocb *kiocb, long res, long res2)
{
struct io_kiocb *req = container_of(kiocb, struct io_kiocb, rw);
+ unsigned ev_flags = 0;
kiocb_end_write(kiocb);
io_fput(req);
- io_cqring_add_event(req->ctx, req->user_data, res, 0);
+
+ if (res > 0 && (req->flags & REQ_F_FORCE_NONBLOCK))
+ ev_flags = IOCQE_FLAG_CACHEHIT;
+
+ io_cqring_add_event(req->ctx, req->user_data, res, ev_flags);
io_free_req(req);
}
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index e23408692118..24906e99fdc7 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -69,6 +69,11 @@ struct io_uring_cqe {
__u32 flags;
};
+/*
+ * io_uring_event->flags
+ */
+#define IOCQE_FLAG_CACHEHIT (1U << 0) /* IO did not hit media */
+
/*
* Magic offsets for the application to mmap the data it needs
*/
--
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 18/19] io_uring: allow workqueue item to handle multiple buffered requests
From: Jens Axboe @ 2019-02-11 19:00 UTC (permalink / raw)
To: linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro, Jens Axboe
In-Reply-To: <20190211190049.7888-1-axboe@kernel.dk>
Right now we punt any buffered request that ends up triggering an
-EAGAIN to an async workqueue. This works fine in terms of providing
async execution of them, but it also can create quite a lot of work
queue items. For sequentially buffered IO, it's advantageous to
serialize the issue of them. For reads, the first one will trigger a
read-ahead, and subsequent request merely end up waiting on later pages
to complete. For writes, devices usually respond better to streamed
sequential writes.
Add state to track the last buffered request we punted to a work queue,
and if the next one is sequential to the previous, attempt to get the
previous work item to handle it. We limit the number of sequential
add-ons to the a multiple (8) of the max read-ahead size of the file.
This should be a good number for both reads and wries, as it defines the
max IO size the device can do directly.
This drastically cuts down on the number of context switches we need to
handle buffered sequential IO, and a basic test case of copying a big
file with io_uring sees a 5x speedup.
Reviewed-by: Hannes Reinecke <hare@suse.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
fs/io_uring.c | 281 ++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 229 insertions(+), 52 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index a0513d4bc35d..ce446f59f092 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -94,6 +94,16 @@ struct io_mapped_ubuf {
unsigned int nr_bvecs;
};
+struct async_list {
+ spinlock_t lock;
+ atomic_t cnt;
+ struct list_head list;
+
+ struct file *file;
+ off_t io_end;
+ size_t io_pages;
+};
+
struct io_ring_ctx {
struct {
struct percpu_ref refs;
@@ -164,6 +174,8 @@ struct io_ring_ctx {
struct list_head cancel_list;
} ____cacheline_aligned_in_smp;
+ struct async_list pending_async[2];
+
#if defined(CONFIG_UNIX)
struct socket *ring_sock;
#endif
@@ -201,6 +213,7 @@ struct io_kiocb {
#define REQ_F_FORCE_NONBLOCK 1 /* inline submission attempt */
#define REQ_F_IOPOLL_COMPLETED 2 /* polled IO has completed */
#define REQ_F_FIXED_FILE 4 /* ctx owns file */
+#define REQ_F_SEQ_PREV 8 /* sequential with previous */
u64 user_data;
u64 error;
@@ -257,6 +270,7 @@ static void io_ring_ctx_ref_free(struct percpu_ref *ref)
static struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p)
{
struct io_ring_ctx *ctx;
+ int i;
ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
if (!ctx)
@@ -272,6 +286,11 @@ static struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p)
init_completion(&ctx->ctx_done);
mutex_init(&ctx->uring_lock);
init_waitqueue_head(&ctx->wait);
+ for (i = 0; i < ARRAY_SIZE(ctx->pending_async); i++) {
+ spin_lock_init(&ctx->pending_async[i].lock);
+ INIT_LIST_HEAD(&ctx->pending_async[i].list);
+ atomic_set(&ctx->pending_async[i].cnt, 0);
+ }
spin_lock_init(&ctx->completion_lock);
INIT_LIST_HEAD(&ctx->poll_list);
INIT_LIST_HEAD(&ctx->cancel_list);
@@ -885,6 +904,47 @@ static int io_import_iovec(struct io_ring_ctx *ctx, int rw,
return import_iovec(rw, buf, sqe_len, UIO_FASTIOV, iovec, iter);
}
+/*
+ * Make a note of the last file/offset/direction we punted to async
+ * context. We'll use this information to see if we can piggy back a
+ * sequential request onto the previous one, if it's still hasn't been
+ * completed by the async worker.
+ */
+static void io_async_list_note(int rw, struct io_kiocb *req, size_t len)
+{
+ struct async_list *async_list = &req->ctx->pending_async[rw];
+ struct kiocb *kiocb = &req->rw;
+ struct file *filp = kiocb->ki_filp;
+ off_t io_end = kiocb->ki_pos + len;
+
+ if (filp == async_list->file && kiocb->ki_pos == async_list->io_end) {
+ unsigned long max_pages;
+
+ /* Use 8x RA size as a decent limiter for both reads/writes */
+ max_pages = filp->f_ra.ra_pages;
+ if (!max_pages)
+ max_pages = VM_MAX_READAHEAD >> (PAGE_SHIFT - 10);
+ max_pages *= 8;
+
+ /* If max pages are exceeded, reset the state */
+ len >>= PAGE_SHIFT;
+ if (async_list->io_pages + len <= max_pages) {
+ req->flags |= REQ_F_SEQ_PREV;
+ async_list->io_pages += len;
+ } else {
+ io_end = 0;
+ async_list->io_pages = 0;
+ }
+ }
+
+ /* New file? Reset state. */
+ if (async_list->file != filp) {
+ async_list->io_pages = 0;
+ async_list->file = filp;
+ }
+ async_list->io_end = io_end;
+}
+
static ssize_t io_read(struct io_kiocb *req, const struct sqe_submit *s,
bool force_nonblock, struct io_submit_state *state)
{
@@ -892,6 +952,7 @@ static ssize_t io_read(struct io_kiocb *req, const struct sqe_submit *s,
struct kiocb *kiocb = &req->rw;
struct iov_iter iter;
struct file *file;
+ size_t iov_count;
ssize_t ret;
ret = io_prep_rw(req, s, force_nonblock, state);
@@ -910,16 +971,24 @@ static ssize_t io_read(struct io_kiocb *req, const struct sqe_submit *s,
if (ret)
goto out_fput;
- ret = rw_verify_area(READ, file, &kiocb->ki_pos, iov_iter_count(&iter));
+ iov_count = iov_iter_count(&iter);
+ ret = rw_verify_area(READ, file, &kiocb->ki_pos, iov_count);
if (!ret) {
ssize_t ret2;
/* Catch -EAGAIN return for forced non-blocking submission */
ret2 = call_read_iter(file, kiocb, &iter);
- if (!force_nonblock || ret2 != -EAGAIN)
+ if (!force_nonblock || ret2 != -EAGAIN) {
io_rw_done(kiocb, ret2);
- else
+ } else {
+ /*
+ * If ->needs_lock is true, we're already in async
+ * context.
+ */
+ if (!s->needs_lock)
+ io_async_list_note(READ, req, iov_count);
ret = -EAGAIN;
+ }
}
kfree(iovec);
out_fput:
@@ -936,14 +1005,12 @@ static ssize_t io_write(struct io_kiocb *req, const struct sqe_submit *s,
struct kiocb *kiocb = &req->rw;
struct iov_iter iter;
struct file *file;
+ size_t iov_count;
ssize_t ret;
ret = io_prep_rw(req, s, force_nonblock, state);
if (ret)
return ret;
- /* Hold on to the file for -EAGAIN */
- if (force_nonblock && !(kiocb->ki_flags & IOCB_DIRECT))
- return -EAGAIN;
ret = -EBADF;
file = kiocb->ki_filp;
@@ -957,8 +1024,17 @@ static ssize_t io_write(struct io_kiocb *req, const struct sqe_submit *s,
if (ret)
goto out_fput;
- ret = rw_verify_area(WRITE, file, &kiocb->ki_pos,
- iov_iter_count(&iter));
+ iov_count = iov_iter_count(&iter);
+
+ ret = -EAGAIN;
+ if (force_nonblock && !(kiocb->ki_flags & IOCB_DIRECT)) {
+ /* If ->needs_lock is true, we're already in async context. */
+ if (!s->needs_lock)
+ io_async_list_note(WRITE, req, iov_count);
+ goto out_free;
+ }
+
+ ret = rw_verify_area(WRITE, file, &kiocb->ki_pos, iov_count);
if (!ret) {
/*
* Open-code file_start_write here to grab freeze protection,
@@ -976,9 +1052,11 @@ static ssize_t io_write(struct io_kiocb *req, const struct sqe_submit *s,
kiocb->ki_flags |= IOCB_WRITE;
io_rw_done(kiocb, call_write_iter(file, kiocb, &iter));
}
+out_free:
kfree(iovec);
out_fput:
- if (unlikely(ret))
+ /* Hold on to the file for -EAGAIN */
+ if (unlikely(ret && ret != -EAGAIN))
io_fput(req);
return ret;
}
@@ -1374,6 +1452,21 @@ static int __io_submit_sqe(struct io_ring_ctx *ctx, struct io_kiocb *req,
return 0;
}
+static struct async_list *io_async_list_from_sqe(struct io_ring_ctx *ctx,
+ const struct io_uring_sqe *sqe)
+{
+ switch (sqe->opcode) {
+ case IORING_OP_READV:
+ case IORING_OP_READ_FIXED:
+ return &ctx->pending_async[READ];
+ case IORING_OP_WRITEV:
+ case IORING_OP_WRITE_FIXED:
+ return &ctx->pending_async[WRITE];
+ default:
+ return NULL;
+ }
+}
+
static inline bool io_sqe_needs_user(const struct io_uring_sqe *sqe)
{
u8 opcode = READ_ONCE(sqe->opcode);
@@ -1385,61 +1478,138 @@ static inline bool io_sqe_needs_user(const struct io_uring_sqe *sqe)
static void io_sq_wq_submit_work(struct work_struct *work)
{
struct io_kiocb *req = container_of(work, struct io_kiocb, work);
- struct sqe_submit *s = &req->submit;
- const struct io_uring_sqe *sqe = s->sqe;
struct io_ring_ctx *ctx = req->ctx;
+ struct mm_struct *cur_mm = NULL;
+ struct async_list *async_list;
+ LIST_HEAD(req_list);
mm_segment_t old_fs;
- bool needs_user;
int ret;
- /* Ensure we clear previously set forced non-block flag */
- req->flags &= ~REQ_F_FORCE_NONBLOCK;
- req->rw.ki_flags &= ~IOCB_NOWAIT;
+ async_list = io_async_list_from_sqe(ctx, req->submit.sqe);
+restart:
+ do {
+ struct sqe_submit *s = &req->submit;
+ const struct io_uring_sqe *sqe = s->sqe;
+
+ /* Ensure we clear previously set forced non-block flag */
+ req->flags &= ~REQ_F_FORCE_NONBLOCK;
+ req->rw.ki_flags &= ~IOCB_NOWAIT;
- s->needs_lock = true;
- s->has_user = false;
+ ret = 0;
+ if (io_sqe_needs_user(sqe) && !cur_mm) {
+ if (!mmget_not_zero(ctx->sqo_mm)) {
+ ret = -EFAULT;
+ } else {
+ cur_mm = ctx->sqo_mm;
+ use_mm(cur_mm);
+ old_fs = get_fs();
+ set_fs(USER_DS);
+ }
+ }
+
+ if (!ret) {
+ s->has_user = cur_mm != NULL;
+ s->needs_lock = true;
+ do {
+ ret = __io_submit_sqe(ctx, req, s, false, NULL);
+ /*
+ * We can get EAGAIN for polled IO even though
+ * we're forcing a sync submission from here,
+ * since we can't wait for request slots on the
+ * block side.
+ */
+ if (ret != -EAGAIN)
+ break;
+ cond_resched();
+ } while (1);
+ }
+ if (ret) {
+ io_cqring_add_event(ctx, sqe->user_data, ret, 0);
+ io_free_req(req);
+ }
+
+ /* async context always use a copy of the sqe */
+ kfree(sqe);
+
+ if (!async_list)
+ break;
+ if (!list_empty(&req_list)) {
+ req = list_first_entry(&req_list, struct io_kiocb,
+ list);
+ list_del(&req->list);
+ continue;
+ }
+ if (list_empty(&async_list->list))
+ break;
+
+ req = NULL;
+ spin_lock(&async_list->lock);
+ if (list_empty(&async_list->list)) {
+ spin_unlock(&async_list->lock);
+ break;
+ }
+ list_splice_init(&async_list->list, &req_list);
+ spin_unlock(&async_list->lock);
+
+ req = list_first_entry(&req_list, struct io_kiocb, list);
+ list_del(&req->list);
+ } while (req);
/*
- * If we're doing IO to fixed buffers, we don't need to get/set
- * user context
+ * Rare case of racing with a submitter. If we find the count has
+ * dropped to zero AND we have pending work items, then restart
+ * the processing. This is a tiny race window.
*/
- needs_user = io_sqe_needs_user(s->sqe);
- if (needs_user) {
- if (!mmget_not_zero(ctx->sqo_mm)) {
- ret = -EFAULT;
- goto err;
+ if (async_list) {
+ ret = atomic_dec_return(&async_list->cnt);
+ while (!ret && !list_empty(&async_list->list)) {
+ spin_lock(&async_list->lock);
+ atomic_inc(&async_list->cnt);
+ list_splice_init(&async_list->list, &req_list);
+ spin_unlock(&async_list->lock);
+
+ if (!list_empty(&req_list)) {
+ req = list_first_entry(&req_list,
+ struct io_kiocb, list);
+ list_del(&req->list);
+ goto restart;
+ }
+ ret = atomic_dec_return(&async_list->cnt);
}
- use_mm(ctx->sqo_mm);
- old_fs = get_fs();
- set_fs(USER_DS);
- s->has_user = true;
}
- do {
- ret = __io_submit_sqe(ctx, req, s, false, NULL);
- /*
- * We can get EAGAIN for polled IO even though we're forcing
- * a sync submission from here, since we can't wait for
- * request slots on the block side.
- */
- if (ret != -EAGAIN)
- break;
- cond_resched();
- } while (1);
-
- if (needs_user) {
+ if (cur_mm) {
set_fs(old_fs);
- unuse_mm(ctx->sqo_mm);
- mmput(ctx->sqo_mm);
- }
-err:
- if (ret) {
- io_cqring_add_event(ctx, sqe->user_data, ret, 0);
- io_free_req(req);
+ unuse_mm(cur_mm);
+ mmput(cur_mm);
}
+}
- /* async context always use a copy of the sqe */
- kfree(sqe);
+/*
+ * See if we can piggy back onto previously submitted work, that is still
+ * running. We currently only allow this if the new request is sequential
+ * to the previous one we punted.
+ */
+static bool io_add_to_prev_work(struct async_list *list, struct io_kiocb *req)
+{
+ bool ret = false;
+
+ if (!list)
+ return false;
+ if (!(req->flags & REQ_F_SEQ_PREV))
+ return false;
+ if (!atomic_read(&list->cnt))
+ return false;
+
+ ret = true;
+ spin_lock(&list->lock);
+ list_add_tail(&req->list, &list->list);
+ if (!atomic_read(&list->cnt)) {
+ list_del_init(&req->list);
+ ret = false;
+ }
+ spin_unlock(&list->lock);
+ return ret;
}
static int io_submit_sqe(struct io_ring_ctx *ctx, struct sqe_submit *s,
@@ -1464,12 +1634,19 @@ static int io_submit_sqe(struct io_ring_ctx *ctx, struct sqe_submit *s,
sqe_copy = kmalloc(sizeof(*sqe_copy), GFP_KERNEL);
if (sqe_copy) {
+ struct async_list *list;
+
memcpy(sqe_copy, s->sqe, sizeof(*sqe_copy));
s->sqe = sqe_copy;
memcpy(&req->submit, s, sizeof(*s));
- INIT_WORK(&req->work, io_sq_wq_submit_work);
- queue_work(ctx->sqo_wq, &req->work);
+ list = io_async_list_from_sqe(ctx, s->sqe);
+ if (!io_add_to_prev_work(list, req)) {
+ if (list)
+ atomic_inc(&list->cnt);
+ INIT_WORK(&req->work, io_sq_wq_submit_work);
+ queue_work(ctx->sqo_wq, &req->work);
+ }
ret = 0;
}
}
--
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 17/19] io_uring: add support for IORING_OP_POLL
From: Jens Axboe @ 2019-02-11 19:00 UTC (permalink / raw)
To: linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro, Jens Axboe
In-Reply-To: <20190211190049.7888-1-axboe@kernel.dk>
This is basically a direct port of bfe4037e722e, which implements a
one-shot poll command through aio. Description below is based on that
commit as well. However, instead of adding a POLL command and relying
on io_cancel(2) to remove it, we mimic the epoll(2) interface of
having a command to add a poll notification, IORING_OP_POLL_ADD,
and one to remove it again, IORING_OP_POLL_REMOVE.
To poll for a file descriptor the application should submit an sqe of
type IORING_OP_POLL. It will poll the fd for the events specified in the
poll_events field.
Unlike poll or epoll without EPOLLONESHOT this interface always works in
one shot mode, that is once the sqe is completed, it will have to be
resubmitted.
Reviewed-by: Hannes Reinecke <hare@suse.com>
Based-on-code-from: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
fs/io_uring.c | 261 +++++++++++++++++++++++++++++++++-
include/uapi/linux/io_uring.h | 3 +
2 files changed, 263 insertions(+), 1 deletion(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 33b6c6167595..a0513d4bc35d 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -161,6 +161,7 @@ struct io_ring_ctx {
* manipulate the list, hence no extra locking is needed there.
*/
struct list_head poll_list;
+ struct list_head cancel_list;
} ____cacheline_aligned_in_smp;
#if defined(CONFIG_UNIX)
@@ -176,8 +177,20 @@ struct sqe_submit {
bool needs_fixed_file;
};
+struct io_poll_iocb {
+ struct file *file;
+ struct wait_queue_head *head;
+ __poll_t events;
+ bool woken;
+ bool canceled;
+ struct wait_queue_entry wait;
+};
+
struct io_kiocb {
- struct kiocb rw;
+ union {
+ struct kiocb rw;
+ struct io_poll_iocb poll;
+ };
struct sqe_submit submit;
@@ -261,6 +274,7 @@ static struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p)
init_waitqueue_head(&ctx->wait);
spin_lock_init(&ctx->completion_lock);
INIT_LIST_HEAD(&ctx->poll_list);
+ INIT_LIST_HEAD(&ctx->cancel_list);
return ctx;
}
@@ -1058,6 +1072,244 @@ static int io_fsync(struct io_kiocb *req, const struct io_uring_sqe *sqe,
return 0;
}
+static void io_poll_remove_one(struct io_kiocb *req)
+{
+ struct io_poll_iocb *poll = &req->poll;
+
+ spin_lock(&poll->head->lock);
+ WRITE_ONCE(poll->canceled, true);
+ if (!list_empty(&poll->wait.entry)) {
+ list_del_init(&poll->wait.entry);
+ queue_work(req->ctx->sqo_wq, &req->work);
+ }
+ spin_unlock(&poll->head->lock);
+
+ list_del_init(&req->list);
+}
+
+static void io_poll_remove_all(struct io_ring_ctx *ctx)
+{
+ struct io_kiocb *req;
+
+ spin_lock_irq(&ctx->completion_lock);
+ while (!list_empty(&ctx->cancel_list)) {
+ req = list_first_entry(&ctx->cancel_list, struct io_kiocb,list);
+ io_poll_remove_one(req);
+ }
+ spin_unlock_irq(&ctx->completion_lock);
+}
+
+/*
+ * Find a running poll command that matches one specified in sqe->addr,
+ * and remove it if found.
+ */
+static int io_poll_remove(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+{
+ struct io_ring_ctx *ctx = req->ctx;
+ struct io_kiocb *poll_req, *next;
+ int ret = -ENOENT;
+
+ if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
+ return -EINVAL;
+ if (sqe->ioprio || sqe->off || sqe->len || sqe->buf_index ||
+ sqe->poll_events)
+ return -EINVAL;
+
+ spin_lock_irq(&ctx->completion_lock);
+ list_for_each_entry_safe(poll_req, next, &ctx->cancel_list, list) {
+ if (READ_ONCE(sqe->addr) == poll_req->user_data) {
+ io_poll_remove_one(poll_req);
+ ret = 0;
+ break;
+ }
+ }
+ spin_unlock_irq(&ctx->completion_lock);
+
+ io_cqring_add_event(req->ctx, sqe->user_data, ret, 0);
+ io_free_req(req);
+ return 0;
+}
+
+static void io_poll_complete(struct io_kiocb *req, __poll_t mask)
+{
+ io_cqring_add_event(req->ctx, req->user_data, mangle_poll(mask), 0);
+ io_fput(req);
+ io_free_req(req);
+}
+
+static void io_poll_complete_work(struct work_struct *work)
+{
+ struct io_kiocb *req = container_of(work, struct io_kiocb, work);
+ struct io_poll_iocb *poll = &req->poll;
+ struct poll_table_struct pt = { ._key = poll->events };
+ struct io_ring_ctx *ctx = req->ctx;
+ __poll_t mask = 0;
+
+ if (!READ_ONCE(poll->canceled))
+ mask = vfs_poll(poll->file, &pt) & poll->events;
+
+ /*
+ * Note that ->ki_cancel callers also delete iocb from active_reqs after
+ * calling ->ki_cancel. We need the ctx_lock roundtrip here to
+ * synchronize with them. In the cancellation case the list_del_init
+ * itself is not actually needed, but harmless so we keep it in to
+ * avoid further branches in the fast path.
+ */
+ spin_lock_irq(&ctx->completion_lock);
+ if (!mask && !READ_ONCE(poll->canceled)) {
+ add_wait_queue(poll->head, &poll->wait);
+ spin_unlock_irq(&ctx->completion_lock);
+ return;
+ }
+ list_del_init(&req->list);
+ spin_unlock_irq(&ctx->completion_lock);
+
+ io_poll_complete(req, mask);
+}
+
+static int io_poll_wake(struct wait_queue_entry *wait, unsigned mode, int sync,
+ void *key)
+{
+ struct io_poll_iocb *poll = container_of(wait, struct io_poll_iocb,
+ wait);
+ struct io_kiocb *req = container_of(poll, struct io_kiocb, poll);
+ struct io_ring_ctx *ctx = req->ctx;
+ __poll_t mask = key_to_poll(key);
+
+ poll->woken = true;
+
+ /* for instances that support it check for an event match first: */
+ if (mask) {
+ if (!(mask & poll->events))
+ return 0;
+
+ /* try to complete the iocb inline if we can: */
+ if (spin_trylock(&ctx->completion_lock)) {
+ list_del(&req->list);
+ spin_unlock(&ctx->completion_lock);
+
+ list_del_init(&poll->wait.entry);
+ io_poll_complete(req, mask);
+ return 1;
+ }
+ }
+
+ list_del_init(&poll->wait.entry);
+ queue_work(ctx->sqo_wq, &req->work);
+ return 1;
+}
+
+struct io_poll_table {
+ struct poll_table_struct pt;
+ struct io_kiocb *req;
+ int error;
+};
+
+static void io_poll_queue_proc(struct file *file, struct wait_queue_head *head,
+ struct poll_table_struct *p)
+{
+ struct io_poll_table *pt = container_of(p, struct io_poll_table, pt);
+
+ if (unlikely(pt->req->poll.head)) {
+ pt->error = -EINVAL;
+ return;
+ }
+
+ pt->error = 0;
+ pt->req->poll.head = head;
+ add_wait_queue(head, &pt->req->poll.wait);
+}
+
+static int io_poll_add(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+{
+ struct io_poll_iocb *poll = &req->poll;
+ struct io_ring_ctx *ctx = req->ctx;
+ struct io_poll_table ipt;
+ unsigned flags;
+ __poll_t mask;
+ u16 events;
+ int fd;
+
+ if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
+ return -EINVAL;
+ if (sqe->addr || sqe->ioprio || sqe->off || sqe->len || sqe->buf_index)
+ return -EINVAL;
+
+ INIT_WORK(&req->work, io_poll_complete_work);
+ events = READ_ONCE(sqe->poll_events);
+ poll->events = demangle_poll(events) | EPOLLERR | EPOLLHUP;
+
+ flags = READ_ONCE(sqe->flags);
+ fd = READ_ONCE(sqe->fd);
+
+ if (flags & IOSQE_FIXED_FILE) {
+ if (unlikely(!ctx->user_files || fd >= ctx->nr_user_files))
+ return -EBADF;
+ poll->file = ctx->user_files[fd];
+ req->flags |= REQ_F_FIXED_FILE;
+ } else {
+ poll->file = fget(fd);
+ }
+ if (unlikely(!poll->file))
+ return -EBADF;
+
+ poll->head = NULL;
+ poll->woken = false;
+ poll->canceled = false;
+
+ ipt.pt._qproc = io_poll_queue_proc;
+ ipt.pt._key = poll->events;
+ ipt.req = req;
+ ipt.error = -EINVAL; /* same as no support for IOCB_CMD_POLL */
+
+ /* initialized the list so that we can do list_empty checks */
+ INIT_LIST_HEAD(&poll->wait.entry);
+ init_waitqueue_func_entry(&poll->wait, io_poll_wake);
+
+ /* one for removal from waitqueue, one for this function */
+ refcount_set(&req->refs, 2);
+
+ mask = vfs_poll(poll->file, &ipt.pt) & poll->events;
+ if (unlikely(!poll->head)) {
+ /* we did not manage to set up a waitqueue, done */
+ goto out;
+ }
+
+ spin_lock_irq(&ctx->completion_lock);
+ spin_lock(&poll->head->lock);
+ if (poll->woken) {
+ /* wake_up context handles the rest */
+ mask = 0;
+ ipt.error = 0;
+ } else if (mask || ipt.error) {
+ /* if we get an error or a mask we are done */
+ WARN_ON_ONCE(list_empty(&poll->wait.entry));
+ list_del_init(&poll->wait.entry);
+ } else {
+ /* actually waiting for an event */
+ list_add_tail(&req->list, &ctx->cancel_list);
+ }
+ spin_unlock(&poll->head->lock);
+ spin_unlock_irq(&ctx->completion_lock);
+
+out:
+ if (unlikely(ipt.error)) {
+ if (!(flags & IOSQE_FIXED_FILE))
+ fput(poll->file);
+ /*
+ * Drop one of our refs to this req, __io_submit_sqe() will
+ * drop the other one since we're returning an error.
+ */
+ io_free_req(req);
+ return ipt.error;
+ }
+
+ if (mask)
+ io_poll_complete(req, mask);
+ io_free_req(req);
+ return 0;
+}
+
static int __io_submit_sqe(struct io_ring_ctx *ctx, struct io_kiocb *req,
const struct sqe_submit *s, bool force_nonblock,
struct io_submit_state *state)
@@ -1093,6 +1345,12 @@ static int __io_submit_sqe(struct io_ring_ctx *ctx, struct io_kiocb *req,
case IORING_OP_FSYNC:
ret = io_fsync(req, s->sqe, force_nonblock);
break;
+ case IORING_OP_POLL_ADD:
+ ret = io_poll_add(req, s->sqe);
+ break;
+ case IORING_OP_POLL_REMOVE:
+ ret = io_poll_remove(req, s->sqe);
+ break;
default:
ret = -EINVAL;
break;
@@ -2081,6 +2339,7 @@ static void io_ring_ctx_wait_and_kill(struct io_ring_ctx *ctx)
percpu_ref_kill(&ctx->refs);
mutex_unlock(&ctx->uring_lock);
+ io_poll_remove_all(ctx);
io_iopoll_reap_events(ctx);
wait_for_completion(&ctx->ctx_done);
io_ring_ctx_free(ctx);
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 0ec74bab8dbe..e23408692118 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -25,6 +25,7 @@ struct io_uring_sqe {
union {
__kernel_rwf_t rw_flags;
__u32 fsync_flags;
+ __u16 poll_events;
};
__u64 user_data; /* data to be passed back at completion time */
union {
@@ -51,6 +52,8 @@ struct io_uring_sqe {
#define IORING_OP_FSYNC 3
#define IORING_OP_READ_FIXED 4
#define IORING_OP_WRITE_FIXED 5
+#define IORING_OP_POLL_ADD 6
+#define IORING_OP_POLL_REMOVE 7
/*
* sqe->fsync_flags
--
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 16/19] io_uring: add io_kiocb ref count
From: Jens Axboe @ 2019-02-11 19:00 UTC (permalink / raw)
To: linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro, Jens Axboe
In-Reply-To: <20190211190049.7888-1-axboe@kernel.dk>
We'll use this for the POLL implementation. Regular requests will
NOT be using references, so initialize it to 0. Any real use of
the io_kiocb ref will initialize it to at least 2.
Reviewed-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
fs/io_uring.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 24c280076e81..33b6c6167595 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -184,6 +184,7 @@ struct io_kiocb {
struct io_ring_ctx *ctx;
struct list_head list;
unsigned int flags;
+ refcount_t refs;
#define REQ_F_FORCE_NONBLOCK 1 /* inline submission attempt */
#define REQ_F_IOPOLL_COMPLETED 2 /* polled IO has completed */
#define REQ_F_FIXED_FILE 4 /* ctx owns file */
@@ -377,6 +378,7 @@ static struct io_kiocb *io_get_req(struct io_ring_ctx *ctx,
req->ctx = ctx;
req->flags = 0;
+ refcount_set(&req->refs, 0);
return req;
out:
io_ring_drop_ctx_refs(ctx, 1);
@@ -394,8 +396,10 @@ static void io_free_req_many(struct io_ring_ctx *ctx, void **reqs, int *nr)
static void io_free_req(struct io_kiocb *req)
{
- io_ring_drop_ctx_refs(req->ctx, 1);
- kmem_cache_free(req_cachep, req);
+ if (!refcount_read(&req->refs) || refcount_dec_and_test(&req->refs)) {
+ io_ring_drop_ctx_refs(req->ctx, 1);
+ kmem_cache_free(req_cachep, req);
+ }
}
/*
--
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 15/19] io_uring: add submission polling
From: Jens Axboe @ 2019-02-11 19:00 UTC (permalink / raw)
To: linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro, Jens Axboe
In-Reply-To: <20190211190049.7888-1-axboe@kernel.dk>
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.
Reviewed-by: Hannes Reinecke <hare@suse.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
fs/io_uring.c | 248 +++++++++++++++++++++++++++++++++-
include/uapi/linux/io_uring.h | 12 +-
2 files changed, 252 insertions(+), 8 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 167c7f96666f..24c280076e81 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -44,6 +44,7 @@
#include <linux/percpu.h>
#include <linux/slab.h>
#include <linux/workqueue.h>
+#include <linux/kthread.h>
#include <linux/blkdev.h>
#include <linux/bvec.h>
#include <linux/net.h>
@@ -108,12 +109,16 @@ struct io_ring_ctx {
unsigned cached_sq_head;
unsigned sq_entries;
unsigned sq_mask;
+ unsigned sq_thread_idle;
struct io_uring_sqe *sq_sqes;
} ____cacheline_aligned_in_smp;
/* IO offload */
struct workqueue_struct *sqo_wq;
+ struct task_struct *sqo_thread; /* if using sq thread polling */
struct mm_struct *sqo_mm;
+ wait_queue_head_t sqo_wait;
+ unsigned sqo_stop;
struct {
/* CQ ring */
@@ -168,6 +173,7 @@ struct sqe_submit {
unsigned short index;
bool has_user;
bool needs_lock;
+ bool needs_fixed_file;
};
struct io_kiocb {
@@ -327,6 +333,8 @@ static void io_cqring_add_event(struct io_ring_ctx *ctx, u64 ki_user_data,
if (waitqueue_active(&ctx->wait))
wake_up(&ctx->wait);
+ if (waitqueue_active(&ctx->sqo_wait))
+ wake_up(&ctx->sqo_wait);
}
static void io_ring_drop_ctx_refs(struct io_ring_ctx *ctx, unsigned refs)
@@ -680,9 +688,10 @@ static bool io_file_supports_async(struct file *file)
return false;
}
-static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe,
+static int io_prep_rw(struct io_kiocb *req, const struct sqe_submit *s,
bool force_nonblock, struct io_submit_state *state)
{
+ const struct io_uring_sqe *sqe = s->sqe;
struct io_ring_ctx *ctx = req->ctx;
struct kiocb *kiocb = &req->rw;
unsigned ioprio, flags;
@@ -702,6 +711,8 @@ static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe,
kiocb->ki_filp = ctx->user_files[fd];
req->flags |= REQ_F_FIXED_FILE;
} else {
+ if (s->needs_fixed_file)
+ return -EBADF;
kiocb->ki_filp = io_file_get(state, fd);
if (unlikely(!kiocb->ki_filp))
return -EBADF;
@@ -865,7 +876,7 @@ static ssize_t io_read(struct io_kiocb *req, const struct sqe_submit *s,
struct file *file;
ssize_t ret;
- ret = io_prep_rw(req, s->sqe, force_nonblock, state);
+ ret = io_prep_rw(req, s, force_nonblock, state);
if (ret)
return ret;
file = kiocb->ki_filp;
@@ -909,7 +920,7 @@ static ssize_t io_write(struct io_kiocb *req, const struct sqe_submit *s,
struct file *file;
ssize_t ret;
- ret = io_prep_rw(req, s->sqe, force_nonblock, state);
+ ret = io_prep_rw(req, s, force_nonblock, state);
if (ret)
return ret;
/* Hold on to the file for -EAGAIN */
@@ -1295,6 +1306,170 @@ static bool io_get_sqring(struct io_ring_ctx *ctx, struct sqe_submit *s)
return false;
}
+static int io_submit_sqes(struct io_ring_ctx *ctx, struct sqe_submit *sqes,
+ unsigned int nr, bool has_user, bool mm_fault)
+{
+ struct io_submit_state state, *statep = NULL;
+ int ret, i, submitted = 0;
+
+ if (nr > IO_PLUG_THRESHOLD) {
+ io_submit_state_start(&state, ctx, nr);
+ statep = &state;
+ }
+
+ for (i = 0; i < nr; i++) {
+ if (unlikely(mm_fault)) {
+ ret = -EFAULT;
+ } else {
+ sqes[i].has_user = has_user;
+ sqes[i].needs_lock = true;
+ sqes[i].needs_fixed_file = true;
+ ret = io_submit_sqe(ctx, &sqes[i], statep);
+ }
+ if (!ret) {
+ submitted++;
+ continue;
+ }
+
+ io_cqring_add_event(ctx, sqes[i].sqe->user_data, ret, 0);
+ }
+
+ if (statep)
+ io_submit_state_end(&state);
+
+ return submitted;
+}
+
+static int io_sq_thread(void *data)
+{
+ struct sqe_submit sqes[IO_IOPOLL_BATCH];
+ struct io_ring_ctx *ctx = data;
+ struct mm_struct *cur_mm = NULL;
+ mm_segment_t old_fs;
+ DEFINE_WAIT(wait);
+ unsigned inflight;
+ unsigned long timeout;
+
+ old_fs = get_fs();
+ set_fs(USER_DS);
+
+ timeout = inflight = 0;
+ while (!kthread_should_stop() && !ctx->sqo_stop) {
+ bool all_fixed, mm_fault = false;
+ int i;
+
+ if (inflight) {
+ unsigned nr_events = 0;
+
+ if (ctx->flags & IORING_SETUP_IOPOLL) {
+ /*
+ * We disallow the app entering submit/complete
+ * with polling, but we still need to lock the
+ * ring to prevent racing with polled issue
+ * that got punted to a workqueue.
+ */
+ mutex_lock(&ctx->uring_lock);
+ io_iopoll_check(ctx, &nr_events, 0);
+ mutex_unlock(&ctx->uring_lock);
+ } else {
+ /*
+ * Normal IO, just pretend everything completed.
+ * We don't have to poll completions for that.
+ */
+ nr_events = inflight;
+ }
+
+ inflight -= nr_events;
+ if (!inflight)
+ timeout = jiffies + ctx->sq_thread_idle;
+ }
+
+ if (!io_get_sqring(ctx, &sqes[0])) {
+ /*
+ * We're polling. If we're within the defined idle
+ * period, then let us spin without work before going
+ * to sleep.
+ */
+ if (inflight || !time_after(jiffies, timeout)) {
+ cpu_relax();
+ continue;
+ }
+
+ /*
+ * Drop cur_mm before scheduling, we can't hold it for
+ * long periods (or over schedule()). Do this before
+ * adding ourselves to the waitqueue, as the unuse/drop
+ * may sleep.
+ */
+ if (cur_mm) {
+ unuse_mm(cur_mm);
+ mmput(cur_mm);
+ cur_mm = NULL;
+ }
+
+ prepare_to_wait(&ctx->sqo_wait, &wait,
+ TASK_INTERRUPTIBLE);
+
+ /* Tell userspace we may need a wakeup call */
+ ctx->sq_ring->flags |= IORING_SQ_NEED_WAKEUP;
+ smp_wmb();
+
+ if (!io_get_sqring(ctx, &sqes[0])) {
+ if (kthread_should_stop()) {
+ finish_wait(&ctx->sqo_wait, &wait);
+ break;
+ }
+ if (signal_pending(current))
+ flush_signals(current);
+ schedule();
+ finish_wait(&ctx->sqo_wait, &wait);
+
+ ctx->sq_ring->flags &= ~IORING_SQ_NEED_WAKEUP;
+ smp_wmb();
+ continue;
+ }
+ finish_wait(&ctx->sqo_wait, &wait);
+
+ ctx->sq_ring->flags &= ~IORING_SQ_NEED_WAKEUP;
+ smp_wmb();
+ }
+
+ i = 0;
+ all_fixed = true;
+ do {
+ if (all_fixed && io_sqe_needs_user(sqes[i].sqe))
+ all_fixed = false;
+
+ i++;
+ if (i == ARRAY_SIZE(sqes))
+ break;
+ } while (io_get_sqring(ctx, &sqes[i]));
+
+ io_commit_sqring(ctx);
+
+ /* Unless all new commands are FIXED regions, grab mm */
+ if (!all_fixed && !cur_mm) {
+ mm_fault = !mmget_not_zero(ctx->sqo_mm);
+ if (!mm_fault) {
+ use_mm(ctx->sqo_mm);
+ cur_mm = ctx->sqo_mm;
+ }
+ }
+
+ inflight += io_submit_sqes(ctx, sqes, i, cur_mm != NULL,
+ mm_fault);
+ }
+
+ io_iopoll_reap_events(ctx);
+
+ set_fs(old_fs);
+ if (cur_mm) {
+ unuse_mm(cur_mm);
+ mmput(cur_mm);
+ }
+ return 0;
+}
+
static int io_ring_submit(struct io_ring_ctx *ctx, unsigned int to_submit)
{
struct io_submit_state state, *statep = NULL;
@@ -1313,6 +1488,7 @@ static int io_ring_submit(struct io_ring_ctx *ctx, unsigned int to_submit)
s.has_user = true;
s.needs_lock = false;
+ s.needs_fixed_file = false;
ret = io_submit_sqe(ctx, &s, statep);
if (ret) {
@@ -1534,13 +1710,47 @@ static int io_sqe_files_register(struct io_ring_ctx *ctx, void __user *arg,
return ret;
}
-static int io_sq_offload_start(struct io_ring_ctx *ctx)
+static int io_sq_offload_start(struct io_ring_ctx *ctx,
+ struct io_uring_params *p)
{
int ret;
+ init_waitqueue_head(&ctx->sqo_wait);
mmgrab(current->mm);
ctx->sqo_mm = current->mm;
+ ctx->sq_thread_idle = msecs_to_jiffies(p->sq_thread_idle);
+ if (!ctx->sq_thread_idle)
+ ctx->sq_thread_idle = HZ;
+
+ ret = -EINVAL;
+ if (!cpu_possible(p->sq_thread_cpu))
+ goto err;
+
+ if (ctx->flags & IORING_SETUP_SQPOLL) {
+ if (p->flags & IORING_SETUP_SQ_AFF) {
+ int cpu;
+
+ cpu = array_index_nospec(p->sq_thread_cpu, NR_CPUS);
+ ctx->sqo_thread = kthread_create_on_cpu(io_sq_thread,
+ ctx, cpu,
+ "io_uring-sq");
+ } else {
+ ctx->sqo_thread = kthread_create(io_sq_thread, ctx,
+ "io_uring-sq");
+ }
+ if (IS_ERR(ctx->sqo_thread)) {
+ ret = PTR_ERR(ctx->sqo_thread);
+ ctx->sqo_thread = NULL;
+ goto err;
+ }
+ wake_up_process(ctx->sqo_thread);
+ } else if (p->flags & IORING_SETUP_SQ_AFF) {
+ /* Can't have SQ_AFF without SQPOLL */
+ ret = -EINVAL;
+ goto err;
+ }
+
/* Do QD, or 2 * CPUS, whatever is smallest */
ctx->sqo_wq = alloc_workqueue("io_ring-wq", WQ_UNBOUND | WQ_FREEZABLE,
min(ctx->sq_entries - 1, 2 * num_online_cpus()));
@@ -1551,6 +1761,12 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx)
return 0;
err:
+ if (ctx->sqo_thread) {
+ ctx->sqo_stop = 1;
+ mb();
+ kthread_stop(ctx->sqo_thread);
+ ctx->sqo_thread = NULL;
+ }
mmdrop(ctx->sqo_mm);
ctx->sqo_mm = NULL;
return ret;
@@ -1801,6 +2017,11 @@ static int io_sqe_buffer_register(struct io_ring_ctx *ctx, void __user *arg,
static void io_ring_ctx_free(struct io_ring_ctx *ctx)
{
+ if (ctx->sqo_thread) {
+ ctx->sqo_stop = 1;
+ mb();
+ kthread_stop(ctx->sqo_thread);
+ }
if (ctx->sqo_wq)
destroy_workqueue(ctx->sqo_wq);
if (ctx->sqo_mm)
@@ -1910,7 +2131,7 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
int submitted = 0;
struct fd f;
- if (flags & ~IORING_ENTER_GETEVENTS)
+ if (flags & ~(IORING_ENTER_GETEVENTS | IORING_ENTER_SQ_WAKEUP))
return -EINVAL;
f = fdget(fd);
@@ -1926,6 +2147,18 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
if (!percpu_ref_tryget(&ctx->refs))
goto out_fput;
+ /*
+ * For SQ polling, the thread will do all submissions and completions.
+ * Just return the requested submit count, and wake the thread if
+ * we were asked to.
+ */
+ if (ctx->flags & IORING_SETUP_SQPOLL) {
+ if (flags & IORING_ENTER_SQ_WAKEUP)
+ wake_up(&ctx->sqo_wait);
+ submitted = to_submit;
+ goto out_ctx;
+ }
+
if (to_submit) {
to_submit = min(to_submit, ctx->sq_entries);
@@ -2103,7 +2336,7 @@ static int io_uring_create(unsigned entries, struct io_uring_params *p)
if (ret)
goto err;
- ret = io_sq_offload_start(ctx);
+ ret = io_sq_offload_start(ctx, p);
if (ret)
goto err;
@@ -2151,7 +2384,8 @@ static long io_uring_setup(u32 entries, struct io_uring_params __user *params)
return -EINVAL;
}
- if (p.flags & ~IORING_SETUP_IOPOLL)
+ if (p.flags & ~(IORING_SETUP_IOPOLL | IORING_SETUP_SQPOLL |
+ IORING_SETUP_SQ_AFF))
return -EINVAL;
ret = io_uring_create(entries, &p);
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 6257478d55e9..0ec74bab8dbe 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -42,6 +42,8 @@ struct io_uring_sqe {
* io_uring_setup() flags
*/
#define IORING_SETUP_IOPOLL (1U << 0) /* io_context is polled */
+#define IORING_SETUP_SQPOLL (1U << 1) /* SQ poll thread */
+#define IORING_SETUP_SQ_AFF (1U << 2) /* sq_thread_cpu is valid */
#define IORING_OP_NOP 0
#define IORING_OP_READV 1
@@ -86,6 +88,11 @@ struct io_sqring_offsets {
__u64 resv2;
};
+/*
+ * sq_ring->flags
+ */
+#define IORING_SQ_NEED_WAKEUP (1U << 0) /* needs io_uring_enter wakeup */
+
struct io_cqring_offsets {
__u32 head;
__u32 tail;
@@ -100,6 +107,7 @@ struct io_cqring_offsets {
* io_uring_enter(2) flags
*/
#define IORING_ENTER_GETEVENTS (1U << 0)
+#define IORING_ENTER_SQ_WAKEUP (1U << 1)
/*
* Passed in for io_uring_setup(2). Copied back with updated info on success
@@ -108,7 +116,9 @@ struct io_uring_params {
__u32 sq_entries;
__u32 cq_entries;
__u32 flags;
- __u32 resv[7];
+ __u32 sq_thread_cpu;
+ __u32 sq_thread_idle;
+ __u32 resv[5];
struct io_sqring_offsets sq_off;
struct io_cqring_offsets cq_off;
};
--
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 14/19] io_uring: add file set registration
From: Jens Axboe @ 2019-02-11 19:00 UTC (permalink / raw)
To: linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro, Jens Axboe
In-Reply-To: <20190211190049.7888-1-axboe@kernel.dk>
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.
Reviewed-by: Hannes Reinecke <hare@suse.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
fs/io_uring.c | 267 ++++++++++++++++++++++++++++++----
include/uapi/linux/io_uring.h | 9 +-
2 files changed, 246 insertions(+), 30 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 0eba20d18f53..167c7f96666f 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -49,6 +49,7 @@
#include <linux/net.h>
#include <net/sock.h>
#include <net/af_unix.h>
+#include <net/scm.h>
#include <linux/anon_inodes.h>
#include <linux/sched/mm.h>
#include <linux/uaccess.h>
@@ -61,6 +62,7 @@
#include "internal.h"
#define IORING_MAX_ENTRIES 4096
+#define IORING_MAX_FIXED_FILES 1024
struct io_uring {
u32 head ____cacheline_aligned_in_smp;
@@ -123,6 +125,14 @@ struct io_ring_ctx {
struct fasync_struct *cq_fasync;
} ____cacheline_aligned_in_smp;
+ /*
+ * If used, fixed file set. Writers must ensure that ->refs is dead,
+ * readers must ensure that ->refs is alive as long as the file* is
+ * used. Only updated through io_uring_register(2).
+ */
+ struct file **user_files;
+ unsigned nr_user_files;
+
/* if used, fixed mapped user buffers */
unsigned nr_user_bufs;
struct io_mapped_ubuf *user_bufs;
@@ -170,6 +180,7 @@ struct io_kiocb {
unsigned int flags;
#define REQ_F_FORCE_NONBLOCK 1 /* inline submission attempt */
#define REQ_F_IOPOLL_COMPLETED 2 /* polled IO has completed */
+#define REQ_F_FIXED_FILE 4 /* ctx owns file */
u64 user_data;
u64 error;
@@ -404,15 +415,17 @@ static void io_iopoll_complete(struct io_ring_ctx *ctx, unsigned int *nr_events,
* Batched puts of the same file, to avoid dirtying the
* file usage count multiple times, if avoidable.
*/
- if (!file) {
- file = req->rw.ki_filp;
- file_count = 1;
- } else if (file == req->rw.ki_filp) {
- file_count++;
- } else {
- fput_many(file, file_count);
- file = req->rw.ki_filp;
- file_count = 1;
+ if (!(req->flags & REQ_F_FIXED_FILE)) {
+ if (!file) {
+ file = req->rw.ki_filp;
+ file_count = 1;
+ } else if (file == req->rw.ki_filp) {
+ file_count++;
+ } else {
+ fput_many(file, file_count);
+ file = req->rw.ki_filp;
+ file_count = 1;
+ }
}
if (to_free == ARRAY_SIZE(reqs))
@@ -544,13 +557,19 @@ static void kiocb_end_write(struct kiocb *kiocb)
}
}
+static void io_fput(struct io_kiocb *req)
+{
+ if (!(req->flags & REQ_F_FIXED_FILE))
+ fput(req->rw.ki_filp);
+}
+
static void io_complete_rw(struct kiocb *kiocb, long res, long res2)
{
struct io_kiocb *req = container_of(kiocb, struct io_kiocb, rw);
kiocb_end_write(kiocb);
- fput(kiocb->ki_filp);
+ io_fput(req);
io_cqring_add_event(req->ctx, req->user_data, res, 0);
io_free_req(req);
}
@@ -666,19 +685,29 @@ static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe,
{
struct io_ring_ctx *ctx = req->ctx;
struct kiocb *kiocb = &req->rw;
- unsigned ioprio;
+ unsigned ioprio, flags;
int fd, ret;
/* For -EAGAIN retry, everything is already prepped */
if (kiocb->ki_filp)
return 0;
+ flags = READ_ONCE(sqe->flags);
fd = READ_ONCE(sqe->fd);
- kiocb->ki_filp = io_file_get(state, fd);
- if (unlikely(!kiocb->ki_filp))
- return -EBADF;
- if (force_nonblock && !io_file_supports_async(kiocb->ki_filp))
- force_nonblock = false;
+
+ if (flags & IOSQE_FIXED_FILE) {
+ if (unlikely(!ctx->user_files ||
+ (unsigned) fd >= ctx->nr_user_files))
+ return -EBADF;
+ kiocb->ki_filp = ctx->user_files[fd];
+ req->flags |= REQ_F_FIXED_FILE;
+ } else {
+ kiocb->ki_filp = io_file_get(state, fd);
+ if (unlikely(!kiocb->ki_filp))
+ return -EBADF;
+ if (force_nonblock && !io_file_supports_async(kiocb->ki_filp))
+ force_nonblock = false;
+ }
kiocb->ki_pos = READ_ONCE(sqe->off);
kiocb->ki_flags = iocb_flags(kiocb->ki_filp);
kiocb->ki_hint = ki_hint_validate(file_write_hint(kiocb->ki_filp));
@@ -718,10 +747,14 @@ static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe,
}
return 0;
out_fput:
- /* in case of error, we didn't use this file reference. drop it. */
- if (state)
- state->used_refs--;
- io_file_put(state, kiocb->ki_filp);
+ if (!(flags & IOSQE_FIXED_FILE)) {
+ /*
+ * in case of error, we didn't use this file reference. drop it.
+ */
+ if (state)
+ state->used_refs--;
+ io_file_put(state, kiocb->ki_filp);
+ }
return ret;
}
@@ -863,7 +896,7 @@ static ssize_t io_read(struct io_kiocb *req, const struct sqe_submit *s,
out_fput:
/* Hold on to the file for -EAGAIN */
if (unlikely(ret && ret != -EAGAIN))
- fput(file);
+ io_fput(req);
return ret;
}
@@ -917,7 +950,7 @@ static ssize_t io_write(struct io_kiocb *req, const struct sqe_submit *s,
kfree(iovec);
out_fput:
if (unlikely(ret))
- fput(file);
+ io_fput(req);
return ret;
}
@@ -940,7 +973,7 @@ static int io_nop(struct io_kiocb *req, u64 user_data)
*/
if (req->rw.ki_filp) {
err = -EBADF;
- fput(req->rw.ki_filp);
+ io_fput(req);
}
io_cqring_add_event(ctx, user_data, err, 0);
io_free_req(req);
@@ -949,21 +982,32 @@ static int io_nop(struct io_kiocb *req, u64 user_data)
static int io_prep_fsync(struct io_kiocb *req, const struct io_uring_sqe *sqe)
{
+ struct io_ring_ctx *ctx = req->ctx;
+ unsigned flags;
int fd;
/* Prep already done */
if (req->rw.ki_filp)
return 0;
- if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
+ if (unlikely(ctx->flags & IORING_SETUP_IOPOLL))
return -EINVAL;
if (unlikely(sqe->addr || sqe->ioprio || sqe->buf_index))
return -EINVAL;
fd = READ_ONCE(sqe->fd);
- req->rw.ki_filp = fget(fd);
- if (unlikely(!req->rw.ki_filp))
- return -EBADF;
+ flags = READ_ONCE(sqe->flags);
+
+ if (flags & IOSQE_FIXED_FILE) {
+ if (unlikely(!ctx->user_files || fd >= ctx->nr_user_files))
+ return -EBADF;
+ req->rw.ki_filp = ctx->user_files[fd];
+ req->flags |= REQ_F_FIXED_FILE;
+ } else {
+ req->rw.ki_filp = fget(fd);
+ if (unlikely(!req->rw.ki_filp))
+ return -EBADF;
+ }
return 0;
}
@@ -993,7 +1037,7 @@ static int io_fsync(struct io_kiocb *req, const struct io_uring_sqe *sqe,
end > 0 ? end : LLONG_MAX,
fsync_flags & IORING_FSYNC_DATASYNC);
- fput(req->rw.ki_filp);
+ io_fput(req);
io_cqring_add_event(req->ctx, sqe->user_data, ret, 0);
io_free_req(req);
return 0;
@@ -1132,7 +1176,7 @@ static int io_submit_sqe(struct io_ring_ctx *ctx, struct sqe_submit *s,
ssize_t ret;
/* enforce forwards compatibility on users */
- if (unlikely(s->sqe->flags))
+ if (unlikely(s->sqe->flags & ~IOSQE_FIXED_FILE))
return -EINVAL;
req = io_get_req(ctx, state);
@@ -1335,6 +1379,161 @@ static int io_cqring_wait(struct io_ring_ctx *ctx, int min_events,
return READ_ONCE(ring->r.head) == READ_ONCE(ring->r.tail) ? ret : 0;
}
+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)
+/*
+ * Ensure the UNIX gc is aware of our file set, so we are certain that
+ * the io_uring can be safely unregistered on process exit, even if we have
+ * loops in the file referencing.
+ */
+static int __io_sqe_files_scm(struct io_ring_ctx *ctx, int nr, int offset)
+{
+ struct sock *sk = ctx->ring_sock->sk;
+ 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 = 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]);
+ }
+
+ fpl->max = fpl->count = nr;
+ UNIXCB(skb).fp = fpl;
+ refcount_add(skb->truesize, &sk->sk_wmem_alloc);
+ skb_queue_head(&sk->sk_receive_queue, skb);
+
+ for (i = 0; i < nr; i++)
+ fput(fpl->fp[i]);
+
+ return 0;
+}
+
+/*
+ * If UNIX sockets are enabled, fd passing can cause a reference cycle which
+ * causes regular reference counting to break down. We rely on the UNIX
+ * garbage collection to take care of this problem for us.
+ */
+static int io_sqe_files_scm(struct io_ring_ctx *ctx)
+{
+ unsigned left, total;
+ int ret = 0;
+
+ total = 0;
+ left = ctx->nr_user_files;
+ while (left) {
+ unsigned this_files = min_t(unsigned, left, SCM_MAX_FD);
+ int ret;
+
+ ret = __io_sqe_files_scm(ctx, this_files, total);
+ if (ret)
+ break;
+ left -= this_files;
+ total += this_files;
+ }
+
+ return ret;
+}
+#else
+static int io_sqe_files_scm(struct io_ring_ctx *ctx)
+{
+ return 0;
+}
+#endif
+
+static int io_sqe_files_register(struct io_ring_ctx *ctx, void __user *arg,
+ unsigned nr_args)
+{
+ __s32 __user *fds = (__s32 __user *) arg;
+ int fd, ret = 0;
+ unsigned i;
+
+ if (ctx->user_files)
+ return -EBUSY;
+ if (!nr_args)
+ return -EINVAL;
+ if (nr_args > IORING_MAX_FIXED_FILES)
+ return -EMFILE;
+
+ ctx->user_files = kcalloc(nr_args, sizeof(struct file *), GFP_KERNEL);
+ if (!ctx->user_files)
+ return -ENOMEM;
+
+ for (i = 0; i < nr_args; i++) {
+ ret = -EFAULT;
+ if (copy_from_user(&fd, &fds[i], sizeof(fd)))
+ break;
+
+ ctx->user_files[i] = fget(fd);
+
+ ret = -EBADF;
+ if (!ctx->user_files[i])
+ break;
+ /*
+ * Don't allow io_uring instances to be registered. If UNIX
+ * isn't enabled, then this causes a reference cycle and this
+ * instance can never get freed. If UNIX is enabled we'll
+ * handle it just fine, but there's still no point in allowing
+ * a ring fd as it doesn't support regular read/write anyway.
+ */
+ if (ctx->user_files[i]->f_op == &io_uring_fops) {
+ fput(ctx->user_files[i]);
+ break;
+ }
+ ctx->nr_user_files++;
+ ret = 0;
+ }
+
+ if (!ret)
+ ret = io_sqe_files_scm(ctx);
+ if (ret)
+ io_sqe_files_unregister(ctx);
+
+ return ret;
+}
+
static int io_sq_offload_start(struct io_ring_ctx *ctx)
{
int ret;
@@ -1609,6 +1808,7 @@ static void io_ring_ctx_free(struct io_ring_ctx *ctx)
io_iopoll_reap_events(ctx);
io_sqe_buffer_unregister(ctx);
+ io_sqe_files_unregister(ctx);
#if defined(CONFIG_UNIX)
if (ctx->ring_sock)
@@ -1988,6 +2188,15 @@ static int __io_uring_register(struct io_ring_ctx *ctx, unsigned opcode,
break;
ret = io_sqe_buffer_unregister(ctx);
break;
+ case IORING_REGISTER_FILES:
+ ret = io_sqe_files_register(ctx, arg, nr_args);
+ break;
+ case IORING_UNREGISTER_FILES:
+ ret = -EINVAL;
+ if (arg || nr_args)
+ break;
+ ret = io_sqe_files_unregister(ctx);
+ break;
default:
ret = -EINVAL;
break;
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index cf28f7a11f12..6257478d55e9 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -16,7 +16,7 @@
*/
struct io_uring_sqe {
__u8 opcode; /* type of operation for this sqe */
- __u8 flags; /* as of now unused */
+ __u8 flags; /* IOSQE_ flags */
__u16 ioprio; /* ioprio for the request */
__s32 fd; /* file descriptor to do IO on */
__u64 off; /* offset into file */
@@ -33,6 +33,11 @@ struct io_uring_sqe {
};
};
+/*
+ * sqe->flags
+ */
+#define IOSQE_FIXED_FILE (1U << 0) /* use fixed fileset */
+
/*
* io_uring_setup() flags
*/
@@ -113,5 +118,7 @@ struct io_uring_params {
*/
#define IORING_REGISTER_BUFFERS 0
#define IORING_UNREGISTER_BUFFERS 1
+#define IORING_REGISTER_FILES 2
+#define IORING_UNREGISTER_FILES 3
#endif
--
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 13/19] net: split out functions related to registering inflight socket files
From: Jens Axboe @ 2019-02-11 19:00 UTC (permalink / raw)
To: linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro, Jens Axboe
In-Reply-To: <20190211190049.7888-1-axboe@kernel.dk>
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.
Reviewed-by: Hannes Reinecke <hare@suse.com>
Acked-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
include/net/af_unix.h | 1 +
net/Makefile | 2 +-
net/unix/Kconfig | 5 ++
net/unix/Makefile | 2 +
net/unix/af_unix.c | 63 +-----------------
net/unix/garbage.c | 71 +-------------------
net/unix/scm.c | 151 ++++++++++++++++++++++++++++++++++++++++++
net/unix/scm.h | 10 +++
8 files changed, 174 insertions(+), 131 deletions(-)
create mode 100644 net/unix/scm.c
create mode 100644 net/unix/scm.h
diff --git a/include/net/af_unix.h b/include/net/af_unix.h
index ddbba838d048..3426d6dacc45 100644
--- a/include/net/af_unix.h
+++ b/include/net/af_unix.h
@@ -10,6 +10,7 @@
void unix_inflight(struct user_struct *user, struct file *fp);
void unix_notinflight(struct user_struct *user, struct file *fp);
+void unix_destruct_scm(struct sk_buff *skb);
void unix_gc(void);
void wait_for_unix_gc(void);
struct sock *unix_get_socket(struct file *filp);
diff --git a/net/Makefile b/net/Makefile
index bdaf53925acd..449fc0b221f8 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -18,7 +18,7 @@ obj-$(CONFIG_NETFILTER) += netfilter/
obj-$(CONFIG_INET) += ipv4/
obj-$(CONFIG_TLS) += tls/
obj-$(CONFIG_XFRM) += xfrm/
-obj-$(CONFIG_UNIX) += unix/
+obj-$(CONFIG_UNIX_SCM) += unix/
obj-$(CONFIG_NET) += ipv6/
obj-$(CONFIG_BPFILTER) += bpfilter/
obj-$(CONFIG_PACKET) += packet/
diff --git a/net/unix/Kconfig b/net/unix/Kconfig
index 8b31ab85d050..3b9e450656a4 100644
--- a/net/unix/Kconfig
+++ b/net/unix/Kconfig
@@ -19,6 +19,11 @@ config UNIX
Say Y unless you know what you are doing.
+config UNIX_SCM
+ bool
+ depends on UNIX
+ default y
+
config UNIX_DIAG
tristate "UNIX: socket monitoring interface"
depends on UNIX
diff --git a/net/unix/Makefile b/net/unix/Makefile
index ffd0a275c3a7..54e58cc4f945 100644
--- a/net/unix/Makefile
+++ b/net/unix/Makefile
@@ -10,3 +10,5 @@ unix-$(CONFIG_SYSCTL) += sysctl_net_unix.o
obj-$(CONFIG_UNIX_DIAG) += unix_diag.o
unix_diag-y := diag.o
+
+obj-$(CONFIG_UNIX_SCM) += scm.o
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 74d1eed7cbd4..2ce32dbb2feb 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -119,6 +119,8 @@
#include <linux/freezer.h>
#include <linux/file.h>
+#include "scm.h"
+
struct hlist_head unix_socket_table[2 * UNIX_HASH_SIZE];
EXPORT_SYMBOL_GPL(unix_socket_table);
DEFINE_SPINLOCK(unix_table_lock);
@@ -1486,67 +1488,6 @@ static int unix_getname(struct socket *sock, struct sockaddr *uaddr, int peer)
return err;
}
-static void unix_detach_fds(struct scm_cookie *scm, struct sk_buff *skb)
-{
- int i;
-
- scm->fp = UNIXCB(skb).fp;
- UNIXCB(skb).fp = NULL;
-
- for (i = scm->fp->count-1; i >= 0; i--)
- unix_notinflight(scm->fp->user, scm->fp->fp[i]);
-}
-
-static void unix_destruct_scm(struct sk_buff *skb)
-{
- struct scm_cookie scm;
- memset(&scm, 0, sizeof(scm));
- scm.pid = UNIXCB(skb).pid;
- if (UNIXCB(skb).fp)
- unix_detach_fds(&scm, skb);
-
- /* Alas, it calls VFS */
- /* So fscking what? fput() had been SMP-safe since the last Summer */
- scm_destroy(&scm);
- sock_wfree(skb);
-}
-
-/*
- * The "user->unix_inflight" variable is protected by the garbage
- * collection lock, and we just read it locklessly here. If you go
- * over the limit, there might be a tiny race in actually noticing
- * it across threads. Tough.
- */
-static inline bool too_many_unix_fds(struct task_struct *p)
-{
- struct user_struct *user = current_user();
-
- if (unlikely(user->unix_inflight > task_rlimit(p, RLIMIT_NOFILE)))
- return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
- return false;
-}
-
-static int unix_attach_fds(struct scm_cookie *scm, struct sk_buff *skb)
-{
- int i;
-
- if (too_many_unix_fds(current))
- return -ETOOMANYREFS;
-
- /*
- * Need to duplicate file references for the sake of garbage
- * collection. Otherwise a socket in the fps might become a
- * candidate for GC while the skb is not yet queued.
- */
- UNIXCB(skb).fp = scm_fp_dup(scm->fp);
- if (!UNIXCB(skb).fp)
- return -ENOMEM;
-
- for (i = scm->fp->count - 1; i >= 0; i--)
- unix_inflight(scm->fp->user, scm->fp->fp[i]);
- return 0;
-}
-
static int unix_scm_to_skb(struct scm_cookie *scm, struct sk_buff *skb, bool send_fds)
{
int err = 0;
diff --git a/net/unix/garbage.c b/net/unix/garbage.c
index f81854d74c7d..8bbe1b8e4ff7 100644
--- a/net/unix/garbage.c
+++ b/net/unix/garbage.c
@@ -86,80 +86,13 @@
#include <net/scm.h>
#include <net/tcp_states.h>
+#include "scm.h"
+
/* Internal data structures and random procedures: */
-static LIST_HEAD(gc_inflight_list);
static LIST_HEAD(gc_candidates);
-static DEFINE_SPINLOCK(unix_gc_lock);
static DECLARE_WAIT_QUEUE_HEAD(unix_gc_wait);
-unsigned int unix_tot_inflight;
-
-struct sock *unix_get_socket(struct file *filp)
-{
- struct sock *u_sock = NULL;
- struct inode *inode = file_inode(filp);
-
- /* Socket ? */
- if (S_ISSOCK(inode->i_mode) && !(filp->f_mode & FMODE_PATH)) {
- struct socket *sock = SOCKET_I(inode);
- struct sock *s = sock->sk;
-
- /* PF_UNIX ? */
- if (s && sock->ops && sock->ops->family == PF_UNIX)
- u_sock = s;
- } else {
- /* Could be an io_uring instance */
- u_sock = io_uring_get_socket(filp);
- }
- return u_sock;
-}
-
-/* Keep the number of times in flight count for the file
- * descriptor if it is for an AF_UNIX socket.
- */
-
-void unix_inflight(struct user_struct *user, struct file *fp)
-{
- struct sock *s = unix_get_socket(fp);
-
- spin_lock(&unix_gc_lock);
-
- if (s) {
- struct unix_sock *u = unix_sk(s);
-
- if (atomic_long_inc_return(&u->inflight) == 1) {
- BUG_ON(!list_empty(&u->link));
- list_add_tail(&u->link, &gc_inflight_list);
- } else {
- BUG_ON(list_empty(&u->link));
- }
- unix_tot_inflight++;
- }
- user->unix_inflight++;
- spin_unlock(&unix_gc_lock);
-}
-
-void unix_notinflight(struct user_struct *user, struct file *fp)
-{
- struct sock *s = unix_get_socket(fp);
-
- spin_lock(&unix_gc_lock);
-
- if (s) {
- struct unix_sock *u = unix_sk(s);
-
- BUG_ON(!atomic_long_read(&u->inflight));
- BUG_ON(list_empty(&u->link));
-
- if (atomic_long_dec_and_test(&u->inflight))
- list_del_init(&u->link);
- unix_tot_inflight--;
- }
- user->unix_inflight--;
- spin_unlock(&unix_gc_lock);
-}
-
static void scan_inflight(struct sock *x, void (*func)(struct unix_sock *),
struct sk_buff_head *hitlist)
{
diff --git a/net/unix/scm.c b/net/unix/scm.c
new file mode 100644
index 000000000000..8c40f2b32392
--- /dev/null
+++ b/net/unix/scm.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/string.h>
+#include <linux/socket.h>
+#include <linux/net.h>
+#include <linux/fs.h>
+#include <net/af_unix.h>
+#include <net/scm.h>
+#include <linux/init.h>
+
+#include "scm.h"
+
+unsigned int unix_tot_inflight;
+EXPORT_SYMBOL(unix_tot_inflight);
+
+LIST_HEAD(gc_inflight_list);
+EXPORT_SYMBOL(gc_inflight_list);
+
+DEFINE_SPINLOCK(unix_gc_lock);
+EXPORT_SYMBOL(unix_gc_lock);
+
+struct sock *unix_get_socket(struct file *filp)
+{
+ struct sock *u_sock = NULL;
+ struct inode *inode = file_inode(filp);
+
+ /* Socket ? */
+ if (S_ISSOCK(inode->i_mode) && !(filp->f_mode & FMODE_PATH)) {
+ struct socket *sock = SOCKET_I(inode);
+ struct sock *s = sock->sk;
+
+ /* PF_UNIX ? */
+ if (s && sock->ops && sock->ops->family == PF_UNIX)
+ u_sock = s;
+ } else {
+ /* Could be an io_uring instance */
+ u_sock = io_uring_get_socket(filp);
+ }
+ return u_sock;
+}
+EXPORT_SYMBOL(unix_get_socket);
+
+/* Keep the number of times in flight count for the file
+ * descriptor if it is for an AF_UNIX socket.
+ */
+void unix_inflight(struct user_struct *user, struct file *fp)
+{
+ struct sock *s = unix_get_socket(fp);
+
+ spin_lock(&unix_gc_lock);
+
+ if (s) {
+ struct unix_sock *u = unix_sk(s);
+
+ if (atomic_long_inc_return(&u->inflight) == 1) {
+ BUG_ON(!list_empty(&u->link));
+ list_add_tail(&u->link, &gc_inflight_list);
+ } else {
+ BUG_ON(list_empty(&u->link));
+ }
+ unix_tot_inflight++;
+ }
+ user->unix_inflight++;
+ spin_unlock(&unix_gc_lock);
+}
+
+void unix_notinflight(struct user_struct *user, struct file *fp)
+{
+ struct sock *s = unix_get_socket(fp);
+
+ spin_lock(&unix_gc_lock);
+
+ if (s) {
+ struct unix_sock *u = unix_sk(s);
+
+ BUG_ON(!atomic_long_read(&u->inflight));
+ BUG_ON(list_empty(&u->link));
+
+ if (atomic_long_dec_and_test(&u->inflight))
+ list_del_init(&u->link);
+ unix_tot_inflight--;
+ }
+ user->unix_inflight--;
+ spin_unlock(&unix_gc_lock);
+}
+
+/*
+ * The "user->unix_inflight" variable is protected by the garbage
+ * collection lock, and we just read it locklessly here. If you go
+ * over the limit, there might be a tiny race in actually noticing
+ * it across threads. Tough.
+ */
+static inline bool too_many_unix_fds(struct task_struct *p)
+{
+ struct user_struct *user = current_user();
+
+ if (unlikely(user->unix_inflight > task_rlimit(p, RLIMIT_NOFILE)))
+ return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
+ return false;
+}
+
+int unix_attach_fds(struct scm_cookie *scm, struct sk_buff *skb)
+{
+ int i;
+
+ if (too_many_unix_fds(current))
+ return -ETOOMANYREFS;
+
+ /*
+ * Need to duplicate file references for the sake of garbage
+ * collection. Otherwise a socket in the fps might become a
+ * candidate for GC while the skb is not yet queued.
+ */
+ UNIXCB(skb).fp = scm_fp_dup(scm->fp);
+ if (!UNIXCB(skb).fp)
+ return -ENOMEM;
+
+ for (i = scm->fp->count - 1; i >= 0; i--)
+ unix_inflight(scm->fp->user, scm->fp->fp[i]);
+ return 0;
+}
+EXPORT_SYMBOL(unix_attach_fds);
+
+void unix_detach_fds(struct scm_cookie *scm, struct sk_buff *skb)
+{
+ int i;
+
+ scm->fp = UNIXCB(skb).fp;
+ UNIXCB(skb).fp = NULL;
+
+ for (i = scm->fp->count-1; i >= 0; i--)
+ unix_notinflight(scm->fp->user, scm->fp->fp[i]);
+}
+EXPORT_SYMBOL(unix_detach_fds);
+
+void unix_destruct_scm(struct sk_buff *skb)
+{
+ struct scm_cookie scm;
+
+ memset(&scm, 0, sizeof(scm));
+ scm.pid = UNIXCB(skb).pid;
+ if (UNIXCB(skb).fp)
+ unix_detach_fds(&scm, skb);
+
+ /* Alas, it calls VFS */
+ /* So fscking what? fput() had been SMP-safe since the last Summer */
+ scm_destroy(&scm);
+ sock_wfree(skb);
+}
+EXPORT_SYMBOL(unix_destruct_scm);
diff --git a/net/unix/scm.h b/net/unix/scm.h
new file mode 100644
index 000000000000..5a255a477f16
--- /dev/null
+++ b/net/unix/scm.h
@@ -0,0 +1,10 @@
+#ifndef NET_UNIX_SCM_H
+#define NET_UNIX_SCM_H
+
+extern struct list_head gc_inflight_list;
+extern spinlock_t unix_gc_lock;
+
+int unix_attach_fds(struct scm_cookie *scm, struct sk_buff *skb);
+void unix_detach_fds(struct scm_cookie *scm, struct sk_buff *skb);
+
+#endif
--
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
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