public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Theodore Ts'o" <tytso@mit.edu>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Anatol Pomozov <anatol.pomozov@gmail.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	Louis Huemiller <lhuemill@google.com>
Subject: Re: [PATCH] fs: Preserve error code in get_empty_filp()
Date: Thu, 21 Feb 2013 11:36:05 -0500	[thread overview]
Message-ID: <20130221163605.GA13379@thunk.org> (raw)
In-Reply-To: <20130215022134.GA11376@ZenIV.linux.org.uk>

> > A little bit of context for this change. We at Google work on a test
> > framework that shows how kernel behaves under memory pressure. In the
> > codepath that I am fixing the syscalls return ENFILE error, but in
> > fact the correct error would be ENOMEM. get_empty_filp() should
> > preserve the original error and not to replace all errors with ENFILE.
> 
> The trouble is, you are introducing previously impossible return values
> for pipe(2).  The rest of it is probably OK (even though I'd prefer to
> split get_empty_filp() part into a separate commit), but this one has
> potential for breaking previously correct userland code.  OTOH, FreeBSD has
> done that a while ago and they apparently had been able to cope with the
> fallout.

Sure, but Posix/SUSv3 has always said that system calls can return
error values that aren't listed in the standard (or the man page).
Given that most applications check for an error, and then use the
errno to log an error which a human can interpret, it would seem to me
to be better to return ENOMEM rather than to return the clearly wrong
ENFILE; after all, we could potentially have only a handful of file
descriptors open at the time when pipe(2) fails due to lack of memory,
and the error code:

       ENFILE The system limit on the total number of open files has
              been reached.

is clearly wrong.

Are you aware of any applications that would blow up if pipe(2)
returned any possible error other than ENFILE?

						- Ted

  reply	other threads:[~2013-02-21 16:36 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-13  3:11 [PATCH] fs: Preserve error code in get_empty_filp() Anatol Pomozov
2012-10-05 18:16 ` Anatol Pomozov
2012-10-05 18:39   ` Theodore Ts'o
2013-02-15  2:21   ` Al Viro
2013-02-21 16:36     ` Theodore Ts'o [this message]
  -- strict thread matches above, loose matches on Subject: below --
2012-08-01 18:19 anatol.pomozov
2012-08-01 18:34 ` anatol.pomozov
2012-08-03  0:30   ` Anatol Pomozov
2012-08-03  0:47 ` Anatol Pomozov
2012-09-04 21:15   ` Anatol Pomozov
2012-08-21 10:06 ` Jan Engelhardt
2012-08-21 13:51   ` Anatol Pomozov

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=20130221163605.GA13379@thunk.org \
    --to=tytso@mit.edu \
    --cc=anatol.pomozov@gmail.com \
    --cc=lhuemill@google.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox