From patchwork Wed Jan 26 20:43:05 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Trond Myklebust X-Patchwork-Id: 509821 X-Patchwork-Delegate: Trond.Myklebust@netapp.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.3) with ESMTP id p0QKhBNI017687 for ; Wed, 26 Jan 2011 20:43:12 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753564Ab1AZUnI (ORCPT ); Wed, 26 Jan 2011 15:43:08 -0500 Received: from mx2.netapp.com ([216.240.18.37]:57907 "EHLO mx2.netapp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753465Ab1AZUnH convert rfc822-to-8bit (ORCPT ); Wed, 26 Jan 2011 15:43:07 -0500 X-IronPort-AV: E=Sophos;i="4.60,381,1291622400"; d="scan'208";a="510405273" Received: from smtp2.corp.netapp.com ([10.57.159.114]) by mx2-out.netapp.com with ESMTP; 26 Jan 2011 12:43:07 -0800 Received: from sacrsexc1-prd.hq.netapp.com (sacrsexc1-prd.hq.netapp.com [10.99.115.27]) by smtp2.corp.netapp.com (8.13.1/8.13.1/NTAP-1.6) with ESMTP id p0QKh7FU006633; Wed, 26 Jan 2011 12:43:07 -0800 (PST) Received: from SACMVEXC2-PRD.hq.netapp.com ([10.99.115.18]) by sacrsexc1-prd.hq.netapp.com with Microsoft SMTPSVC(6.0.3790.3959); Wed, 26 Jan 2011 12:43:07 -0800 Received: from 10.58.52.54 ([10.58.52.54]) by SACMVEXC2-PRD.hq.netapp.com ([10.99.115.16]) with Microsoft Exchange Server HTTP-DAV ; Wed, 26 Jan 2011 20:43:06 +0000 Received: from heimdal.trondhjem.org by SACMVEXC2-PRD.hq.netapp.com; 26 Jan 2011 15:43:05 -0500 Subject: Re: 2.6.38-rc2... NFS sillyrename is broken... From: Trond Myklebust To: "J. R. Okajima" Cc: linux-nfs@vger.kernel.org, Nick Piggin , Linux Filesystem Development In-Reply-To: <1296072867.7127.26.camel@heimdal.trondhjem.org> References: <1295998215.6867.22.camel@heimdal.trondhjem.org> <21515.1296030022@jrobl> <1296072867.7127.26.camel@heimdal.trondhjem.org> Organization: NetApp Inc Date: Wed, 26 Jan 2011 15:43:05 -0500 Message-ID: <1296074585.7127.33.camel@heimdal.trondhjem.org> Mime-Version: 1.0 X-Mailer: Evolution 2.32.1 (2.32.1-1.fc14) X-OriginalArrivalTime: 26 Jan 2011 20:43:07.0039 (UTC) FILETIME=[A2DF72F0:01CBBD99] Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Wed, 26 Jan 2011 20:43:12 +0000 (UTC) diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking index 4471a41..5362714 100644 --- a/Documentation/filesystems/Locking +++ b/Documentation/filesystems/Locking @@ -21,6 +21,7 @@ prototypes: char *(*d_dname)((struct dentry *dentry, char *buffer, int buflen); struct vfsmount *(*d_automount)(struct path *path); int (*d_manage)(struct dentry *, bool); + void (*d_unlink)(struct dentry *, struct dentry *); locking rules: rename_lock ->d_lock may block rcu-walk @@ -33,6 +34,7 @@ d_iput: no no yes no d_dname: no no no no d_automount: no no yes no d_manage: no no yes (ref-walk) maybe +d_unlink no no yes no --------------------------- inode_operations --------------------------- prototypes: diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt index 94cf97b..a69d978 100644 --- a/Documentation/filesystems/vfs.txt +++ b/Documentation/filesystems/vfs.txt @@ -866,6 +866,7 @@ struct dentry_operations { char *(*d_dname)(struct dentry *, char *, int); struct vfsmount *(*d_automount)(struct path *); int (*d_manage)(struct dentry *, bool, bool); + void (*d_unlink)(struct dentry *, struct dentry *); }; d_revalidate: called when the VFS needs to revalidate a dentry. This @@ -973,6 +974,14 @@ struct dentry_operations { This function is only used if DCACHE_MANAGE_TRANSIT is set on the dentry being transited from. + d_unlink: called to allow the filesystem to unlink the dentry after final + use. It is only called when DCACHE_NFSFS_RENAMED is set, and is + designed for use by 'sillyrename' schemes that are commonly + implemented on distributed filesystems such as NFS. + + Note that the filesystem is still responsible for protecting against + races with other lookups. + Example : static char *pipefs_dname(struct dentry *dent, char *buffer, int buflen) diff --git a/fs/dcache.c b/fs/dcache.c index 2a6bd9a..f4dea50 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -301,6 +301,9 @@ static struct dentry *d_kill(struct dentry *dentry, struct dentry *parent) if (parent) spin_unlock(&parent->d_lock); dentry_iput(dentry); + + if (dentry->d_flags & DCACHE_NFSFS_RENAMED) + dentry->d_op->d_unlink(parent, dentry); /* * dentry_iput drops the locks, at which point nobody (except * transient RCU lookups) can reach this dentry. diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index 2c3eb33..322c882 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -1161,19 +1161,22 @@ static void nfs_dentry_iput(struct dentry *dentry, struct inode *inode) if (S_ISDIR(inode->i_mode)) /* drop any readdir cache as it could easily be old */ NFS_I(inode)->cache_validity |= NFS_INO_INVALID_DATA; - - if (dentry->d_flags & DCACHE_NFSFS_RENAMED) { + if (dentry->d_flags & DCACHE_NFSFS_RENAMED) drop_nlink(inode); - nfs_complete_unlink(dentry, inode); - } iput(inode); } +static void nfs_d_unlink(struct dentry *parent, struct dentry *dentry) +{ + nfs_complete_unlink(parent, dentry); +} + const struct dentry_operations nfs_dentry_operations = { .d_revalidate = nfs_lookup_revalidate, .d_delete = nfs_dentry_delete, .d_iput = nfs_dentry_iput, .d_automount = nfs_d_automount, + .d_unlink = nfs_d_unlink, }; static struct dentry *nfs_lookup(struct inode *dir, struct dentry * dentry, struct nameidata *nd) @@ -1248,6 +1251,7 @@ const struct dentry_operations nfs4_dentry_operations = { .d_delete = nfs_dentry_delete, .d_iput = nfs_dentry_iput, .d_automount = nfs_d_automount, + .d_unlink = nfs_d_unlink, }; /* diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c index e313a51..9160cad 100644 --- a/fs/nfs/unlink.c +++ b/fs/nfs/unlink.c @@ -184,19 +184,17 @@ static int nfs_do_call_unlink(struct dentry *parent, struct inode *dir, struct n return 1; } -static int nfs_call_unlink(struct dentry *dentry, struct nfs_unlinkdata *data) +static int nfs_call_unlink(struct dentry *parent, struct dentry *dentry, struct nfs_unlinkdata *data) { - struct dentry *parent; struct inode *dir; int ret = 0; - parent = dget_parent(dentry); if (parent == NULL) - goto out_free; + goto out; dir = parent->d_inode; if (nfs_copy_dname(dentry, data) != 0) - goto out_dput; + goto out; /* Non-exclusive lock protects against concurrent lookup() calls */ spin_lock(&dir->i_lock); if (atomic_inc_not_zero(&NFS_I(dir)->silly_count) == 0) { @@ -204,13 +202,11 @@ static int nfs_call_unlink(struct dentry *dentry, struct nfs_unlinkdata *data) hlist_add_head(&data->list, &NFS_I(dir)->silly_list); spin_unlock(&dir->i_lock); ret = 1; - goto out_dput; + goto out; } spin_unlock(&dir->i_lock); ret = nfs_do_call_unlink(parent, dir, data); -out_dput: - dput(parent); -out_free: +out: return ret; } @@ -283,26 +279,24 @@ out: /** * nfs_complete_unlink - Initialize completion of the sillydelete + * @parent: parent directory * @dentry: dentry to delete - * @inode: inode * * Since we're most likely to be called by dentry_iput(), we * only use the dentry to find the sillydelete. We then copy the name * into the qstr. */ void -nfs_complete_unlink(struct dentry *dentry, struct inode *inode) +nfs_complete_unlink(struct dentry *parent, struct dentry *dentry) { struct nfs_unlinkdata *data = NULL; - spin_lock(&dentry->d_lock); if (dentry->d_flags & DCACHE_NFSFS_RENAMED) { dentry->d_flags &= ~DCACHE_NFSFS_RENAMED; data = dentry->d_fsdata; } - spin_unlock(&dentry->d_lock); - if (data != NULL && (NFS_STALE(inode) || !nfs_call_unlink(dentry, data))) + if (data != NULL && !nfs_call_unlink(parent, dentry, data)) nfs_free_unlinkdata(data); } diff --git a/include/linux/dcache.h b/include/linux/dcache.h index f958c19..a3fd768 100644 --- a/include/linux/dcache.h +++ b/include/linux/dcache.h @@ -169,6 +169,7 @@ struct dentry_operations { char *(*d_dname)(struct dentry *, char *, int); struct vfsmount *(*d_automount)(struct path *); int (*d_manage)(struct dentry *, bool, bool); + void (*d_unlink)(struct dentry *, struct dentry *); } ____cacheline_aligned; /* diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h index 6023efa..11d4b40 100644 --- a/include/linux/nfs_fs.h +++ b/include/linux/nfs_fs.h @@ -488,7 +488,7 @@ extern void nfs_release_automount_timer(void); /* * linux/fs/nfs/unlink.c */ -extern void nfs_complete_unlink(struct dentry *dentry, struct inode *); +extern void nfs_complete_unlink(struct dentry *dentry, struct 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);