On Thu, 2011-01-13 at 11:03 +0800, Ian Kent wrote: > On Thu, 2011-01-13 at 10:23 +0800, Ian Kent wrote: > > On Wed, 2011-01-12 at 14:11 -0600, Alex Elder wrote: > > > On Wed, 2011-01-12 at 12:15 +0800, Ian Kent wrote: > > > > On Wed, 2011-01-12 at 11:59 +0800, Ian Kent wrote: . . . > > > > Could you try this patch please. > > > > > > OK, sorry for the delay. I tried the patch. I applied > > > it against 4162cf64973df51fc885825bc9ca4d055891c49f, > > > which is the linus/master branch I had on hand. This > > > time I got a different failure due to a null pointer > > > dereference. Console capture below. I can log > > > in still but the boot sequence never got to the > > > login prompt on the console as it normally does. > > > > > > Sorry, that is rather an obvious mistake on my part. > > There's a call to d_op->d_revalidate() just below where the dentry > > operations are set. I'm tempted to just call the revalidate function > > directly since it is always called anyway. But let me check we've done > > in the our current of tree autofs patch series first. > > Ha, there is no revalidate in the current not yet merged autofs. > So I think we just need to work around it. > > Try this and see if it resolves the issue. This one works. I am able to boot without hitting the BUG_ON() in d_set_d_op() and there is no longer the null pointer dereference that occurred with the last one. -Alex > autofs4 - set dentry op in ->lookup() only > > From: Ian Kent <raven@xxxxxxxxxx> > > With the introduction of the vfs-scale patch series setting dentry > operations more than once (or changing them) triggers a BUG_ON(). > Since the two dentry operations used are the same, just set the > operations in ->lookup() and remove the set in ->symlink() and > ->mkdir(). > > Also, move the d_add() in ->symlink() and ->mkdir() to the end of > the function. > --- > > fs/autofs4/root.c | 40 +++++++--------------------------------- > 1 files changed, 7 insertions(+), 33 deletions(-) > > > diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c > index 651e4ef..0897706 100644 > --- a/fs/autofs4/root.c > +++ b/fs/autofs4/root.c > @@ -419,12 +419,6 @@ void autofs4_dentry_release(struct dentry *de) > } > } > > -/* For dentries of directories in the root dir */ > -static const struct dentry_operations autofs4_root_dentry_operations = { > - .d_revalidate = autofs4_revalidate, > - .d_release = autofs4_dentry_release, > -}; > - > /* For other dentries */ > static const struct dentry_operations autofs4_dentry_operations = { > .d_revalidate = autofs4_revalidate, > @@ -568,19 +562,6 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s > ino = autofs4_dentry_ino(dentry); > } else { > /* > - * Mark the dentry incomplete but don't hash it. We do this > - * to serialize our inode creation operations (symlink and > - * mkdir) which prevents deadlock during the callback to > - * the daemon. Subsequent user space lookups for the same > - * dentry are placed on the wait queue while the daemon > - * itself is allowed passage unresticted so the create > - * operation itself can then hash the dentry. Finally, > - * we check for the hashed dentry and return the newly > - * hashed dentry. > - */ > - d_set_d_op(dentry, &autofs4_root_dentry_operations); > - > - /* > * And we need to ensure that the same dentry is used for > * all following lookup calls until it is hashed so that > * the dentry flags are persistent throughout the request. > @@ -589,6 +570,8 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s > if (!ino) > return ERR_PTR(-ENOMEM); > > + d_set_d_op(dentry, &autofs4_dentry_operations); > + > dentry->d_fsdata = ino; > ino->dentry = dentry; > > @@ -714,12 +697,6 @@ static int autofs4_dir_symlink(struct inode *dir, > kfree(ino); > return -ENOMEM; > } > - d_add(dentry, inode); > - > - if (dir == dir->i_sb->s_root->d_inode) > - d_set_d_op(dentry, &autofs4_root_dentry_operations); > - else > - d_set_d_op(dentry, &autofs4_dentry_operations); > > dentry->d_fsdata = ino; > ino->dentry = dget(dentry); > @@ -732,6 +709,8 @@ static int autofs4_dir_symlink(struct inode *dir, > ino->u.symlink = cp; > dir->i_mtime = CURRENT_TIME; > > + d_add(dentry, inode); > + > return 0; > } > > @@ -849,12 +828,6 @@ static int autofs4_dir_mkdir(struct inode *dir, struct dentry *dentry, int mode) > kfree(ino); > return -ENOMEM; > } > - d_add(dentry, inode); > - > - if (dir == dir->i_sb->s_root->d_inode) > - d_set_d_op(dentry, &autofs4_root_dentry_operations); > - else > - d_set_d_op(dentry, &autofs4_dentry_operations); > > dentry->d_fsdata = ino; > ino->dentry = dget(dentry); > @@ -866,6 +839,8 @@ static int autofs4_dir_mkdir(struct inode *dir, struct dentry *dentry, int mode) > inc_nlink(dir); > dir->i_mtime = CURRENT_TIME; > > + d_add(dentry, inode); > + > return 0; > } > > @@ -944,8 +919,7 @@ static inline int autofs4_ask_umount(struct vfsmount *mnt, int __user *p) > int is_autofs4_dentry(struct dentry *dentry) > { > return dentry && dentry->d_inode && > - (dentry->d_op == &autofs4_root_dentry_operations || > - dentry->d_op == &autofs4_dentry_operations) && > + (dentry->d_op == &autofs4_dentry_operations) && > dentry->d_fsdata != NULL; > } > > > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html