linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "J. Bruce Fields" <bfields@fieldses.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [RFC][PATCH] nfsd regression since delayed fput()
Date: Thu, 17 Oct 2013 21:12:07 +0100	[thread overview]
Message-ID: <20131017201207.GT13318@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CA+55aFzZE2OLbYM865QPKmgkp_Tx=8AEZS-HV21iAsxgETb2Nw@mail.gmail.com>

On Thu, Oct 17, 2013 at 12:09:00PM -0700, Linus Torvalds wrote:
> All it needs is a non-unixy filesystem getting nfs-exported. Like FAT.
> 
> Now, I actually think that FAT is fine with unlinking a file that is
> in use, but my whole argument was "unintended consequences". It is
> *not* obvious that keeping a reference is valid.
>
> Look at rmdir() instead of unlink(), for example. Even POSIX allows
> EBUSY for a directory entry that is still in use, and requires it for
> a non-empty one. So even with a Unix filesystem, I can imagine
> somebody (and by somebody, I mean "knfsd, as response to normal NFS
> client traffic") doing:
> 
>  - read file (delayed fput)
>  - unlink file
>  - rmdir directory that file used to be in and that *should* be empty
>  - delayed fput happens now.
> 
> and even a posix-compliant filesystem could have issues with this
> (again, the *example* for a filesystem with issues would be NFS, and I
> realize you don't re-export it, but the point is, filesystem semantics
> can make this problematic).

Sure, I agree that such a scenario might happen; the question was "which
fs would have to be exported to step on that?" and AFAICS none of the
exportable filesystems have any problems of that sort.
 
> Again, the above is an *example* of subtle issues with delaying the fput.

And we have a tool for dealing with those, if they turn out to be real -
flush_delayed_fput(), doing all pending delayed ones.  I would rather
avoid using it unless we have a case where it's really needed, though;
it can have interesting consequences wrt locking order, etc.

Frankly, in case of knfsd I'm a lot more concerned about the amount of
struct file (all for the same few disk files) sitting around opened and
waiting to be closed, just because there's a client saturating a 10G
link with read requests...

  reply	other threads:[~2013-10-17 20:12 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-16 18:52 [RFC][PATCH] nfsd regression since delayed fput() Al Viro
2013-10-17  4:48 ` Linus Torvalds
2013-10-17 18:14   ` J. Bruce Fields
2013-10-17 18:29     ` Linus Torvalds
2013-10-17 18:39       ` Al Viro
2013-10-17 19:09         ` Linus Torvalds
2013-10-17 20:12           ` Al Viro [this message]
2013-10-17 22:33             ` Linus Torvalds
2013-10-25 14:18               ` Al Viro

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=20131017201207.GT13318@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=bfields@fieldses.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=torvalds@linux-foundation.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).