From: Linus Torvalds <torvalds@linux-foundation.org>
To: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: Nick Bowler <nbowler@elliptictech.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-nfs@vger.kernel.org
Subject: Re: [PATCH 2/3] NFS: lock the readdir page while it is in use
Date: Tue, 30 Nov 2010 20:10:08 -0800 [thread overview]
Message-ID: <AANLkTin8FSeM8cQ8BWYWjiyGy33EwzpMfFwauK+4j0uT@mail.gmail.com> (raw)
In-Reply-To: <1291175234-23824-3-git-send-email-Trond.Myklebust@netapp.com>
On Tue, Nov 30, 2010 at 7:47 PM, Trond Myklebust
<Trond.Myklebust@netapp.com> wrote:
> Otherwise, the VM may end up removing it while we're reading from it.
I don't think this is valid.
Maybe it fixes a bug, but the commit description is misleading at
best. Since you have a reference count to the page, the page is not
going away. Locking may hide some other bug (due to serializing with
other code you care about), but it is _not_ about the "VM may end up
removing it".
Even from a serialization angle, I think this patch is a bit suspect,
since readdir() will always be called under the inode semaphore, so I
think you'll always be serialized wrt other readdir users. Of course,
you may have invalidation events etc that are outside of readdir, so
...
Anyway if this patch matters, there's something else going on, and you
need to describe that.
Linus
next prev parent reply other threads:[~2010-12-01 4:10 UTC|newest]
Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-11-30 17:42 [PATCH] NFS: Fix a readdirplus bug Trond Myklebust
2010-11-30 22:10 ` Linus Torvalds
2010-11-30 22:13 ` Trond Myklebust
2010-12-01 3:47 ` [PATCH 0/3] Fix more NFS readdir regressions Trond Myklebust
2010-12-01 3:47 ` [PATCH 1/3] NFS: Ensure we use the correct cookie in nfs_readdir_xdr_filler Trond Myklebust
2010-12-01 15:04 ` Nick Bowler
2010-12-01 15:36 ` [PATCH v2 0/3] Fix more NFS readdir regressions Trond Myklebust
2010-12-01 15:36 ` [PATCH v2 1/3] NFS: Ensure we use the correct cookie in nfs_readdir_xdr_filler Trond Myklebust
2010-12-01 15:36 ` [PATCH v2 2/3] NFS: lock the readdir page while it is in use Trond Myklebust
2010-12-01 15:36 ` [PATCH v2 3/3] NFS: Fix a memory leak in nfs_readdir Trond Myklebust
2010-12-01 16:17 ` Linus Torvalds
2010-12-01 16:35 ` Rik van Riel
2010-12-01 16:45 ` Benny Halevy
2010-12-01 16:47 ` Linus Torvalds
2010-12-01 17:02 ` Rik van Riel
2010-12-01 17:58 ` Trond Myklebust
2010-12-01 18:29 ` Miklos Szeredi
2010-12-01 18:54 ` Trond Myklebust
2010-12-01 19:23 ` Hugh Dickins
2010-12-01 19:52 ` Linus Torvalds
2010-12-01 20:05 ` Trond Myklebust
2010-12-01 20:39 ` Andrew Morton
2010-12-01 21:29 ` Neil Brown
2010-12-01 22:43 ` Andrew Morton
2010-12-01 23:01 ` Neil Brown
2010-12-01 19:47 ` Linus Torvalds
2010-12-01 20:10 ` Trond Myklebust
2010-12-01 20:18 ` Linus Torvalds
2010-12-01 20:31 ` Hugh Dickins
2010-12-01 20:33 ` Andrew Morton
2010-12-01 21:02 ` Hugh Dickins
2010-12-01 21:15 ` Hugh Dickins
2010-12-01 21:38 ` Andrew Morton
2010-12-01 21:51 ` Trond Myklebust
2010-12-01 22:13 ` Andrew Morton
2010-12-01 22:24 ` Linus Torvalds
2010-12-01 22:38 ` Andrew Morton
2010-12-01 22:47 ` Trond Myklebust
2010-12-01 23:21 ` Trond Myklebust
2010-12-01 23:46 ` Andrew Morton
2010-12-01 23:56 ` Trond Myklebust
2010-12-01 23:31 ` Linus Torvalds
2010-12-01 23:36 ` Andrew Morton
2010-12-02 1:05 ` Linus Torvalds
2010-12-02 1:22 ` Andrew Morton
2010-12-02 1:42 ` Linus Torvalds
2010-12-02 2:05 ` Andrew Morton
2010-12-02 3:08 ` [PATCH v3 0/3] Fix more NFS readdir regressions Trond Myklebust
2010-12-02 3:08 ` [PATCH v3 1/3] NFS: Ensure we use the correct cookie in nfs_readdir_xdr_filler Trond Myklebust
2010-12-02 3:08 ` [PATCH v3 2/3] Call the filesystem back whenever a page is removed from the page cache Trond Myklebust
2010-12-02 3:34 ` Hugh Dickins
2010-12-02 3:53 ` Trond Myklebust
2010-12-02 3:58 ` Linus Torvalds
2010-12-06 16:59 ` [PATCH v4 0/3] Fix more NFS readdir regressions Trond Myklebust
2010-12-06 16:59 ` [PATCH v4 1/3] NFS: Ensure we use the correct cookie in nfs_readdir_xdr_filler Trond Myklebust
2010-12-06 16:59 ` [PATCH v4 2/3] Call the filesystem back whenever a page is removed from the page cache Trond Myklebust
2010-12-07 7:08 ` Nick Piggin
2010-12-06 16:59 ` [PATCH v4 3/3] NFS: Fix a memory leak in nfs_readdir Trond Myklebust
2010-12-02 3:08 ` [PATCH v3 " Trond Myklebust
2010-12-03 9:12 ` [PATCH v2 " Nick Piggin
2010-12-01 23:43 ` Trond Myklebust
2010-12-01 22:43 ` Hugh Dickins
2010-12-01 3:47 ` [PATCH 2/3] NFS: lock the readdir page while it is in use Trond Myklebust
2010-12-01 4:10 ` Linus Torvalds [this message]
2010-12-01 4:29 ` Trond Myklebust
2010-12-01 5:06 ` Linus Torvalds
2010-12-01 14:49 ` Trond Myklebust
2010-12-01 13:14 ` Rik van Riel
2010-12-01 14:55 ` Trond Myklebust
2010-12-01 3:47 ` [PATCH 3/3] NFS: Fix a memory leak in nfs_readdir Trond Myklebust
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=AANLkTin8FSeM8cQ8BWYWjiyGy33EwzpMfFwauK+4j0uT@mail.gmail.com \
--to=torvalds@linux-foundation.org \
--cc=Trond.Myklebust@netapp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=nbowler@elliptictech.com \
/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).