All of lore.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Cong Wang <cwang@twopensource.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	 "linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	 linux-fsdevel@vger.kernel.org,  netdev <netdev@vger.kernel.org>,
	 linux-mm@kvack.org
Subject: Re: [PATCH] fdtable: Avoid triggering OOMs from alloc_fdmem
Date: Tue, 04 Feb 2014 10:53:35 -0800	[thread overview]
Message-ID: <878utqlnf4.fsf@xmission.com> (raw)
In-Reply-To: <CAHA+R7OLnrujsinNhwVvZyJDz+BrTxYmw0gWeSSyq+dJ2LF9qg@mail.gmail.com> (Cong Wang's message of "Tue, 4 Feb 2014 10:25:33 -0800")

Cong Wang <cwang@twopensource.com> writes:

> On Mon, Feb 3, 2014 at 9:26 PM, Eric W. Biederman <ebiederm@xmission.com> wrote:
>> diff --git a/fs/file.c b/fs/file.c
>> index 771578b33fb6..db25c2bdfe46 100644
>> --- a/fs/file.c
>> +++ b/fs/file.c
>> @@ -34,7 +34,7 @@ static void *alloc_fdmem(size_t size)
>>          * vmalloc() if the allocation size will be considered "large" by the VM.
>>          */
>>         if (size <= (PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER)) {
>> -               void *data = kmalloc(size, GFP_KERNEL|__GFP_NOWARN);
>> +               void *data = kmalloc(size, GFP_KERNEL|__GFP_NOWARN|__GFP_NORETRY);
>>                 if (data != NULL)
>>                         return data;
>>         }
>
> Or try again without __GFP_NORETRY like we do in nelink mmap?

I think I would much rather keep the current semantics of return -ENOMEM
and keep the problem localized then trigger a box wide OOM thank you
very much.


Retrying the kmalloc without __GFP_NORETRY is pointless.  If you are in
the unlikely 0.01% of the time when the kmalloc fails it is almost
certainly going to fail again.  Writing out_of_memory() as kmalloc()
is pointless and very confusing.

The vmalloc won't fail unless you are on a 32bit box.  So it isn't a
case that anyone has to deal with in practice.

Eric

--
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>

WARNING: multiple messages have this Message-ID (diff)
From: ebiederm@xmission.com (Eric W. Biederman)
To: Cong Wang <cwang@twopensource.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	linux-fsdevel@vger.kernel.org, netdev <netdev@vger.kernel.org>,
	linux-mm@kvack.org
Subject: Re: [PATCH] fdtable: Avoid triggering OOMs from alloc_fdmem
Date: Tue, 04 Feb 2014 10:53:35 -0800	[thread overview]
Message-ID: <878utqlnf4.fsf@xmission.com> (raw)
In-Reply-To: <CAHA+R7OLnrujsinNhwVvZyJDz+BrTxYmw0gWeSSyq+dJ2LF9qg@mail.gmail.com> (Cong Wang's message of "Tue, 4 Feb 2014 10:25:33 -0800")

Cong Wang <cwang@twopensource.com> writes:

> On Mon, Feb 3, 2014 at 9:26 PM, Eric W. Biederman <ebiederm@xmission.com> wrote:
>> diff --git a/fs/file.c b/fs/file.c
>> index 771578b33fb6..db25c2bdfe46 100644
>> --- a/fs/file.c
>> +++ b/fs/file.c
>> @@ -34,7 +34,7 @@ static void *alloc_fdmem(size_t size)
>>          * vmalloc() if the allocation size will be considered "large" by the VM.
>>          */
>>         if (size <= (PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER)) {
>> -               void *data = kmalloc(size, GFP_KERNEL|__GFP_NOWARN);
>> +               void *data = kmalloc(size, GFP_KERNEL|__GFP_NOWARN|__GFP_NORETRY);
>>                 if (data != NULL)
>>                         return data;
>>         }
>
> Or try again without __GFP_NORETRY like we do in nelink mmap?

I think I would much rather keep the current semantics of return -ENOMEM
and keep the problem localized then trigger a box wide OOM thank you
very much.


Retrying the kmalloc without __GFP_NORETRY is pointless.  If you are in
the unlikely 0.01% of the time when the kmalloc fails it is almost
certainly going to fail again.  Writing out_of_memory() as kmalloc()
is pointless and very confusing.

The vmalloc won't fail unless you are on a 32bit box.  So it isn't a
case that anyone has to deal with in practice.

Eric

--
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>

WARNING: multiple messages have this Message-ID (diff)
From: ebiederm@xmission.com (Eric W. Biederman)
To: Cong Wang <cwang@twopensource.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	"linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	linux-fsdevel@vger.kernel.org, netdev <netdev@vger.kernel.org>,
	linux-mm@kvack.org
Subject: Re: [PATCH] fdtable: Avoid triggering OOMs from alloc_fdmem
Date: Tue, 04 Feb 2014 10:53:35 -0800	[thread overview]
Message-ID: <878utqlnf4.fsf@xmission.com> (raw)
In-Reply-To: <CAHA+R7OLnrujsinNhwVvZyJDz+BrTxYmw0gWeSSyq+dJ2LF9qg@mail.gmail.com> (Cong Wang's message of "Tue, 4 Feb 2014 10:25:33 -0800")

Cong Wang <cwang@twopensource.com> writes:

> On Mon, Feb 3, 2014 at 9:26 PM, Eric W. Biederman <ebiederm@xmission.com> wrote:
>> diff --git a/fs/file.c b/fs/file.c
>> index 771578b33fb6..db25c2bdfe46 100644
>> --- a/fs/file.c
>> +++ b/fs/file.c
>> @@ -34,7 +34,7 @@ static void *alloc_fdmem(size_t size)
>>          * vmalloc() if the allocation size will be considered "large" by the VM.
>>          */
>>         if (size <= (PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER)) {
>> -               void *data = kmalloc(size, GFP_KERNEL|__GFP_NOWARN);
>> +               void *data = kmalloc(size, GFP_KERNEL|__GFP_NOWARN|__GFP_NORETRY);
>>                 if (data != NULL)
>>                         return data;
>>         }
>
> Or try again without __GFP_NORETRY like we do in nelink mmap?

I think I would much rather keep the current semantics of return -ENOMEM
and keep the problem localized then trigger a box wide OOM thank you
very much.


Retrying the kmalloc without __GFP_NORETRY is pointless.  If you are in
the unlikely 0.01% of the time when the kmalloc fails it is almost
certainly going to fail again.  Writing out_of_memory() as kmalloc()
is pointless and very confusing.

The vmalloc won't fail unless you are on a 32bit box.  So it isn't a
case that anyone has to deal with in practice.

Eric

  reply	other threads:[~2014-02-04 18:53 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-04  5:26 [PATCH] fdtable: Avoid triggering OOMs from alloc_fdmem Eric W. Biederman
2014-02-04  5:26 ` Eric W. Biederman
2014-02-04  5:26 ` Eric W. Biederman
2014-02-04 16:18 ` Eric Dumazet
2014-02-04 16:18   ` Eric Dumazet
2014-02-04 17:22   ` Eric W. Biederman
2014-02-04 17:22     ` Eric W. Biederman
2014-02-04 18:44     ` Eric Dumazet
2014-02-04 18:44       ` Eric Dumazet
2014-02-04 18:57       ` Eric W. Biederman
2014-02-04 18:57         ` Eric W. Biederman
2014-02-04 22:48         ` Eric Dumazet
2014-02-04 22:48           ` Eric Dumazet
2014-02-04 21:27     ` David Rientjes
2014-02-04 21:27       ` David Rientjes
2014-02-04 18:25 ` Cong Wang
2014-02-04 18:25   ` Cong Wang
2014-02-04 18:53   ` Eric W. Biederman [this message]
2014-02-04 18:53     ` Eric W. Biederman
2014-02-04 18:53     ` Eric W. Biederman

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=878utqlnf4.fsf@xmission.com \
    --to=ebiederm@xmission.com \
    --cc=akpm@linux-foundation.org \
    --cc=cwang@twopensource.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=netdev@vger.kernel.org \
    /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.