* [PATCH v2] NFS: Ensure that rmdir() waits for sillyrenames to complete
@ 2013-09-02 18:17 Trond Myklebust
2013-09-03 9:33 ` William Dauchy
0 siblings, 1 reply; 6+ messages in thread
From: Trond Myklebust @ 2013-09-02 18:17 UTC (permalink / raw)
To: linux-nfs; +Cc: andros
If an NFS client does
mkdir("dir");
fd = open("dir/file");
unlink("dir/file");
close(fd);
rmdir("dir");
then the asynchronous nature of the sillyrename operation means that
we can end up getting EBUSY for the rmdir() in the above test. Fix
that by ensuring that we wait for any in-progress sillyrenames
before sending the rmdir() to the server.
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
fs/nfs/dir.c | 19 +++++++++++++------
fs/nfs/unlink.c | 7 +++++++
include/linux/nfs_fs.h | 1 +
3 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index d8149e9..187caa4 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1694,12 +1694,19 @@ int nfs_rmdir(struct inode *dir, struct dentry *dentry)
dir->i_sb->s_id, dir->i_ino, dentry->d_name.name);
trace_nfs_rmdir_enter(dir, dentry);
- error = NFS_PROTO(dir)->rmdir(dir, &dentry->d_name);
- /* Ensure the VFS deletes this inode */
- if (error == 0 && dentry->d_inode != NULL)
- clear_nlink(dentry->d_inode);
- else if (error == -ENOENT)
- nfs_dentry_handle_enoent(dentry);
+ if (dentry->d_inode) {
+ nfs_wait_on_sillyrename(dentry);
+ error = NFS_PROTO(dir)->rmdir(dir, &dentry->d_name);
+ /* Ensure the VFS deletes this inode */
+ switch (error) {
+ case 0:
+ clear_nlink(dentry->d_inode);
+ break;
+ case -ENOENT:
+ nfs_dentry_handle_enoent(dentry);
+ }
+ } else
+ error = NFS_PROTO(dir)->rmdir(dir, &dentry->d_name);
trace_nfs_rmdir_exit(dir, dentry, error);
return error;
diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c
index 2c1485d..0c6dfe0 100644
--- a/fs/nfs/unlink.c
+++ b/fs/nfs/unlink.c
@@ -207,6 +207,13 @@ out_free:
return ret;
}
+void nfs_wait_on_sillyrename(struct dentry *dentry)
+{
+ struct nfs_inode *nfsi = NFS_I(dentry->d_inode);
+
+ wait_event(nfsi->waitqueue, atomic_read(&nfsi->silly_count) == 1);
+}
+
void nfs_block_sillyrename(struct dentry *dentry)
{
struct nfs_inode *nfsi = NFS_I(dentry->d_inode);
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 7125cef..3ea4cde 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -524,6 +524,7 @@ static inline void nfs4_label_free(void *label) {}
* linux/fs/nfs/unlink.c
*/
extern void nfs_complete_unlink(struct dentry *dentry, struct inode *);
+extern void nfs_wait_on_sillyrename(struct dentry *dentry);
extern void nfs_block_sillyrename(struct dentry *dentry);
extern void nfs_unblock_sillyrename(struct dentry *dentry);
extern int nfs_sillyrename(struct inode *dir, struct dentry *dentry);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] NFS: Ensure that rmdir() waits for sillyrenames to complete
2013-09-02 18:17 [PATCH v2] NFS: Ensure that rmdir() waits for sillyrenames to complete Trond Myklebust
@ 2013-09-03 9:33 ` William Dauchy
2013-09-03 18:50 ` Myklebust, Trond
0 siblings, 1 reply; 6+ messages in thread
From: William Dauchy @ 2013-09-03 9:33 UTC (permalink / raw)
To: Trond Myklebust; +Cc: Linux NFS mailing list, andros, William Dauchy
On Mon, Sep 2, 2013 at 8:17 PM, Trond Myklebust
<Trond.Myklebust@netapp.com> wrote:
> If an NFS client does
>
> mkdir("dir");
> fd = open("dir/file");
> unlink("dir/file");
> close(fd);
> rmdir("dir");
>
> then the asynchronous nature of the sillyrename operation means that
> we can end up getting EBUSY for the rmdir() in the above test. Fix
> that by ensuring that we wait for any in-progress sillyrenames
> before sending the rmdir() to the server.
I tested the patch on top of a 3.10.x
When doing heavy operations like rm -rf dir/ with lots of data, the
process gets stuck for ever.
removing the patch fixes the issue.
--
William
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] NFS: Ensure that rmdir() waits for sillyrenames to complete
2013-09-03 9:33 ` William Dauchy
@ 2013-09-03 18:50 ` Myklebust, Trond
2013-09-03 20:44 ` William Dauchy
0 siblings, 1 reply; 6+ messages in thread
From: Myklebust, Trond @ 2013-09-03 18:50 UTC (permalink / raw)
To: William Dauchy; +Cc: Linux NFS mailing list, Adamson, Andy, William Dauchy
[-- Attachment #1: Type: text/plain, Size: 1016 bytes --]
On Tue, 2013-09-03 at 11:33 +0200, William Dauchy wrote:
> On Mon, Sep 2, 2013 at 8:17 PM, Trond Myklebust
> <Trond.Myklebust@netapp.com> wrote:
> > If an NFS client does
> >
> > mkdir("dir");
> > fd = open("dir/file");
> > unlink("dir/file");
> > close(fd);
> > rmdir("dir");
> >
> > then the asynchronous nature of the sillyrename operation means that
> > we can end up getting EBUSY for the rmdir() in the above test. Fix
> > that by ensuring that we wait for any in-progress sillyrenames
> > before sending the rmdir() to the server.
>
> I tested the patch on top of a 3.10.x
> When doing heavy operations like rm -rf dir/ with lots of data, the
> process gets stuck for ever.
> removing the patch fixes the issue.
>
Hi William,
Thanks again for testing! Does the following fixup on top of the 'v2'
patch also help?
Cheers
Trond
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust@netapp.com
www.netapp.com
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-fixup-NFS-Ensure-that-rmdir-waits-for-sillyrenames-t.patch --]
[-- Type: text/x-patch; name="0001-fixup-NFS-Ensure-that-rmdir-waits-for-sillyrenames-t.patch", Size: 803 bytes --]
From 2875e6f36755db12ed6a5b363f0d14a26fcb495b Mon Sep 17 00:00:00 2001
From: Trond Myklebust <Trond.Myklebust@netapp.com>
Date: Tue, 3 Sep 2013 14:48:21 -0400
Subject: [PATCH] fixup! NFS: Ensure that rmdir() waits for sillyrenames to
complete
---
fs/nfs/unlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c
index 0c6dfe0..bb939ed 100644
--- a/fs/nfs/unlink.c
+++ b/fs/nfs/unlink.c
@@ -211,7 +211,7 @@ void nfs_wait_on_sillyrename(struct dentry *dentry)
{
struct nfs_inode *nfsi = NFS_I(dentry->d_inode);
- wait_event(nfsi->waitqueue, atomic_read(&nfsi->silly_count) == 1);
+ wait_event(nfsi->waitqueue, atomic_read(&nfsi->silly_count) <= 1);
}
void nfs_block_sillyrename(struct dentry *dentry)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] NFS: Ensure that rmdir() waits for sillyrenames to complete
2013-09-03 18:50 ` Myklebust, Trond
@ 2013-09-03 20:44 ` William Dauchy
2013-09-03 20:47 ` Myklebust, Trond
0 siblings, 1 reply; 6+ messages in thread
From: William Dauchy @ 2013-09-03 20:44 UTC (permalink / raw)
To: Myklebust, Trond
Cc: William Dauchy, Linux NFS mailing list, Adamson, Andy,
William Dauchy
[-- Attachment #1: Type: text/plain, Size: 686 bytes --]
On Sep03 18:50, Myklebust, Trond wrote:
> Thanks again for testing! Does the following fixup on top of the 'v2'
> patch also help?
> --- a/fs/nfs/unlink.c
> +++ b/fs/nfs/unlink.c
> @@ -211,7 +211,7 @@ void nfs_wait_on_sillyrename(struct dentry *dentry)
> {
> struct nfs_inode *nfsi = NFS_I(dentry->d_inode);
>
> - wait_event(nfsi->waitqueue, atomic_read(&nfsi->silly_count) == 1);
> + wait_event(nfsi->waitqueue, atomic_read(&nfsi->silly_count) <= 1);
> }
>
> void nfs_block_sillyrename(struct dentry *dentry)
hmmm I just realized your v2 version had `== 1`; in fact I tested your v1
with `== 0`. Let me some time to retest the v2.
Thanks,
--
William
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] NFS: Ensure that rmdir() waits for sillyrenames to complete
2013-09-03 20:44 ` William Dauchy
@ 2013-09-03 20:47 ` Myklebust, Trond
2013-09-04 9:19 ` William Dauchy
0 siblings, 1 reply; 6+ messages in thread
From: Myklebust, Trond @ 2013-09-03 20:47 UTC (permalink / raw)
To: William Dauchy; +Cc: William Dauchy, Linux NFS mailing list, Adamson, Andy
T24gVHVlLCAyMDEzLTA5LTAzIGF0IDIyOjQ0ICswMjAwLCBXaWxsaWFtIERhdWNoeSB3cm90ZToN
Cj4gT24gU2VwMDMgMTg6NTAsIE15a2xlYnVzdCwgVHJvbmQgd3JvdGU6DQo+ID4gVGhhbmtzIGFn
YWluIGZvciB0ZXN0aW5nISBEb2VzIHRoZSBmb2xsb3dpbmcgZml4dXAgb24gdG9wIG9mIHRoZSAn
djInDQo+ID4gcGF0Y2ggYWxzbyBoZWxwPw0KPiA+IC0tLSBhL2ZzL25mcy91bmxpbmsuYw0KPiA+
ICsrKyBiL2ZzL25mcy91bmxpbmsuYw0KPiA+IEBAIC0yMTEsNyArMjExLDcgQEAgdm9pZCBuZnNf
d2FpdF9vbl9zaWxseXJlbmFtZShzdHJ1Y3QgZGVudHJ5ICpkZW50cnkpDQo+ID4gIHsNCj4gPiAg
CXN0cnVjdCBuZnNfaW5vZGUgKm5mc2kgPSBORlNfSShkZW50cnktPmRfaW5vZGUpOw0KPiA+ICAN
Cj4gPiAtCXdhaXRfZXZlbnQobmZzaS0+d2FpdHF1ZXVlLCBhdG9taWNfcmVhZCgmbmZzaS0+c2ls
bHlfY291bnQpID09IDEpOw0KPiA+ICsJd2FpdF9ldmVudChuZnNpLT53YWl0cXVldWUsIGF0b21p
Y19yZWFkKCZuZnNpLT5zaWxseV9jb3VudCkgPD0gMSk7DQo+ID4gIH0NCj4gPiAgDQo+ID4gIHZv
aWQgbmZzX2Jsb2NrX3NpbGx5cmVuYW1lKHN0cnVjdCBkZW50cnkgKmRlbnRyeSkNCj4gDQo+IGht
bW0gSSBqdXN0IHJlYWxpemVkIHlvdXIgdjIgdmVyc2lvbiBoYWQgYD09IDFgOyBpbiBmYWN0IEkg
dGVzdGVkIHlvdXIgdjENCj4gd2l0aCBgPT0gMGAuIExldCBtZSBzb21lIHRpbWUgdG8gcmV0ZXN0
IHRoZSB2Mi4NCg0KT0suIFRoYW5rcyBhZ2FpbiENCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxp
bnV4IE5GUyBjbGllbnQgbWFpbnRhaW5lcg0KDQpOZXRBcHANClRyb25kLk15a2xlYnVzdEBuZXRh
cHAuY29tDQp3d3cubmV0YXBwLmNvbQ0K
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] NFS: Ensure that rmdir() waits for sillyrenames to complete
2013-09-03 20:47 ` Myklebust, Trond
@ 2013-09-04 9:19 ` William Dauchy
0 siblings, 0 replies; 6+ messages in thread
From: William Dauchy @ 2013-09-04 9:19 UTC (permalink / raw)
To: Myklebust, Trond
Cc: William Dauchy, William Dauchy, Linux NFS mailing list,
Adamson, Andy
[-- Attachment #1: Type: text/plain, Size: 1062 bytes --]
On Sep03 20:47, Myklebust, Trond wrote:
> On Tue, 2013-09-03 at 22:44 +0200, William Dauchy wrote:
> > On Sep03 18:50, Myklebust, Trond wrote:
> > > Thanks again for testing! Does the following fixup on top of the 'v2'
> > > patch also help?
> > > --- a/fs/nfs/unlink.c
> > > +++ b/fs/nfs/unlink.c
> > > @@ -211,7 +211,7 @@ void nfs_wait_on_sillyrename(struct dentry *dentry)
> > > {
> > > struct nfs_inode *nfsi = NFS_I(dentry->d_inode);
> > >
> > > - wait_event(nfsi->waitqueue, atomic_read(&nfsi->silly_count) == 1);
> > > + wait_event(nfsi->waitqueue, atomic_read(&nfsi->silly_count) <= 1);
> > > }
> > >
> > > void nfs_block_sillyrename(struct dentry *dentry)
> >
> > hmmm I just realized your v2 version had `== 1`; in fact I tested your v1
> > with `== 0`. Let me some time to retest the v2.
After some more testing the v2 was ok with `== 1`; I tested the v1 too
fast without seeing a v2 has been resent. I guess you can revert `<= 1`
Tested-by: William Dauchy <william@gandi.net>
Sorry for the noise,
--
William
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-09-04 9:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-02 18:17 [PATCH v2] NFS: Ensure that rmdir() waits for sillyrenames to complete Trond Myklebust
2013-09-03 9:33 ` William Dauchy
2013-09-03 18:50 ` Myklebust, Trond
2013-09-03 20:44 ` William Dauchy
2013-09-03 20:47 ` Myklebust, Trond
2013-09-04 9:19 ` William Dauchy
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.