From: Dave Hansen <haveblue@us.ibm.com>
To: Erez Zadok <ezk@cs.sunysb.edu>
Cc: bfields@fieldses.org, Andrew Morton <akpm@linux-foundation.org>,
linux-kernel@vger.kernel.org, nfs@lists.sourceforge.net
Subject: Re: [PATCH] NFSD: fix wrong mnt_writer count in rename (MMOTM 2007-11-10-19-05)
Date: Tue, 13 Nov 2007 09:39:41 -0800 [thread overview]
Message-ID: <1194975581.7078.144.camel@localhost> (raw)
In-Reply-To: <200711130115.lAD1FBfG015626@agora.fsl.cs.sunysb.edu>
On Mon, 2007-11-12 at 20:15 -0500, Erez Zadok wrote:
> NFSD forgets to call mnt_drop_write after a successful rename. Here's a
> fix. (Ah, the curse of a stackable file system developer: you have to debug
> everyone else's too. :-)
>
> One thing I wasn't sure is whether I could move the mnt_drop_write line a
> little above, just after the call to vfs_rename. If we can drop the ref
> earlier, it could improve scalability/performance just a bit, no? (i.e.,
> what are the ramifications of holding on to this mnt writer ref longer than
> needed?)
I do see some write-ish things happening inside of the nfsd_sync_dir()
call chain: the fsync() and filemap_fdatawrite().
The danger here is that the moment you mnt_drop_write(), the filesystem
is mounted r/o by someone else, then those fsync() writes occur when
everyone thinks it is r/o. So, I think your fix is the most
conservative, and the most correct.
> Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
>
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 7dfde65..47aec49 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1693,6 +1693,7 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
> if (!host_err)
> host_err = nfsd_sync_dir(fdentry);
> }
> + mnt_drop_write(ffhp->fh_export->ex_path.mnt);
>
> out_dput_new:
> dput(ndentry);
Otherwise, looks good to me. Thanks.
Signed-off-by: Dave Hansen <haveblue@us.ibm.com>
-- Dave
prev parent reply other threads:[~2007-11-13 17:39 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-11-13 1:15 [PATCH] NFSD: fix wrong mnt_writer count in rename (MMOTM 2007-11-10-19-05) Erez Zadok
2007-11-13 1:15 ` Erez Zadok
2007-11-13 17:39 ` Dave Hansen [this message]
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=1194975581.7078.144.camel@localhost \
--to=haveblue@us.ibm.com \
--cc=akpm@linux-foundation.org \
--cc=bfields@fieldses.org \
--cc=ezk@cs.sunysb.edu \
--cc=linux-kernel@vger.kernel.org \
--cc=nfs@lists.sourceforge.net \
/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.