First please Cc Anton on this as he wrote the original version of what's now d_add_ci, I suspect he might have some useful comments. On Tue, May 13, 2008 at 05:57:52PM +1000, Barry Naujok wrote: > Another unusual interaction with the dcache is not storing > negative dentries like other filesystems doing a d_add(dentry, NULL) > when an ENOENT is returned. During the VFS lookup, if a dentry > returned has no inode, dput is called and ENOENT is returned. > By not doing a d_add, this actually removes it completely from > the dcache to be reused. That is a way to implement this correctly, but I suspect not creating negative dentries will degrade performance quite badly on some workloads. Then again CI is useful only for samba serving where the namecache on the client side should mitigate that effect. We'd probably be better off long-term implementing Anton's earlier suggestion to have a routine that purges all ci aliased negative dentries on a successfull lookup. > create/rename have to be modified to > support unhashed dentries being passed in. > + if (ci_sfep) > + return XFS_ERROR(xfs_dir_cilookup_result(args, > + ci_sfep->name, ci_sfep->namelen)); Putting a function call inside XFS_ERROR is quite unreadable. Should be easy to fix as there's already an error variable in scope. > @@ -1646,15 +1653,18 @@ xfs_lookup( > return XFS_ERROR(EIO); > > lock_mode = xfs_ilock_map_shared(dp); > - error = xfs_dir_lookup(NULL, dp, name, &inum); > + error = xfs_dir_lookup(NULL, dp, name, &inum, ci_match); > xfs_iunlock_map_shared(dp, lock_mode); > > if (error) > goto out; > > error = xfs_iget(dp->i_mount, NULL, inum, 0, 0, ipp, 0); > - if (error) > + if (error) { > + if (ci_match && *ci_match) > + kmem_free(name->name, name->len); > goto out; normal style would be to add a out_free_name label for this one to move the error handling code into one place at the end of the function. -- 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