[PATCH] d_flags races

Alexander Viro (viro@math.psu.edu)
Wed, 18 Apr 2001 06:28:45 -0400 (EDT)


Linus, we need to make access to bits of ->d_flags atomic.
That used to be protected by BKL, but since we've got DCACHE_REFERENCED
it's no longer true - prune_dcache() and d_lookup() are not under BKL
and while they are safe wrt each other (both are under dcache_lock)
they race with every place in filesystems that touches ->d_flags.
Solution: use set_bit/clear_bit/test_bit.
Patch follows. Please, apply.
A:

diff -urN S4-pre4/fs/autofs/root.c S4-pre4-d_flags/fs/autofs/root.c
--- S4-pre4/fs/autofs/root.c Fri Feb 16 17:28:15 2001
+++ S4-pre4-d_flags/fs/autofs/root.c Wed Apr 18 06:17:59 2001
@@ -94,7 +94,7 @@
/* Turn this into a real negative dentry? */
if (status == -ENOENT) {
dentry->d_time = jiffies + AUTOFS_NEGATIVE_TIMEOUT;
- dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
+ clear_bit(D_Autofs_Pending, &dentry->d_flags);
return 1;
} else if (status) {
/* Return a negative dentry, but leave it "pending" */
@@ -129,7 +129,7 @@
autofs_update_usage(&sbi->dirhash,ent);
}

- dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
+ clear_bit(D_Autofs_Pending, &dentry->d_flags);
return 1;
}

@@ -152,7 +152,7 @@
sbi = autofs_sbi(dir->i_sb);

/* Pending dentry */
- if ( dentry->d_flags & DCACHE_AUTOFS_PENDING ) {
+ if (test_bit(D_Autofs_Pending, &dentry->d_flags))
if (autofs_oz_mode(sbi))
res = 1;
else
@@ -219,7 +219,7 @@
* We need to do this before we release the directory semaphore.
*/
dentry->d_op = &autofs_dentry_operations;
- dentry->d_flags |= DCACHE_AUTOFS_PENDING;
+ set_bit(D_Autofs_Pending, &dentry->d_flags);
d_add(dentry, NULL);

up(&dir->i_sem);
@@ -230,7 +230,7 @@
* If we are still pending, check if we had to handle
* a signal. If so we can force a restart..
*/
- if (dentry->d_flags & DCACHE_AUTOFS_PENDING) {
+ if (test_bit(D_Autofs_Pending, &dentry->d_flags)) {
if (signal_pending(current))
return ERR_PTR(-ERESTARTNOINTR);
}
diff -urN S4-pre4/fs/autofs4/autofs_i.h S4-pre4-d_flags/fs/autofs4/autofs_i.h
--- S4-pre4/fs/autofs4/autofs_i.h Fri Feb 16 22:52:15 2001
+++ S4-pre4-d_flags/fs/autofs4/autofs_i.h Wed Apr 18 06:17:59 2001
@@ -119,7 +119,7 @@
{
struct autofs_info *inf = autofs4_dentry_ino(dentry);

- return (dentry->d_flags & DCACHE_AUTOFS_PENDING) ||
+ return (test_bit(D_Autofs_Pending, &dentry->d_flags)) ||
(inf != NULL && inf->flags & AUTOFS_INF_EXPIRING);
}

diff -urN S4-pre4/fs/autofs4/expire.c S4-pre4-d_flags/fs/autofs4/expire.c
--- S4-pre4/fs/autofs4/expire.c Fri Feb 16 19:36:08 2001
+++ S4-pre4-d_flags/fs/autofs4/expire.c Wed Apr 18 06:17:59 2001
@@ -194,7 +194,7 @@
}

/* No point expiring a pending mount */
- if (dentry->d_flags & DCACHE_AUTOFS_PENDING)
+ if (test_bit(D_Autofs_Pending, &dentry->d_flags))
continue;

if (!do_now) {
diff -urN S4-pre4/fs/autofs4/root.c S4-pre4-d_flags/fs/autofs4/root.c
--- S4-pre4/fs/autofs4/root.c Fri Feb 16 19:36:08 2001
+++ S4-pre4-d_flags/fs/autofs4/root.c Wed Apr 18 06:17:59 2001
@@ -82,7 +82,7 @@
if (de_info && (de_info->flags & AUTOFS_INF_EXPIRING)) {
DPRINTK(("try_to_fill_entry: waiting for expire %p name=%.*s, flags&PENDING=%s de_info=%p de_info->flags=%x\n",
dentry, dentry->d_name.len, dentry->d_name.name,
- dentry->d_flags & DCACHE_AUTOFS_PENDING?"t":"f",
+ test_bit(D_Autofs_Pending, &dentry->d_flags)?"t":"f",
de_info, de_info?de_info->flags:0));
status = autofs4_wait(sbi, &dentry->d_name, NFY_NONE);

@@ -109,7 +109,7 @@
/* Turn this into a real negative dentry? */
if (status == -ENOENT) {
dentry->d_time = jiffies + AUTOFS_NEGATIVE_TIMEOUT;
- dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
+ clear_bit(D_Autofs_Pending, &dentry->d_flags);
return 1;
} else if (status) {
/* Return a negative dentry, but leave it "pending" */
@@ -134,7 +134,7 @@
if (!autofs4_oz_mode(sbi))
autofs4_update_usage(dentry);

- dentry->d_flags &= ~DCACHE_AUTOFS_PENDING;
+ clear_bit(D_Autofs_Pending, &dentry->d_flags);
return 1;
}

@@ -277,7 +277,7 @@
dentry->d_op = &autofs4_root_dentry_operations;

if (!oz_mode)
- dentry->d_flags |= DCACHE_AUTOFS_PENDING;
+ set_bit(D_Autofs_Pending, &dentry->d_flags);
dentry->d_fsdata = NULL;
d_add(dentry, NULL);

@@ -291,7 +291,7 @@
* If we are still pending, check if we had to handle
* a signal. If so we can force a restart..
*/
- if (dentry->d_flags & DCACHE_AUTOFS_PENDING) {
+ if (test_bit(D_Autofs_Pending, &dentry->d_flags)) {
if (signal_pending(current))
return ERR_PTR(-ERESTARTNOINTR);
}
diff -urN S4-pre4/fs/dcache.c S4-pre4-d_flags/fs/dcache.c
--- S4-pre4/fs/dcache.c Tue Apr 17 23:40:32 2001
+++ S4-pre4-d_flags/fs/dcache.c Wed Apr 18 06:19:09 2001
@@ -337,8 +337,7 @@
dentry = list_entry(tmp, struct dentry, d_lru);

/* If the dentry was recently referenced, don't free it. */
- if (dentry->d_flags & DCACHE_REFERENCED) {
- dentry->d_flags &= ~DCACHE_REFERENCED;
+ if (test_and_clear_bit(D_Referenced, &dentry->d_flags)) {
list_add(&dentry->d_lru, &dentry_unused);
continue;
}
@@ -733,7 +732,7 @@
continue;
}
__dget_locked(dentry);
- dentry->d_flags |= DCACHE_REFERENCED;
+ set_bit(D_Referenced, &dentry->d_flags);
spin_unlock(&dcache_lock);
return dentry;
}
diff -urN S4-pre4/fs/nfs/dir.c S4-pre4-d_flags/fs/nfs/dir.c
--- S4-pre4/fs/nfs/dir.c Sun Apr 1 23:57:20 2001
+++ S4-pre4-d_flags/fs/nfs/dir.c Wed Apr 18 06:17:59 2001
@@ -567,7 +567,7 @@
dentry->d_parent->d_name.name, dentry->d_name.name,
dentry->d_flags);

- if (dentry->d_flags & DCACHE_NFSFS_RENAMED) {
+ if (test_bit(D_NFS_Renamed, &dentry->d_flags)) {
/* Unhash it, so that ->d_iput() would be called */
return 1;
}
@@ -581,7 +581,7 @@
*/
static void nfs_dentry_iput(struct dentry *dentry, struct inode *inode)
{
- if (dentry->d_flags & DCACHE_NFSFS_RENAMED) {
+ if (test_bit(D_NFS_Renamed, &dentry->d_flags)) {
lock_kernel();
nfs_complete_unlink(dentry);
unlock_kernel();
@@ -785,7 +785,7 @@
* We don't allow a dentry to be silly-renamed twice.
*/
error = -EBUSY;
- if (dentry->d_flags & DCACHE_NFSFS_RENAMED)
+ if (test_bit(D_NFS_Renamed, &dentry->d_flags))
goto out;

sprintf(silly, ".nfs%*.*lx",
@@ -859,7 +859,7 @@
}

/* If the dentry was sillyrenamed, we simply call d_delete() */
- if (dentry->d_flags & DCACHE_NFSFS_RENAMED) {
+ if (test_bit(D_NFS_Renamed, &dentry->d_flags)) {
error = 0;
goto out_delete;
}
diff -urN S4-pre4/fs/nfs/unlink.c S4-pre4-d_flags/fs/nfs/unlink.c
--- S4-pre4/fs/nfs/unlink.c Fri Feb 16 22:52:05 2001
+++ S4-pre4-d_flags/fs/nfs/unlink.c Wed Apr 18 06:17:59 2001
@@ -179,7 +179,7 @@
task->tk_action = nfs_async_unlink_init;
task->tk_release = nfs_async_unlink_release;

- dentry->d_flags |= DCACHE_NFSFS_RENAMED;
+ set_bit(D_NFS_Renamed, &dentry->d_flags);
data->cred = rpcauth_lookupcred(clnt->cl_auth, 0);

rpc_sleep_on(&nfs_delete_queue, task, NULL, NULL);
@@ -209,7 +209,7 @@
return;
data->count++;
nfs_copy_dname(dentry, data);
- dentry->d_flags &= ~DCACHE_NFSFS_RENAMED;
+ clear_bit(D_NFS_Renamed, &dentry->d_flags);
if (data->task.tk_rpcwait == &nfs_delete_queue)
rpc_wake_up_task(&data->task);
nfs_put_unlinkdata(data);
diff -urN S4-pre4/fs/nfsd/nfsfh.c S4-pre4-d_flags/fs/nfsd/nfsfh.c
--- S4-pre4/fs/nfsd/nfsfh.c Fri Feb 16 22:52:09 2001
+++ S4-pre4-d_flags/fs/nfsd/nfsfh.c Wed Apr 18 06:20:22 2001
@@ -154,7 +154,7 @@
spin_lock(&dcache_lock);
for (lp = inode->i_dentry.next; lp != &inode->i_dentry ; lp=lp->next) {
result = list_entry(lp,struct dentry, d_alias);
- if (! (result->d_flags & DCACHE_NFSD_DISCONNECTED)) {
+ if (!test_bit(D_Disconnected, &result->d_flags)) {
dget_locked(result);
spin_unlock(&dcache_lock);
iput(inode);
@@ -167,7 +167,7 @@
iput(inode);
return ERR_PTR(-ENOMEM);
}
- result->d_flags |= DCACHE_NFSD_DISCONNECTED;
+ set_bit(D_Disconnected, &result->d_flags);
d_rehash(result); /* so a dput won't loose it */
return result;
}
@@ -182,7 +182,7 @@
#ifdef NFSD_PARANOIA
if (!IS_ROOT(target))
printk("nfsd: d_splice with no-root target: %s/%s\n", parent->d_name.name, name->name);
- if (!(target->d_flags & DCACHE_NFSD_DISCONNECTED))
+ if (!test_bit(D_Disconnected, &target->d_flags))
printk("nfsd: d_splice with non-DISCONNECTED target: %s/%s\n", parent->d_name.name, name->name);
#endif
name->hash = full_name_hash(name->name, name->len);
@@ -205,9 +205,9 @@
* the children are connected, but it must be a singluar (non-forking)
* branch
*/
- if (!(parent->d_flags & DCACHE_NFSD_DISCONNECTED)) {
+ if (!test_bit(D_Disconnected, &parent->d_flags)) {
while (target) {
- target->d_flags &= ~DCACHE_NFSD_DISCONNECTED;
+ clear_bit(D_Disconnected, &target->d_flags);
parent = target;
spin_lock(&dcache_lock);
if (list_empty(&parent->d_subdirs))
@@ -266,7 +266,7 @@
if (pdentry == NULL) {
pdentry = d_alloc_root(igrab(tdentry->d_inode));
if (pdentry) {
- pdentry->d_flags |= DCACHE_NFSD_DISCONNECTED;
+ set_bit(D_Disconnected, &pdentry->d_flags);
d_rehash(pdentry);
}
}
@@ -363,14 +363,14 @@
down(&sb->s_nfsd_free_path_sem);
result = nfsd_iget(sb, ino, generation);
if (IS_ERR(result)
- || !(result->d_flags & DCACHE_NFSD_DISCONNECTED)
+ || !test_bit(D_Disconnected, &result->d_flags)
|| (!S_ISDIR(result->d_inode->i_mode) && ! needpath)) {
up(&sb->s_nfsd_free_path_sem);

err = PTR_ERR(result);
if (IS_ERR(result))
goto err_out;
- if ((result->d_flags & DCACHE_NFSD_DISCONNECTED))
+ if (test_bit(D_Disconnected, &result->d_flags))
nfsdstats.fh_anon++;
return result;
}
@@ -396,7 +396,7 @@
|| !S_ISDIR(dentry->d_inode->i_mode)) {
goto err_dentry;
}
- if (!(dentry->d_flags & DCACHE_NFSD_DISCONNECTED))
+ if (!test_bit(D_Disconnected, &dentry->d_flags))
found = 1;
tmp = splice(result, dentry);
err = PTR_ERR(tmp);
@@ -434,7 +434,7 @@
goto err_dentry;
}

- if (!(dentry->d_flags & DCACHE_NFSD_DISCONNECTED))
+ if (!test_bit(D_Disconnected, &dentry->d_flags))
found = 1;

tmp = splice(dentry, pdentry);
@@ -603,7 +603,7 @@
}
#ifdef NFSD_PARANOIA
if (S_ISDIR(dentry->d_inode->i_mode) &&
- (dentry->d_flags & DCACHE_NFSD_DISCONNECTED)) {
+ test_bit(D_Disconnected, &dentry->d_flags)) {
printk("nfsd: find_fh_dentry returned a DISCONNECTED directory: %s/%s\n",
dentry->d_parent->d_name.name, dentry->d_name.name);
}
diff -urN S4-pre4/include/linux/dcache.h S4-pre4-d_flags/include/linux/dcache.h
--- S4-pre4/include/linux/dcache.h Sun Apr 1 23:57:20 2001
+++ S4-pre4-d_flags/include/linux/dcache.h Wed Apr 18 06:17:59 2001
@@ -65,7 +65,7 @@

struct dentry {
atomic_t d_count;
- unsigned int d_flags;
+ unsigned long d_flags;
struct inode * d_inode; /* Where the name belongs to - NULL is negative */
struct dentry * d_parent; /* parent directory */
struct list_head d_vfsmnt;
@@ -111,18 +111,20 @@
*/

/* d_flags entries */
-#define DCACHE_AUTOFS_PENDING 0x0001 /* autofs: "under construction" */
-#define DCACHE_NFSFS_RENAMED 0x0002 /* this dentry has been "silly
- * renamed" and has to be
- * deleted on the last dput()
- */
-#define DCACHE_NFSD_DISCONNECTED 0x0004 /* This dentry is not currently connected to the
- * dcache tree. Its parent will either be itself,
- * or will have this flag as well.
- * If this dentry points to a directory, then
- * s_nfsd_free_path semaphore will be down
- */
-#define DCACHE_REFERENCED 0x0008 /* Recently used, don't discard. */
+enum {
+ D_Autofs_Pending, /* autofs: "under construction" */
+ D_NFS_Renamed, /* this dentry has been "silly
+ * renamed" and has to be
+ * deleted on the last dput()
+ */
+ D_Disconnected, /* This dentry is not currently connected to
+ * the dcache tree. Its parent will either be
+ * itself, or will have this flag as well.
+ * If this dentry points to a directory, then
+ * s_nfsd_free_path semaphore will be down
+ */
+ D_Referenced, /* Recently used, don't discard. */
+};

extern spinlock_t dcache_lock;

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/