* [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.