All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/15] VFS: File pinning: pre-script-run fixups
@ 2015-03-25 14:43 David Howells
  2015-03-25 14:43 ` [PATCH 01/15] configfs: Fix inconsistent use of file_inode() vs file->f_path.dentry->d_inode David Howells
                   ` (14 more replies)
  0 siblings, 15 replies; 19+ messages in thread
From: David Howells @ 2015-03-25 14:43 UTC (permalink / raw)
  To: viro; +Cc: dhowells, linux-unionfs, linux-kernel, miklos


Hi Al,

Could you have a look over these patches please?  They can be divided into a
number of subsets:

 (1) A fix for configfs to be consistent about the use of file_inode() vs
     dentry->d_inode within a function.

 (2) Fix various accesses to dentry->d_inode to use d_inode(dentry) where there
     are brackets and things that cause the RE to not match in the scripted
     mass changes.

 (3) Similar to (2), but instances of dentry->d_inode should be changed to
     d_backing_inode(dentry) instead where the code is dealing with someone
     else's dentries and inodes.

 (4) Fix up the chelsio driver to use file_inode() rather than its own wrapper.

 (5) Use d_is_dir() instead of S_ISDIR() where we can.

 (6) Fix up NFS to not use d_inode as a variable name.


 (7) Supply d_really_is_positive/negative() to ignore the dentry type field
     (unlike d_is_positive/negative()) for use in filesystems and pretty much
     anywhere you'd use d_inode() rather than d_backing_inode().

Then there's the last three patches which form a subset.

 (8) Impose a partial ordering on reads and writes of ->d_inode and the type
     field in ->d_flags.  Always set the inode pointer *before* the type and
     always read the type *before* the inode pointer.  This should allow us to
     collapse:

	if (!dentry->d_inode || d_is_negative(dentry)) {
    
     down to:
    
    	if (d_is_negative(dentry)) {

     during RCU pathwalk.  I think.

 (9) Make pathwalk use d_is_reg() rather than S_ISREG() so that we don't need
     the inode pointer there.

Ideally, I'd like to kill struct nameidata::inode, but I think we can only do
that if we do proper unconditional COW and discard on directory dentries when
killing them rather than recycling them.  I think the only actual RCU-mode user
of nd->inode remaining is inode_permission().


The patches can also be found here:

	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=file-pin-devel

The scripted changed to d_inode() can be found here:

	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=file-pin-fs-experimental

And the scripted changed to d_backing_inode() can be found here:

	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=file-pin-nonfs-experimental

David
---
David Howells (15):
      configfs: Fix inconsistent use of file_inode() vs file->f_path.dentry->d_inode
      VFS: Fix up missed bits of apparmor to use d_inode()
      VFS: Fix up audit to use d_backing_inode()
      VFS: Fix up missed bits of lustre to use d_inode()
      VFS: Fix up missed bits of ecryptfs to use d_inode()
      VFS: Fix up missed bits of reiserfs to use d_inode()
      VFS: AF_UNIX sockets should call mknod on the top layer only
      VFS: Cachefiles should perform fs modifications on the top layer only
      VFS: Fix up some ->d_inode accesses in the chelsio driver
      VFS: Fix up debugfs to use d_is_dir() in place of S_ISDIR()
      NFS: Don't use d_inode as a variable name
      VFS: Add owner-filesystem positive/negative dentry checks
      VFS: Impose ordering on accesses of d_inode and d_flags
      VFS: Combine inode checks with d_is_negative() and d_is_positive() in pathwalk
      VFS: Make pathwalk use d_is_reg() rather than S_ISREG()


 drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c |   21 ++-----
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.h |    2 -
 drivers/staging/lustre/lustre/llite/namei.c        |    2 -
 drivers/staging/lustre/lustre/llite/statahead.c    |    8 +--
 fs/cachefiles/interface.c                          |    4 +
 fs/cachefiles/namei.c                              |   52 +++++++++---------
 fs/configfs/dir.c                                  |    2 -
 fs/dcache.c                                        |   47 +++++++++++++---
 fs/debugfs/inode.c                                 |    2 -
 fs/ecryptfs/inode.c                                |    4 +
 fs/namei.c                                         |    8 +--
 fs/nfs/read.c                                      |    8 +--
 fs/reiserfs/xattr.h                                |    2 -
 include/linux/dcache.h                             |   59 ++++++++++++++------
 kernel/audit_watch.c                               |    2 -
 net/unix/af_unix.c                                 |    2 -
 security/apparmor/apparmorfs.c                     |    2 -
 17 files changed, 136 insertions(+), 91 deletions(-)


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 01/15] configfs: Fix inconsistent use of file_inode() vs file->f_path.dentry->d_inode
  2015-03-25 14:43 [PATCH 00/15] VFS: File pinning: pre-script-run fixups David Howells
@ 2015-03-25 14:43 ` David Howells
  2015-03-25 14:43 ` [PATCH 02/15] VFS: Fix up missed bits of apparmor to use d_inode() David Howells
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: David Howells @ 2015-03-25 14:43 UTC (permalink / raw)
  To: viro; +Cc: dhowells, linux-unionfs, linux-kernel, miklos

Fix inconsistent use of file_inode() vs file->f_path.dentry->d_inode.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/configfs/dir.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index cf0db005d2f5..acb3d63bc9dc 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -1598,7 +1598,7 @@ static loff_t configfs_dir_lseek(struct file *file, loff_t offset, int whence)
 			if (offset >= 0)
 				break;
 		default:
-			mutex_unlock(&file_inode(file)->i_mutex);
+			mutex_unlock(&dentry->d_inode->i_mutex);
 			return -EINVAL;
 	}
 	if (offset != file->f_pos) {


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 02/15] VFS: Fix up missed bits of apparmor to use d_inode()
  2015-03-25 14:43 [PATCH 00/15] VFS: File pinning: pre-script-run fixups David Howells
  2015-03-25 14:43 ` [PATCH 01/15] configfs: Fix inconsistent use of file_inode() vs file->f_path.dentry->d_inode David Howells
@ 2015-03-25 14:43 ` David Howells
  2015-03-25 14:43 ` [PATCH 03/15] VFS: Fix up audit to use d_backing_inode() David Howells
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: David Howells @ 2015-03-25 14:43 UTC (permalink / raw)
  To: viro; +Cc: dhowells, linux-unionfs, linux-kernel, miklos

Fix up missed bits of apparmor to use d_inode() where the clause doesn't
match the autoconversion script RE because of brackets.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 security/apparmor/apparmorfs.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 7db9954f1af2..ad4fa49ad1db 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -365,7 +365,7 @@ void __aa_fs_profile_rmdir(struct aa_profile *profile)
 		if (!profile->dents[i])
 			continue;
 
-		r = profile->dents[i]->d_inode->i_private;
+		r = d_inode(profile->dents[i])->i_private;
 		securityfs_remove(profile->dents[i]);
 		aa_put_replacedby(r);
 		profile->dents[i] = NULL;

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 03/15] VFS: Fix up audit to use d_backing_inode()
  2015-03-25 14:43 [PATCH 00/15] VFS: File pinning: pre-script-run fixups David Howells
  2015-03-25 14:43 ` [PATCH 01/15] configfs: Fix inconsistent use of file_inode() vs file->f_path.dentry->d_inode David Howells
  2015-03-25 14:43 ` [PATCH 02/15] VFS: Fix up missed bits of apparmor to use d_inode() David Howells
@ 2015-03-25 14:43 ` David Howells
  2015-03-25 14:44 ` [PATCH 04/15] VFS: Fix up missed bits of lustre to use d_inode() David Howells
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: David Howells @ 2015-03-25 14:43 UTC (permalink / raw)
  To: viro; +Cc: dhowells, linux-unionfs, linux-kernel, miklos

Fix up audit to use d_backing_inode() where there's a bit that involves casting
and doesn't match the script RE to automatically change to d_backing_inode().

Signed-off-by: David Howells <dhowells@redhat.com>
---

 kernel/audit_watch.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index ad9c1682f616..89f28f5f7fda 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -482,7 +482,7 @@ static int audit_watch_handle_event(struct fsnotify_group *group,
 
 	switch (data_type) {
 	case (FSNOTIFY_EVENT_PATH):
-		inode = ((struct path *)data)->dentry->d_inode;
+		inode = d_backing_inode(((struct path *)data)->dentry);
 		break;
 	case (FSNOTIFY_EVENT_INODE):
 		inode = (struct inode *)data;


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 04/15] VFS: Fix up missed bits of lustre to use d_inode()
  2015-03-25 14:43 [PATCH 00/15] VFS: File pinning: pre-script-run fixups David Howells
                   ` (2 preceding siblings ...)
  2015-03-25 14:43 ` [PATCH 03/15] VFS: Fix up audit to use d_backing_inode() David Howells
@ 2015-03-25 14:44 ` David Howells
  2015-03-25 14:44 ` [PATCH 05/15] VFS: Fix up missed bits of ecryptfs " David Howells
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: David Howells @ 2015-03-25 14:44 UTC (permalink / raw)
  To: viro; +Cc: dhowells, linux-unionfs, linux-kernel, miklos

Fix up missed bits of lustre to use d_inode() where the clause didn't match
the autoconversion script RE because of brackets.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 drivers/staging/lustre/lustre/llite/namei.c     |    2 +-
 drivers/staging/lustre/lustre/llite/statahead.c |    8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/namei.c b/drivers/staging/lustre/lustre/llite/namei.c
index 890ac190f5fa..a27193b316eb 100644
--- a/drivers/staging/lustre/lustre/llite/namei.c
+++ b/drivers/staging/lustre/lustre/llite/namei.c
@@ -447,7 +447,7 @@ static int ll_lookup_it_finish(struct ptlrpc_request *request,
 		   !it_disposition(it, DISP_OPEN_CREATE)) {
 		/* With DISP_OPEN_CREATE dentry will
 		   instantiated in ll_create_it. */
-		LASSERT((*de)->d_inode == NULL);
+		LASSERT(d_inode(*de) == NULL);
 		d_instantiate(*de, inode);
 	}
 
diff --git a/drivers/staging/lustre/lustre/llite/statahead.c b/drivers/staging/lustre/lustre/llite/statahead.c
index 6ad9dd0fe2b3..b27dc991aa3a 100644
--- a/drivers/staging/lustre/lustre/llite/statahead.c
+++ b/drivers/staging/lustre/lustre/llite/statahead.c
@@ -1595,7 +1595,7 @@ int do_statahead_enter(struct inode *dir, struct dentry **dentryp,
 			rc = md_revalidate_lock(ll_i2mdexp(dir), &it,
 						ll_inode2fid(inode), &bits);
 			if (rc == 1) {
-				if ((*dentryp)->d_inode == NULL) {
+				if (d_inode(*dentryp) == NULL) {
 					struct dentry *alias;
 
 					alias = ll_splice_alias(inode,
@@ -1605,13 +1605,13 @@ int do_statahead_enter(struct inode *dir, struct dentry **dentryp,
 						return PTR_ERR(alias);
 					}
 					*dentryp = alias;
-				} else if ((*dentryp)->d_inode != inode) {
+				} else if (d_inode(*dentryp) != inode) {
 					/* revalidate, but inode is recreated */
 					CDEBUG(D_READA,
 					      "stale dentry %pd inode %lu/%u, statahead inode %lu/%u\n",
 					      *dentryp,
-					      (*dentryp)->d_inode->i_ino,
-					      (*dentryp)->d_inode->i_generation,
+					      d_inode(*dentryp)->i_ino,
+					      d_inode(*dentryp)->i_generation,
 					      inode->i_ino,
 					      inode->i_generation);
 					ll_sai_unplug(sai, entry);

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 05/15] VFS: Fix up missed bits of ecryptfs to use d_inode()
  2015-03-25 14:43 [PATCH 00/15] VFS: File pinning: pre-script-run fixups David Howells
                   ` (3 preceding siblings ...)
  2015-03-25 14:44 ` [PATCH 04/15] VFS: Fix up missed bits of lustre to use d_inode() David Howells
@ 2015-03-25 14:44 ` David Howells
  2015-03-25 14:44 ` [PATCH 06/15] VFS: Fix up missed bits of reiserfs " David Howells
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: David Howells @ 2015-03-25 14:44 UTC (permalink / raw)
  To: viro; +Cc: dhowells, linux-unionfs, linux-kernel, miklos

Fix up missed bits of ecryptfs to use d_inode() where the clause didn't match
the autoconversion script RE because of brackets.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/ecryptfs/inode.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index b08b5187f662..99e3b589edf8 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -681,8 +681,8 @@ static void *ecryptfs_follow_link(struct dentry *dentry, struct nameidata *nd)
 	char *buf = ecryptfs_readlink_lower(dentry, &len);
 	if (IS_ERR(buf))
 		goto out;
-	fsstack_copy_attr_atime(dentry->d_inode,
-				ecryptfs_dentry_to_lower(dentry)->d_inode);
+	fsstack_copy_attr_atime(d_inode(dentry),
+				d_inode(ecryptfs_dentry_to_lower(dentry)));
 	buf[len] = '\0';
 out:
 	nd_set_link(nd, buf);

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 06/15] VFS: Fix up missed bits of reiserfs to use d_inode()
  2015-03-25 14:43 [PATCH 00/15] VFS: File pinning: pre-script-run fixups David Howells
                   ` (4 preceding siblings ...)
  2015-03-25 14:44 ` [PATCH 05/15] VFS: Fix up missed bits of ecryptfs " David Howells
@ 2015-03-25 14:44 ` David Howells
  2015-03-25 14:44 ` [PATCH 07/15] VFS: AF_UNIX sockets should call mknod on the top layer only David Howells
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: David Howells @ 2015-03-25 14:44 UTC (permalink / raw)
  To: viro; +Cc: dhowells, linux-unionfs, linux-kernel, miklos

Fix up missed bits of reiserfs to use d_inode() where the clause didn't match
the autoconversion script RE because of brackets.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/reiserfs/xattr.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/reiserfs/xattr.h b/fs/reiserfs/xattr.h
index f620e9678dd5..15dde6262c00 100644
--- a/fs/reiserfs/xattr.h
+++ b/fs/reiserfs/xattr.h
@@ -78,7 +78,7 @@ static inline size_t reiserfs_xattr_jcreate_nblocks(struct inode *inode)
 
 	if ((REISERFS_I(inode)->i_flags & i_has_xattr_dir) == 0) {
 		nblocks += JOURNAL_BLOCKS_PER_OBJECT(inode->i_sb);
-		if (!REISERFS_SB(inode->i_sb)->xattr_root->d_inode)
+		if (d_really_is_negative(REISERFS_SB(inode->i_sb)->xattr_root))
 			nblocks += JOURNAL_BLOCKS_PER_OBJECT(inode->i_sb);
 	}
 

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 07/15] VFS: AF_UNIX sockets should call mknod on the top layer only
  2015-03-25 14:43 [PATCH 00/15] VFS: File pinning: pre-script-run fixups David Howells
                   ` (5 preceding siblings ...)
  2015-03-25 14:44 ` [PATCH 06/15] VFS: Fix up missed bits of reiserfs " David Howells
@ 2015-03-25 14:44 ` David Howells
  2015-03-25 14:44 ` [PATCH 08/15] VFS: Cachefiles should perform fs modifications " David Howells
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: David Howells @ 2015-03-25 14:44 UTC (permalink / raw)
  To: viro; +Cc: dhowells, linux-unionfs, linux-kernel, miklos

AF_UNIX sockets should call mknod on the top layer only and should not attempt
to modify the lower layer in a layered filesystem such as overlayfs.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/unix/af_unix.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 526b6edab018..bd3a1cfd4b73 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -844,7 +844,7 @@ static int unix_mknod(const char *sun_path, umode_t mode, struct path *res)
 	 */
 	err = security_path_mknod(&path, dentry, mode, 0);
 	if (!err) {
-		err = vfs_mknod(path.dentry->d_inode, dentry, mode, 0);
+		err = vfs_mknod(d_inode(path.dentry), dentry, mode, 0);
 		if (!err) {
 			res->mnt = mntget(path.mnt);
 			res->dentry = dget(dentry);


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 08/15] VFS: Cachefiles should perform fs modifications on the top layer only
  2015-03-25 14:43 [PATCH 00/15] VFS: File pinning: pre-script-run fixups David Howells
                   ` (6 preceding siblings ...)
  2015-03-25 14:44 ` [PATCH 07/15] VFS: AF_UNIX sockets should call mknod on the top layer only David Howells
@ 2015-03-25 14:44 ` David Howells
  2015-03-25 14:44 ` [PATCH 09/15] VFS: Fix up some ->d_inode accesses in the chelsio driver David Howells
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: David Howells @ 2015-03-25 14:44 UTC (permalink / raw)
  To: viro; +Cc: dhowells, linux-unionfs, linux-kernel, miklos

Cachefiles should perform fs modifications (eg. vfs_unlink()) on the top layer
only and should not attempt to alter the lower layer.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/cachefiles/interface.c |    4 ++-
 fs/cachefiles/namei.c     |   52 +++++++++++++++++++++++----------------------
 2 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/fs/cachefiles/interface.c b/fs/cachefiles/interface.c
index 232426214fdd..f1fb0a21bb5a 100644
--- a/fs/cachefiles/interface.c
+++ b/fs/cachefiles/interface.c
@@ -446,7 +446,7 @@ static int cachefiles_attr_changed(struct fscache_object *_object)
 		return 0;
 
 	cachefiles_begin_secure(cache, &saved_cred);
-	mutex_lock(&object->backer->d_inode->i_mutex);
+	mutex_lock(&d_inode(object->backer)->i_mutex);
 
 	/* if there's an extension to a partial page at the end of the backing
 	 * file, we need to discard the partial page so that we pick up new
@@ -465,7 +465,7 @@ static int cachefiles_attr_changed(struct fscache_object *_object)
 	ret = notify_change(object->backer, &newattrs, NULL);
 
 truncate_failed:
-	mutex_unlock(&object->backer->d_inode->i_mutex);
+	mutex_unlock(&d_inode(object->backer)->i_mutex);
 	cachefiles_end_secure(cache, saved_cred);
 
 	if (ret == -EIO) {
diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
index 1e51714eb33e..61396359863f 100644
--- a/fs/cachefiles/namei.c
+++ b/fs/cachefiles/namei.c
@@ -286,13 +286,13 @@ static int cachefiles_bury_object(struct cachefiles_cache *cache,
 		if (ret < 0) {
 			cachefiles_io_error(cache, "Unlink security error");
 		} else {
-			ret = vfs_unlink(dir->d_inode, rep, NULL);
+			ret = vfs_unlink(d_inode(dir), rep, NULL);
 
 			if (preemptive)
 				cachefiles_mark_object_buried(cache, rep);
 		}
 
-		mutex_unlock(&dir->d_inode->i_mutex);
+		mutex_unlock(&d_inode(dir)->i_mutex);
 
 		if (ret == -EIO)
 			cachefiles_io_error(cache, "Unlink failed");
@@ -303,7 +303,7 @@ static int cachefiles_bury_object(struct cachefiles_cache *cache,
 
 	/* directories have to be moved to the graveyard */
 	_debug("move stale object to graveyard");
-	mutex_unlock(&dir->d_inode->i_mutex);
+	mutex_unlock(&d_inode(dir)->i_mutex);
 
 try_again:
 	/* first step is to make up a grave dentry in the graveyard */
@@ -387,8 +387,8 @@ try_again:
 	if (ret < 0) {
 		cachefiles_io_error(cache, "Rename security error %d", ret);
 	} else {
-		ret = vfs_rename(dir->d_inode, rep,
-				 cache->graveyard->d_inode, grave, NULL, 0);
+		ret = vfs_rename(d_inode(dir), rep,
+				 d_inode(cache->graveyard), grave, NULL, 0);
 		if (ret != 0 && ret != -ENOMEM)
 			cachefiles_io_error(cache,
 					    "Rename failed with error %d", ret);
@@ -420,13 +420,13 @@ int cachefiles_delete_object(struct cachefiles_cache *cache,
 
 	dir = dget_parent(object->dentry);
 
-	mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_PARENT);
+	mutex_lock_nested(&d_inode(dir)->i_mutex, I_MUTEX_PARENT);
 
 	if (test_bit(CACHEFILES_OBJECT_BURIED, &object->flags)) {
 		/* object allocation for the same key preemptively deleted this
 		 * object's file so that it could create its own file */
 		_debug("object preemptively buried");
-		mutex_unlock(&dir->d_inode->i_mutex);
+		mutex_unlock(&d_inode(dir)->i_mutex);
 		ret = 0;
 	} else {
 		/* we need to check that our parent is _still_ our parent - it
@@ -438,7 +438,7 @@ int cachefiles_delete_object(struct cachefiles_cache *cache,
 			/* it got moved, presumably by cachefilesd culling it,
 			 * so it's no longer in the key path and we can ignore
 			 * it */
-			mutex_unlock(&dir->d_inode->i_mutex);
+			mutex_unlock(&d_inode(dir)->i_mutex);
 			ret = 0;
 		}
 	}
@@ -497,7 +497,7 @@ lookup_again:
 	/* search the current directory for the element name */
 	_debug("lookup '%s'", name);
 
-	mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_PARENT);
+	mutex_lock_nested(&d_inode(dir)->i_mutex, I_MUTEX_PARENT);
 
 	start = jiffies;
 	next = lookup_one_len(name, dir, nlen);
@@ -529,7 +529,7 @@ lookup_again:
 			if (ret < 0)
 				goto create_error;
 			start = jiffies;
-			ret = vfs_mkdir(dir->d_inode, next, 0);
+			ret = vfs_mkdir(d_inode(dir), next, 0);
 			cachefiles_hist(cachefiles_mkdir_histogram, start);
 			if (ret < 0)
 				goto create_error;
@@ -558,7 +558,7 @@ lookup_again:
 			if (ret < 0)
 				goto create_error;
 			start = jiffies;
-			ret = vfs_create(dir->d_inode, next, S_IFREG, true);
+			ret = vfs_create(d_inode(dir), next, S_IFREG, true);
 			cachefiles_hist(cachefiles_create_histogram, start);
 			if (ret < 0)
 				goto create_error;
@@ -581,7 +581,7 @@ lookup_again:
 	/* process the next component */
 	if (key) {
 		_debug("advance");
-		mutex_unlock(&dir->d_inode->i_mutex);
+		mutex_unlock(&d_inode(dir)->i_mutex);
 		dput(dir);
 		dir = next;
 		next = NULL;
@@ -617,7 +617,7 @@ lookup_again:
 	/* note that we're now using this object */
 	ret = cachefiles_mark_object_active(cache, object);
 
-	mutex_unlock(&dir->d_inode->i_mutex);
+	mutex_unlock(&d_inode(dir)->i_mutex);
 	dput(dir);
 	dir = NULL;
 
@@ -695,7 +695,7 @@ lookup_error:
 		cachefiles_io_error(cache, "Lookup failed");
 	next = NULL;
 error:
-	mutex_unlock(&dir->d_inode->i_mutex);
+	mutex_unlock(&d_inode(dir)->i_mutex);
 	dput(next);
 error_out2:
 	dput(dir);
@@ -719,7 +719,7 @@ struct dentry *cachefiles_get_directory(struct cachefiles_cache *cache,
 	_enter(",,%s", dirname);
 
 	/* search the current directory for the element name */
-	mutex_lock(&dir->d_inode->i_mutex);
+	mutex_lock(&d_inode(dir)->i_mutex);
 
 	start = jiffies;
 	subdir = lookup_one_len(dirname, dir, strlen(dirname));
@@ -746,7 +746,7 @@ struct dentry *cachefiles_get_directory(struct cachefiles_cache *cache,
 		ret = security_path_mkdir(&path, subdir, 0700);
 		if (ret < 0)
 			goto mkdir_error;
-		ret = vfs_mkdir(dir->d_inode, subdir, 0700);
+		ret = vfs_mkdir(d_inode(dir), subdir, 0700);
 		if (ret < 0)
 			goto mkdir_error;
 
@@ -758,7 +758,7 @@ struct dentry *cachefiles_get_directory(struct cachefiles_cache *cache,
 		       subdir->d_inode->i_ino);
 	}
 
-	mutex_unlock(&dir->d_inode->i_mutex);
+	mutex_unlock(&d_inode(dir)->i_mutex);
 
 	/* we need to make sure the subdir is a directory */
 	ASSERT(subdir->d_inode);
@@ -790,19 +790,19 @@ check_error:
 	return ERR_PTR(ret);
 
 mkdir_error:
-	mutex_unlock(&dir->d_inode->i_mutex);
+	mutex_unlock(&d_inode(dir)->i_mutex);
 	dput(subdir);
 	pr_err("mkdir %s failed with error %d\n", dirname, ret);
 	return ERR_PTR(ret);
 
 lookup_error:
-	mutex_unlock(&dir->d_inode->i_mutex);
+	mutex_unlock(&d_inode(dir)->i_mutex);
 	ret = PTR_ERR(subdir);
 	pr_err("Lookup %s failed with error %d\n", dirname, ret);
 	return ERR_PTR(ret);
 
 nomem_d_alloc:
-	mutex_unlock(&dir->d_inode->i_mutex);
+	mutex_unlock(&d_inode(dir)->i_mutex);
 	_leave(" = -ENOMEM");
 	return ERR_PTR(-ENOMEM);
 }
@@ -827,7 +827,7 @@ static struct dentry *cachefiles_check_active(struct cachefiles_cache *cache,
 	//       dir, filename);
 
 	/* look up the victim */
-	mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_PARENT);
+	mutex_lock_nested(&d_inode(dir)->i_mutex, I_MUTEX_PARENT);
 
 	start = jiffies;
 	victim = lookup_one_len(filename, dir, strlen(filename));
@@ -842,7 +842,7 @@ static struct dentry *cachefiles_check_active(struct cachefiles_cache *cache,
 	 * at the netfs's request whilst the cull was in progress
 	 */
 	if (!victim->d_inode) {
-		mutex_unlock(&dir->d_inode->i_mutex);
+		mutex_unlock(&d_inode(dir)->i_mutex);
 		dput(victim);
 		_leave(" = -ENOENT [absent]");
 		return ERR_PTR(-ENOENT);
@@ -871,13 +871,13 @@ static struct dentry *cachefiles_check_active(struct cachefiles_cache *cache,
 
 object_in_use:
 	read_unlock(&cache->active_lock);
-	mutex_unlock(&dir->d_inode->i_mutex);
+	mutex_unlock(&d_inode(dir)->i_mutex);
 	dput(victim);
 	//_leave(" = -EBUSY [in use]");
 	return ERR_PTR(-EBUSY);
 
 lookup_error:
-	mutex_unlock(&dir->d_inode->i_mutex);
+	mutex_unlock(&d_inode(dir)->i_mutex);
 	ret = PTR_ERR(victim);
 	if (ret == -ENOENT) {
 		/* file or dir now absent - probably retired by netfs */
@@ -936,7 +936,7 @@ int cachefiles_cull(struct cachefiles_cache *cache, struct dentry *dir,
 	return 0;
 
 error_unlock:
-	mutex_unlock(&dir->d_inode->i_mutex);
+	mutex_unlock(&d_inode(dir)->i_mutex);
 error:
 	dput(victim);
 	if (ret == -ENOENT) {
@@ -971,7 +971,7 @@ int cachefiles_check_in_use(struct cachefiles_cache *cache, struct dentry *dir,
 	if (IS_ERR(victim))
 		return PTR_ERR(victim);
 
-	mutex_unlock(&dir->d_inode->i_mutex);
+	mutex_unlock(&d_inode(dir)->i_mutex);
 	dput(victim);
 	//_leave(" = 0");
 	return 0;

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 09/15] VFS: Fix up some ->d_inode accesses in the chelsio driver
  2015-03-25 14:43 [PATCH 00/15] VFS: File pinning: pre-script-run fixups David Howells
                   ` (7 preceding siblings ...)
  2015-03-25 14:44 ` [PATCH 08/15] VFS: Cachefiles should perform fs modifications " David Howells
@ 2015-03-25 14:44 ` David Howells
  2015-03-25 14:45 ` [PATCH 10/15] VFS: Fix up debugfs to use d_is_dir() in place of S_ISDIR() David Howells
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: David Howells @ 2015-03-25 14:44 UTC (permalink / raw)
  To: viro; +Cc: dhowells, linux-unionfs, linux-kernel, miklos

Fix up some ->d_inode accesses in the chelsio driver.

 (1) FILE_DATA() should just be replaced with file_inode().

 (2) set_debugfs_file_size() should be removed and debugfs_create_file_size()
     should be used to create the file.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c |   21 +++++++-------------
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.h |    2 --
 2 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
index 78854ceb0870..d3476b72f3ae 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
@@ -538,7 +538,7 @@ static ssize_t tp_la_write(struct file *file, const char __user *buf,
 	char s[32];
 	unsigned long val;
 	size_t size = min(sizeof(s) - 1, count);
-	struct adapter *adap = FILE_DATA(file)->i_private;
+	struct adapter *adap = file_inode(file)->i_private;
 
 	if (copy_from_user(s, buf, size))
 		return -EFAULT;
@@ -647,7 +647,7 @@ static int pm_stats_open(struct inode *inode, struct file *file)
 static ssize_t pm_stats_clear(struct file *file, const char __user *buf,
 			      size_t count, loff_t *pos)
 {
-	struct adapter *adap = FILE_DATA(file)->i_private;
+	struct adapter *adap = file_inode(file)->i_private;
 
 	t4_write_reg(adap, PM_RX_STAT_CONFIG_A, 0);
 	t4_write_reg(adap, PM_TX_STAT_CONFIG_A, 0);
@@ -999,7 +999,7 @@ static ssize_t mbox_write(struct file *file, const char __user *buf,
 		   &data[7], &c) < 8 || c != '\n')
 		return -EINVAL;
 
-	ino = FILE_DATA(file);
+	ino = file_inode(file);
 	mbox = (uintptr_t)ino->i_private & 7;
 	adap = ino->i_private - mbox;
 	addr = adap->regs + PF_REG(mbox, CIM_PF_MAILBOX_DATA_A);
@@ -1028,7 +1028,7 @@ static ssize_t flash_read(struct file *file, char __user *buf, size_t count,
 			  loff_t *ppos)
 {
 	loff_t pos = *ppos;
-	loff_t avail = FILE_DATA(file)->i_size;
+	loff_t avail = file_inode(file)->i_size;
 	struct adapter *adap = file->private_data;
 
 	if (pos < 0)
@@ -1473,7 +1473,7 @@ static ssize_t rss_key_write(struct file *file, const char __user *buf,
 	int i, j;
 	u32 key[10];
 	char s[100], *p;
-	struct adapter *adap = FILE_DATA(file)->i_private;
+	struct adapter *adap = file_inode(file)->i_private;
 
 	if (count > sizeof(s) - 1)
 		return -EINVAL;
@@ -1941,12 +1941,6 @@ static const struct file_operations mem_debugfs_fops = {
 	.llseek  = default_llseek,
 };
 
-static void set_debugfs_file_size(struct dentry *de, loff_t size)
-{
-	if (!IS_ERR(de) && de->d_inode)
-		de->d_inode->i_size = size;
-}
-
 static void add_debugfs_mem(struct adapter *adap, const char *name,
 			    unsigned int idx, unsigned int size_mb)
 {
@@ -2062,9 +2056,8 @@ int t4_setup_debugfs(struct adapter *adap)
 		}
 	}
 
-	de = debugfs_create_file("flash", S_IRUSR, adap->debugfs_root, adap,
-				 &flash_debugfs_fops);
-	set_debugfs_file_size(de, adap->params.sf_size);
+	de = debugfs_create_file_size("flash", S_IRUSR, adap->debugfs_root, adap,
+				      &flash_debugfs_fops, adap->params.sf_size);
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.h
index 8f418ba868bd..23f43a0f8950 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.h
@@ -37,8 +37,6 @@
 
 #include <linux/export.h>
 
-#define FILE_DATA(_file) ((_file)->f_path.dentry->d_inode)
-
 #define DEFINE_SIMPLE_DEBUGFS_FILE(name) \
 static int name##_open(struct inode *inode, struct file *file) \
 { \


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 10/15] VFS: Fix up debugfs to use d_is_dir() in place of S_ISDIR()
  2015-03-25 14:43 [PATCH 00/15] VFS: File pinning: pre-script-run fixups David Howells
                   ` (8 preceding siblings ...)
  2015-03-25 14:44 ` [PATCH 09/15] VFS: Fix up some ->d_inode accesses in the chelsio driver David Howells
@ 2015-03-25 14:45 ` David Howells
  2015-03-25 14:45 ` [PATCH 11/15] NFS: Don't use d_inode as a variable name David Howells
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: David Howells @ 2015-03-25 14:45 UTC (permalink / raw)
  To: viro; +Cc: dhowells, linux-unionfs, linux-kernel, miklos

Fix up debugfs to use d_is_dir(dentry) in place of
S_ISDIR(dentry->d_inode->i_mode).

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/debugfs/inode.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 96400ab42d13..26856ecdea5e 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -521,7 +521,7 @@ static int __debugfs_remove(struct dentry *dentry, struct dentry *parent)
 
 	if (debugfs_positive(dentry)) {
 		dget(dentry);
-		if (S_ISDIR(dentry->d_inode->i_mode))
+		if (d_is_dir(dentry))
 			ret = simple_rmdir(parent->d_inode, dentry);
 		else
 			simple_unlink(parent->d_inode, dentry);

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 11/15] NFS: Don't use d_inode as a variable name
  2015-03-25 14:43 [PATCH 00/15] VFS: File pinning: pre-script-run fixups David Howells
                   ` (9 preceding siblings ...)
  2015-03-25 14:45 ` [PATCH 10/15] VFS: Fix up debugfs to use d_is_dir() in place of S_ISDIR() David Howells
@ 2015-03-25 14:45 ` David Howells
  2015-03-25 14:45 ` [PATCH 12/15] VFS: Add owner-filesystem positive/negative dentry checks David Howells
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: David Howells @ 2015-03-25 14:45 UTC (permalink / raw)
  To: viro; +Cc: dhowells, linux-unionfs, linux-kernel, miklos

Don't use d_inode as a variable name as it now masks a function name.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/nfs/read.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index 568ecf0a880f..b8f5c63f77b2 100644
--- a/fs/nfs/read.c
+++ b/fs/nfs/read.c
@@ -117,15 +117,15 @@ int nfs_readpage_async(struct nfs_open_context *ctx, struct inode *inode,
 
 static void nfs_readpage_release(struct nfs_page *req)
 {
-	struct inode *d_inode = req->wb_context->dentry->d_inode;
+	struct inode *inode = req->wb_context->dentry->d_inode;
 
-	dprintk("NFS: read done (%s/%llu %d@%lld)\n", d_inode->i_sb->s_id,
-		(unsigned long long)NFS_FILEID(d_inode), req->wb_bytes,
+	dprintk("NFS: read done (%s/%llu %d@%lld)\n", inode->i_sb->s_id,
+		(unsigned long long)NFS_FILEID(inode), req->wb_bytes,
 		(long long)req_offset(req));
 
 	if (nfs_page_group_sync_on_bit(req, PG_UNLOCKPAGE)) {
 		if (PageUptodate(req->wb_page))
-			nfs_readpage_to_fscache(d_inode, req->wb_page, 0);
+			nfs_readpage_to_fscache(inode, req->wb_page, 0);
 
 		unlock_page(req->wb_page);
 	}


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 12/15] VFS: Add owner-filesystem positive/negative dentry checks
  2015-03-25 14:43 [PATCH 00/15] VFS: File pinning: pre-script-run fixups David Howells
                   ` (10 preceding siblings ...)
  2015-03-25 14:45 ` [PATCH 11/15] NFS: Don't use d_inode as a variable name David Howells
@ 2015-03-25 14:45 ` David Howells
  2015-03-26 13:05   ` Miklos Szeredi
  2015-03-27 14:42   ` David Howells
  2015-03-25 14:45 ` [PATCH 13/15] VFS: Impose ordering on accesses of d_inode and d_flags David Howells
                   ` (2 subsequent siblings)
  14 siblings, 2 replies; 19+ messages in thread
From: David Howells @ 2015-03-25 14:45 UTC (permalink / raw)
  To: viro; +Cc: dhowells, linux-unionfs, linux-kernel, miklos

Supply two functions to test whether a filesystem's own dentries are positive
or negative (d_really_is_positive() and d_really_is_negative()).

The problem is that the DCACHE_ENTRY_TYPE field of dentry->d_flags may be
overridden by the union part of a layered filesystem and isn't thus
necessarily indicative of the type of dentry.

Normally, this would involve a negative dentry (ie. ->d_inode == NULL) having
->d_layer.lower pointed to a lower layer dentry, DCACHE_PINNING_LOWER set and
the DCACHE_ENTRY_TYPE field set to something other than DCACHE_MISS_TYPE - but
it could also involve, say, a DCACHE_SPECIAL_TYPE being overridden to
DCACHE_WHITEOUT_TYPE if a 0,0 chardev is detected in the top layer.

However, inside a filesystem, when that fs is looking at its own dentries, it
probably wants to know if they are really negative or not - and doesn't care
about the fallthrough bits used by the union.

To this end, a filesystem should normally use d_really_is_positive/negative()
when looking at its own dentries rather than d_is_positive/negative() and
should use d_inode() to get at the inode.

Anyone looking at someone else's dentries (this includes pathwalk) should use
d_is_xxx() and d_backing_inode().

Signed-off-by: David Howells <dhowells@redhat.com>
---

 include/linux/dcache.h |   38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index d8358799c594..e83768ee38fc 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -482,6 +482,44 @@ static inline bool d_is_positive(const struct dentry *dentry)
 	return !d_is_negative(dentry);
 }
 
+/**
+ * d_really_is_negative - Determine if a dentry is really negative (ignoring fallthroughs)
+ * @dentry: The dentry in question
+ *
+ * Returns true if the dentry represents either an absent name or a name that
+ * doesn't map to an inode (ie. ->d_inode is NULL).  The dentry could represent
+ * a true miss, a whiteout that isn't represented by a 0,0 chardev or a
+ * fallthrough marker in an opaque directory.
+ *
+ * Note!  (1) This should be used *only* by a filesystem to examine its own
+ * dentries.  It should not be used to look at some other filesystem's
+ * dentries.  (2) It should also be used in combination with d_inode() to get
+ * the inode.  (3) The dentry may have something attached to ->d_lower and the
+ * type field of the flags may be set to something other than miss or whiteout.
+ */
+static inline bool d_really_is_negative(const struct dentry *dentry)
+{
+	return dentry->d_inode == NULL;
+}
+
+/**
+ * d_really_is_positive - Determine if a dentry is really positive (ignoring fallthroughs)
+ * @dentry: The dentry in question
+ *
+ * Returns true if the dentry represents a name that maps to an inode
+ * (ie. ->d_inode is not NULL).  The dentry might still represent a whiteout if
+ * that is represented on medium as a 0,0 chardev.
+ *
+ * Note!  (1) This should be used *only* by a filesystem to examine its own
+ * dentries.  It should not be used to look at some other filesystem's
+ * dentries.  (2) It should also be used in combination with d_inode() to get
+ * the inode.
+ */
+static inline bool d_really_is_positive(const struct dentry *dentry)
+{
+	return dentry->d_inode != NULL;
+}
+
 extern void d_set_fallthru(struct dentry *dentry);
 
 static inline bool d_is_fallthru(const struct dentry *dentry)

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 13/15] VFS: Impose ordering on accesses of d_inode and d_flags
  2015-03-25 14:43 [PATCH 00/15] VFS: File pinning: pre-script-run fixups David Howells
                   ` (11 preceding siblings ...)
  2015-03-25 14:45 ` [PATCH 12/15] VFS: Add owner-filesystem positive/negative dentry checks David Howells
@ 2015-03-25 14:45 ` David Howells
  2015-03-25 14:45 ` [PATCH 14/15] VFS: Combine inode checks with d_is_negative() and d_is_positive() in pathwalk David Howells
  2015-03-25 14:45 ` [PATCH 15/15] VFS: Make pathwalk use d_is_reg() rather than S_ISREG() David Howells
  14 siblings, 0 replies; 19+ messages in thread
From: David Howells @ 2015-03-25 14:45 UTC (permalink / raw)
  To: viro; +Cc: dhowells, linux-unionfs, linux-kernel, miklos

Impose ordering on accesses of d_inode and d_flags to avoid the need to do
this:

	if (!dentry->d_inode || d_is_negative(dentry)) {

when this:

	if (d_is_negative(dentry)) {

should suffice.

This check is especially problematic if a dentry can have its type field set
to something other than DENTRY_MISS_TYPE when d_inode is NULL (as in
unionmount).

What we really need to do is stick a write barrier between setting d_inode and
setting d_flags and a read barrier between reading d_flags and reading
d_inode.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/dcache.c            |   47 +++++++++++++++++++++++++++++++++++++++--------
 include/linux/dcache.h |   21 +++------------------
 2 files changed, 42 insertions(+), 26 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index c71e3732e53b..b9e8b41459a4 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -269,6 +269,41 @@ static inline int dname_external(const struct dentry *dentry)
 	return dentry->d_name.name != dentry->d_iname;
 }
 
+/*
+ * Make sure other CPUs see the inode attached before the type is set.
+ */
+static inline void __d_set_inode_and_type(struct dentry *dentry,
+					  struct inode *inode,
+					  unsigned type_flags)
+{
+	unsigned flags;
+
+	dentry->d_inode = inode;
+	smp_wmb();
+	flags = READ_ONCE(dentry->d_flags);
+	flags &= ~(DCACHE_ENTRY_TYPE | DCACHE_FALLTHRU);
+	flags |= type_flags;
+	WRITE_ONCE(dentry->d_flags, flags);
+}
+
+/*
+ * Ideally, we want to make sure that other CPUs see the flags cleared before
+ * the inode is detached, but this is really a violation of RCU principles
+ * since the ordering suggests we should always set inode before flags.
+ *
+ * We should instead replace or discard the entire dentry - but that sucks
+ * performancewise on mass deletion/rename.
+ */
+static inline void __d_clear_type_and_inode(struct dentry *dentry)
+{
+	unsigned flags = READ_ONCE(dentry->d_flags);
+
+	flags &= ~(DCACHE_ENTRY_TYPE | DCACHE_FALLTHRU);
+	WRITE_ONCE(dentry->d_flags, flags);
+	smp_wmb();
+	dentry->d_inode = NULL;
+}
+
 static void dentry_free(struct dentry *dentry)
 {
 	WARN_ON(!hlist_unhashed(&dentry->d_u.d_alias));
@@ -311,7 +346,7 @@ static void dentry_iput(struct dentry * dentry)
 {
 	struct inode *inode = dentry->d_inode;
 	if (inode) {
-		dentry->d_inode = NULL;
+		__d_clear_type_and_inode(dentry);
 		hlist_del_init(&dentry->d_u.d_alias);
 		spin_unlock(&dentry->d_lock);
 		spin_unlock(&inode->i_lock);
@@ -335,8 +370,7 @@ static void dentry_unlink_inode(struct dentry * dentry)
 	__releases(dentry->d_inode->i_lock)
 {
 	struct inode *inode = dentry->d_inode;
-	__d_clear_type(dentry);
-	dentry->d_inode = NULL;
+	__d_clear_type_and_inode(dentry);
 	hlist_del_init(&dentry->d_u.d_alias);
 	dentry_rcuwalk_barrier(dentry);
 	spin_unlock(&dentry->d_lock);
@@ -1715,11 +1749,9 @@ static void __d_instantiate(struct dentry *dentry, struct inode *inode)
 	unsigned add_flags = d_flags_for_inode(inode);
 
 	spin_lock(&dentry->d_lock);
-	dentry->d_flags &= ~(DCACHE_ENTRY_TYPE | DCACHE_FALLTHRU);
-	dentry->d_flags |= add_flags;
 	if (inode)
 		hlist_add_head(&dentry->d_u.d_alias, &inode->i_dentry);
-	dentry->d_inode = inode;
+	__d_set_inode_and_type(dentry, inode, add_flags);
 	dentry_rcuwalk_barrier(dentry);
 	spin_unlock(&dentry->d_lock);
 	fsnotify_d_instantiate(dentry, inode);
@@ -1937,8 +1969,7 @@ static struct dentry *__d_obtain_alias(struct inode *inode, int disconnected)
 		add_flags |= DCACHE_DISCONNECTED;
 
 	spin_lock(&tmp->d_lock);
-	tmp->d_inode = inode;
-	tmp->d_flags |= add_flags;
+	__d_set_inode_and_type(tmp, inode, add_flags);
 	hlist_add_head(&tmp->d_u.d_alias, &inode->i_dentry);
 	hlist_bl_lock(&tmp->d_sb->s_anon);
 	hlist_bl_add_head(&tmp->d_hash, &tmp->d_sb->s_anon);
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index e83768ee38fc..df334cbacc6d 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -404,26 +404,11 @@ static inline bool d_mountpoint(const struct dentry *dentry)
 /*
  * Directory cache entry type accessor functions.
  */
-static inline void __d_set_type(struct dentry *dentry, unsigned type)
-{
-	dentry->d_flags = (dentry->d_flags & ~DCACHE_ENTRY_TYPE) | type;
-}
-
-static inline void __d_clear_type(struct dentry *dentry)
-{
-	__d_set_type(dentry, DCACHE_MISS_TYPE);
-}
-
-static inline void d_set_type(struct dentry *dentry, unsigned type)
-{
-	spin_lock(&dentry->d_lock);
-	__d_set_type(dentry, type);
-	spin_unlock(&dentry->d_lock);
-}
-
 static inline unsigned __d_entry_type(const struct dentry *dentry)
 {
-	return dentry->d_flags & DCACHE_ENTRY_TYPE;
+	unsigned type = READ_ONCE(dentry->d_flags);
+	smp_rmb();
+	return type & DCACHE_ENTRY_TYPE;
 }
 
 static inline bool d_is_miss(const struct dentry *dentry)

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 14/15] VFS: Combine inode checks with d_is_negative() and d_is_positive() in pathwalk
  2015-03-25 14:43 [PATCH 00/15] VFS: File pinning: pre-script-run fixups David Howells
                   ` (12 preceding siblings ...)
  2015-03-25 14:45 ` [PATCH 13/15] VFS: Impose ordering on accesses of d_inode and d_flags David Howells
@ 2015-03-25 14:45 ` David Howells
  2015-03-25 14:45 ` [PATCH 15/15] VFS: Make pathwalk use d_is_reg() rather than S_ISREG() David Howells
  14 siblings, 0 replies; 19+ messages in thread
From: David Howells @ 2015-03-25 14:45 UTC (permalink / raw)
  To: viro; +Cc: dhowells, linux-unionfs, linux-kernel, miklos

Where we have:

    	if (!dentry->d_inode || d_is_negative(dentry)) {

type constructions in pathwalk we should be able to eliminate the check of
d_inode and rely solely on the result of d_is_negative() or d_is_positive().

What we do have to take care to do is to read d_inode after calling a
d_is_xxx() typecheck function to get the barriering right.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/namei.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index c83145af4bfc..66234b836360 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1586,7 +1586,7 @@ static inline int walk_component(struct nameidata *nd, struct path *path,
 		inode = path->dentry->d_inode;
 	}
 	err = -ENOENT;
-	if (!inode || d_is_negative(path->dentry))
+	if (d_is_negative(path->dentry))
 		goto out_path_put;
 
 	if (should_follow_link(path->dentry, follow)) {
@@ -2313,7 +2313,7 @@ mountpoint_last(struct nameidata *nd, struct path *path)
 	mutex_unlock(&dir->d_inode->i_mutex);
 
 done:
-	if (!dentry->d_inode || d_is_negative(dentry)) {
+	if (d_is_negative(dentry)) {
 		error = -ENOENT;
 		dput(dentry);
 		goto out;
@@ -3040,7 +3040,7 @@ retry_lookup:
 finish_lookup:
 	/* we _can_ be in RCU mode here */
 	error = -ENOENT;
-	if (!inode || d_is_negative(path->dentry)) {
+	if (d_is_negative(path->dentry)) {
 		path_to_nameidata(path, nd);
 		goto out;
 	}

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 15/15] VFS: Make pathwalk use d_is_reg() rather than S_ISREG()
  2015-03-25 14:43 [PATCH 00/15] VFS: File pinning: pre-script-run fixups David Howells
                   ` (13 preceding siblings ...)
  2015-03-25 14:45 ` [PATCH 14/15] VFS: Combine inode checks with d_is_negative() and d_is_positive() in pathwalk David Howells
@ 2015-03-25 14:45 ` David Howells
  14 siblings, 0 replies; 19+ messages in thread
From: David Howells @ 2015-03-25 14:45 UTC (permalink / raw)
  To: viro; +Cc: dhowells, linux-unionfs, linux-kernel, miklos

Make pathwalk use d_is_reg() rather than S_ISREG() to determine whether to
honour O_TRUNC.  Since this occurs after complete_walk(), the dentry type
field cannot change and the inode pointer cannot change as we hold a ref on
the dentry, so this should be safe.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/namei.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/namei.c b/fs/namei.c
index 66234b836360..5a8dee5c333e 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3079,7 +3079,7 @@ finish_open:
 	error = -ENOTDIR;
 	if ((nd->flags & LOOKUP_DIRECTORY) && !d_can_lookup(nd->path.dentry))
 		goto out;
-	if (!S_ISREG(nd->inode->i_mode))
+	if (!d_is_reg(nd->path.dentry))
 		will_truncate = false;
 
 	if (will_truncate) {

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 12/15] VFS: Add owner-filesystem positive/negative dentry checks
  2015-03-25 14:45 ` [PATCH 12/15] VFS: Add owner-filesystem positive/negative dentry checks David Howells
@ 2015-03-26 13:05   ` Miklos Szeredi
  2015-03-27 14:42   ` David Howells
  1 sibling, 0 replies; 19+ messages in thread
From: Miklos Szeredi @ 2015-03-26 13:05 UTC (permalink / raw)
  To: David Howells; +Cc: viro, linux-unionfs, Kernel Mailing List

On Wed, Mar 25, 2015 at 3:45 PM, David Howells <dhowells@redhat.com> wrote:
> Supply two functions to test whether a filesystem's own dentries are positive
> or negative (d_really_is_positive() and d_really_is_negative()).
>
> The problem is that the DCACHE_ENTRY_TYPE field of dentry->d_flags may be
> overridden by the union part of a layered filesystem and isn't thus
> necessarily indicative of the type of dentry.
>
> Normally, this would involve a negative dentry (ie. ->d_inode == NULL) having
> ->d_layer.lower pointed to a lower layer dentry, DCACHE_PINNING_LOWER set and
> the DCACHE_ENTRY_TYPE field set to something other than DCACHE_MISS_TYPE - but
> it could also involve, say, a DCACHE_SPECIAL_TYPE being overridden to
> DCACHE_WHITEOUT_TYPE if a 0,0 chardev is detected in the top layer.
>
> However, inside a filesystem, when that fs is looking at its own dentries, it
> probably wants to know if they are really negative or not - and doesn't care
> about the fallthrough bits used by the union.
>
> To this end, a filesystem should normally use d_really_is_positive/negative()
> when looking at its own dentries rather than d_is_positive/negative() and
> should use d_inode() to get at the inode.
>
> Anyone looking at someone else's dentries (this includes pathwalk) should use
> d_is_xxx() and d_backing_inode().

I think this is confusing as hell, there needs to be more consistency
in the naming.  E.g. d_backing_is_positive() vs. d_is_positive().   I
know it's the other way round now, but only with a few users.  Also a
separate include file might help, that needs explicit including to get
the "backing" variants and which would have big fat warnings all over.

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 12/15] VFS: Add owner-filesystem positive/negative dentry checks
  2015-03-25 14:45 ` [PATCH 12/15] VFS: Add owner-filesystem positive/negative dentry checks David Howells
  2015-03-26 13:05   ` Miklos Szeredi
@ 2015-03-27 14:42   ` David Howells
  2015-03-27 15:27     ` Miklos Szeredi
  1 sibling, 1 reply; 19+ messages in thread
From: David Howells @ 2015-03-27 14:42 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: dhowells, viro, linux-unionfs, Kernel Mailing List

Miklos Szeredi <miklos@szeredi.hu> wrote:

> I think this is confusing as hell, there needs to be more consistency
> in the naming.  E.g. d_backing_is_positive() vs. d_is_positive().   I
> know it's the other way round now, but only with a few users.

Yeah.  The problem is that all of:

	__d_entry_type()
	d_is_miss()
	d_is_whiteout()
	d_can_lookup()
	d_is_autodir()
	d_is_dir()
	d_is_symlink()
	d_is_reg()
	d_is_special()
	d_is_file()
	d_is_negative()
	d_is_positive()

refer to the 'backing' inode (if there is one) in the case that you have a
unionmount and the top dentry's ->d_inode is NULL.  (Well, technically, that
doesn't happen in the case of directories)

Of course, if we decide we aren't going to do unionmount, certain things
become simpler.

> Also a separate include file might help, that needs explicit including to
> get the "backing" variants

I would like to see a 'for fs implementer' header and a 'for fs user' header
but Al didn't like that last time I suggested it.

However, it doesn't help with the naming since there are situations where you
need *both* - eg. overlayfs.

> and which would have big fat warnings all over.

Well, we could argue about which side should have the warnings.

David

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 12/15] VFS: Add owner-filesystem positive/negative dentry checks
  2015-03-27 14:42   ` David Howells
@ 2015-03-27 15:27     ` Miklos Szeredi
  0 siblings, 0 replies; 19+ messages in thread
From: Miklos Szeredi @ 2015-03-27 15:27 UTC (permalink / raw)
  To: David Howells; +Cc: viro, linux-unionfs, Kernel Mailing List

On Fri, Mar 27, 2015 at 3:42 PM, David Howells <dhowells@redhat.com> wrote:
> Miklos Szeredi <miklos@szeredi.hu> wrote:
>
>> I think this is confusing as hell, there needs to be more consistency
>> in the naming.  E.g. d_backing_is_positive() vs. d_is_positive().   I
>> know it's the other way round now, but only with a few users.
>
> Yeah.  The problem is that all of:
>
>         __d_entry_type()
>         d_is_miss()
>         d_is_whiteout()
>         d_can_lookup()
>         d_is_autodir()
>         d_is_dir()
>         d_is_symlink()
>         d_is_reg()
>         d_is_special()
>         d_is_file()
>         d_is_negative()
>         d_is_positive()
>
> refer to the 'backing' inode (if there is one) in the case that you have a
> unionmount and the top dentry's ->d_inode is NULL.  (Well, technically, that
> doesn't happen in the case of directories)

Looks to me we actually need two variants of all of the above, since
most filesystems never want to refer to the backing inode.

>
> Of course, if we decide we aren't going to do unionmount, certain things
> become simpler.
>
>> Also a separate include file might help, that needs explicit including to
>> get the "backing" variants
>
> I would like to see a 'for fs implementer' header and a 'for fs user' header
> but Al didn't like that last time I suggested it.
>
> However, it doesn't help with the naming since there are situations where you
> need *both* - eg. overlayfs.

Not sure what you mean, the naming *must* be different even if we have
two headers and overlayfs can just include both.

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2015-03-27 15:27 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-25 14:43 [PATCH 00/15] VFS: File pinning: pre-script-run fixups David Howells
2015-03-25 14:43 ` [PATCH 01/15] configfs: Fix inconsistent use of file_inode() vs file->f_path.dentry->d_inode David Howells
2015-03-25 14:43 ` [PATCH 02/15] VFS: Fix up missed bits of apparmor to use d_inode() David Howells
2015-03-25 14:43 ` [PATCH 03/15] VFS: Fix up audit to use d_backing_inode() David Howells
2015-03-25 14:44 ` [PATCH 04/15] VFS: Fix up missed bits of lustre to use d_inode() David Howells
2015-03-25 14:44 ` [PATCH 05/15] VFS: Fix up missed bits of ecryptfs " David Howells
2015-03-25 14:44 ` [PATCH 06/15] VFS: Fix up missed bits of reiserfs " David Howells
2015-03-25 14:44 ` [PATCH 07/15] VFS: AF_UNIX sockets should call mknod on the top layer only David Howells
2015-03-25 14:44 ` [PATCH 08/15] VFS: Cachefiles should perform fs modifications " David Howells
2015-03-25 14:44 ` [PATCH 09/15] VFS: Fix up some ->d_inode accesses in the chelsio driver David Howells
2015-03-25 14:45 ` [PATCH 10/15] VFS: Fix up debugfs to use d_is_dir() in place of S_ISDIR() David Howells
2015-03-25 14:45 ` [PATCH 11/15] NFS: Don't use d_inode as a variable name David Howells
2015-03-25 14:45 ` [PATCH 12/15] VFS: Add owner-filesystem positive/negative dentry checks David Howells
2015-03-26 13:05   ` Miklos Szeredi
2015-03-27 14:42   ` David Howells
2015-03-27 15:27     ` Miklos Szeredi
2015-03-25 14:45 ` [PATCH 13/15] VFS: Impose ordering on accesses of d_inode and d_flags David Howells
2015-03-25 14:45 ` [PATCH 14/15] VFS: Combine inode checks with d_is_negative() and d_is_positive() in pathwalk David Howells
2015-03-25 14:45 ` [PATCH 15/15] VFS: Make pathwalk use d_is_reg() rather than S_ISREG() David Howells

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.