From: Vlastimil Babka <vbabka@suse.cz>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>,
Andrew Morton <akpm@linux-foundation.org>,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, Michal Hocko <mhocko@kernel.org>,
netdev@vger.kernel.org, Linux API <linux-api@vger.kernel.org>,
linux-man@vger.kernel.org
Subject: Re: [PATCH v2] fs/select: add vmalloc fallback for select(2)
Date: Thu, 22 Sep 2016 19:55:09 +0200 [thread overview]
Message-ID: <a212f313-1f34-7c83-3aab-b45374875493@suse.cz> (raw)
In-Reply-To: <1474564068.23058.144.camel@edumazet-glaptop3.roam.corp.google.com>
On 09/22/2016 07:07 PM, Eric Dumazet wrote:
> On Thu, 2016-09-22 at 18:56 +0200, Vlastimil Babka wrote:
>> On 09/22/2016 06:49 PM, Eric Dumazet wrote:
>> > On Thu, 2016-09-22 at 18:43 +0200, Vlastimil Babka wrote:
>> >> The select(2) syscall performs a kmalloc(size, GFP_KERNEL) where size grows
>> >> with the number of fds passed. We had a customer report page allocation
>> >> failures of order-4 for this allocation. This is a costly order, so it might
>> >> easily fail, as the VM expects such allocation to have a lower-order fallback.
>> >>
>> >> Such trivial fallback is vmalloc(), as the memory doesn't have to be
>> >> physically contiguous. Also the allocation is temporary for the duration of the
>> >> syscall, so it's unlikely to stress vmalloc too much.
>> >
>> > vmalloc() uses a vmap_area_lock spinlock, and TLB flushes.
>> >
>> > So I guess allowing vmalloc() being called from an innocent application
>> > doing a select() might be dangerous, especially if this select() happens
>> > thousands of time per second.
>>
>> Isn't seq_buf_alloc() similarly exposed? And ipc_alloc()?
>
> Possibly.
>
> We don't have a library function (attempting kmalloc(), fallback to
> vmalloc() presumably to avoid abuses, but I guess some patches were
> accepted without thinking about this.
So in the case of select() it seems like the memory we need 6 bits per file
descriptor, multiplied by the highest possible file descriptor (nfds) as passed
to the syscall. According to the man page of select:
EINVAL nfds is negative or exceeds the RLIMIT_NOFILE resource limit (see
getrlimit(2)).
The code actually seems to silently cap the value instead of returning EINVAL
though? (IIUC):
/* max_fds can increase, so grab it once to avoid race */
rcu_read_lock();
fdt = files_fdtable(current->files);
max_fds = fdt->max_fds;
rcu_read_unlock();
if (n > max_fds)
n = max_fds;
The default for this cap seems to be 1024 where I checked (again, IIUC, it's
what ulimit -n returns?). I wasn't able to change it to more than 2048, which
makes the bitmaps still below PAGE_SIZE.
So if I get that right, the system admin would have to allow really large
RLIMIT_NOFILE to even make vmalloc() possible here. So I don't see it as a large
concern?
Vlastimil
WARNING: multiple messages have this Message-ID (diff)
From: Vlastimil Babka <vbabka@suse.cz>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>,
Andrew Morton <akpm@linux-foundation.org>,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, Michal Hocko <mhocko@kernel.org>,
netdev@vger.kernel.org, Linux API <linux-api@vger.kernel.org>,
linux-man@vger.kernel.org
Subject: Re: [PATCH v2] fs/select: add vmalloc fallback for select(2)
Date: Thu, 22 Sep 2016 19:55:09 +0200 [thread overview]
Message-ID: <a212f313-1f34-7c83-3aab-b45374875493@suse.cz> (raw)
In-Reply-To: <1474564068.23058.144.camel@edumazet-glaptop3.roam.corp.google.com>
On 09/22/2016 07:07 PM, Eric Dumazet wrote:
> On Thu, 2016-09-22 at 18:56 +0200, Vlastimil Babka wrote:
>> On 09/22/2016 06:49 PM, Eric Dumazet wrote:
>> > On Thu, 2016-09-22 at 18:43 +0200, Vlastimil Babka wrote:
>> >> The select(2) syscall performs a kmalloc(size, GFP_KERNEL) where size grows
>> >> with the number of fds passed. We had a customer report page allocation
>> >> failures of order-4 for this allocation. This is a costly order, so it might
>> >> easily fail, as the VM expects such allocation to have a lower-order fallback.
>> >>
>> >> Such trivial fallback is vmalloc(), as the memory doesn't have to be
>> >> physically contiguous. Also the allocation is temporary for the duration of the
>> >> syscall, so it's unlikely to stress vmalloc too much.
>> >
>> > vmalloc() uses a vmap_area_lock spinlock, and TLB flushes.
>> >
>> > So I guess allowing vmalloc() being called from an innocent application
>> > doing a select() might be dangerous, especially if this select() happens
>> > thousands of time per second.
>>
>> Isn't seq_buf_alloc() similarly exposed? And ipc_alloc()?
>
> Possibly.
>
> We don't have a library function (attempting kmalloc(), fallback to
> vmalloc() presumably to avoid abuses, but I guess some patches were
> accepted without thinking about this.
So in the case of select() it seems like the memory we need 6 bits per file
descriptor, multiplied by the highest possible file descriptor (nfds) as passed
to the syscall. According to the man page of select:
EINVAL nfds is negative or exceeds the RLIMIT_NOFILE resource limit (see
getrlimit(2)).
The code actually seems to silently cap the value instead of returning EINVAL
though? (IIUC):
/* max_fds can increase, so grab it once to avoid race */
rcu_read_lock();
fdt = files_fdtable(current->files);
max_fds = fdt->max_fds;
rcu_read_unlock();
if (n > max_fds)
n = max_fds;
The default for this cap seems to be 1024 where I checked (again, IIUC, it's
what ulimit -n returns?). I wasn't able to change it to more than 2048, which
makes the bitmaps still below PAGE_SIZE.
So if I get that right, the system admin would have to allow really large
RLIMIT_NOFILE to even make vmalloc() possible here. So I don't see it as a large
concern?
Vlastimil
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2016-09-22 17:55 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-22 16:43 [PATCH v2] fs/select: add vmalloc fallback for select(2) Vlastimil Babka
2016-09-22 16:43 ` Vlastimil Babka
2016-09-22 16:49 ` Eric Dumazet
2016-09-22 16:49 ` Eric Dumazet
2016-09-22 16:49 ` Eric Dumazet
2016-09-22 16:56 ` Vlastimil Babka
2016-09-22 16:56 ` Vlastimil Babka
2016-09-22 17:07 ` Eric Dumazet
2016-09-22 17:07 ` Eric Dumazet
2016-09-22 17:07 ` Eric Dumazet
2016-09-22 17:55 ` Vlastimil Babka [this message]
2016-09-22 17:55 ` Vlastimil Babka
2016-09-23 9:42 ` David Laight
2016-09-23 9:58 ` Vlastimil Babka
2016-09-23 9:58 ` Vlastimil Babka
2016-09-23 9:58 ` Vlastimil Babka
2016-09-23 13:35 ` David Laight
2016-09-26 10:01 ` Vlastimil Babka
2016-09-26 10:01 ` Vlastimil Babka
2016-09-26 15:02 ` David Laight
2016-09-25 18:50 ` Andi Kleen
2016-09-25 18:50 ` Andi Kleen
2016-09-27 0:01 ` Andrew Morton
2016-09-27 0:01 ` Andrew Morton
2016-09-27 1:38 ` Eric Dumazet
2016-09-27 1:38 ` Eric Dumazet
2016-09-27 1:38 ` Eric Dumazet
2016-09-27 8:13 ` Vlastimil Babka
2016-09-27 8:13 ` Vlastimil Babka
2016-09-27 13:34 ` Eric Dumazet
2016-09-27 13:34 ` Eric Dumazet
2016-09-27 13:34 ` Eric Dumazet
2016-09-27 8:06 ` Vlastimil Babka
2016-09-27 8:06 ` Vlastimil Babka
2016-09-27 8:45 ` [PATCH v3] " Vlastimil Babka
2016-09-27 8:45 ` Vlastimil Babka
2016-09-27 10:22 ` Michal Hocko
2016-09-27 10:22 ` Michal Hocko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a212f313-1f34-7c83-3aab-b45374875493@suse.cz \
--to=vbabka@suse.cz \
--cc=akpm@linux-foundation.org \
--cc=eric.dumazet@gmail.com \
--cc=linux-api@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-man@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.