On Mon, Jul 18, 2011 at 08:03:14AM -0400, Jeff Layton wrote:
> It currently calls d_lookup to get a dentry and then passes that to
> d_materialise_unique. This is wrong as d_materialise_unique is intended
> to introduce a new dentry into the tree. It also uses d_lookup when
> lookup_one_len would generally be a better choice since it does
> permission checks.
>
> Also, fix the dentry hash calculation to work with nocase mounts.
Huh? First of all, lookup_one_len() doesn't return NULL on failure,
it returns ERR_PTR(). What's more, it already does d_alloc(), ->lookup(),
etc. and you don't need to bother with d_materialise_unique() and this
lookup-by-hand code in there. Or with calculating hash - also done
by lookup_one_len(), TYVM... If anything, I'd start with this as the first
approximation and probably looked into simplifying the loop a bit more -
lookup_one_len() doesn't need name component to be NUL-terminated...
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 112fbd96..2d74619 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -574,51 +574,13 @@ cifs_get_root(struct smb_vol *vol, struct super_block *sb)
full_path[i] = 0;
cFYI(1, "get dentry for %s", pstart);
- name.name = pstart;
- name.len = len;
- name.hash = full_name_hash(pstart, len);
- dchild = d_lookup(dparent, &name);
- if (dchild == NULL) {
- cFYI(1, "not exists");
- dchild = d_alloc(dparent, &name);
- if (dchild == NULL) {
- dput(dparent);
- dparent = ERR_PTR(-ENOMEM);
- goto out;
- }
- }
-
- cFYI(1, "get inode");
- if (dchild->d_inode == NULL) {
- cFYI(1, "not exists");
- inode = NULL;
- if (cifs_sb_master_tcon(CIFS_SB(sb))->unix_ext)
- rc = cifs_get_inode_info_unix(&inode, full_path,
- sb, xid);
- else
- rc = cifs_get_inode_info(&inode, full_path,
- NULL, sb, xid, NULL);
- if (rc) {
- dput(dchild);
- dput(dparent);
- dparent = ERR_PTR(rc);
- goto out;
- }
- alias = d_materialise_unique(dchild, inode);
- if (alias != NULL) {
- dput(dchild);
- if (IS_ERR(alias)) {
- dput(dparent);
- dparent = ERR_PTR(-EINVAL); /* XXX */
- goto out;
- }
- dchild = alias;
- }
- }
- cFYI(1, "parent %p, child %p", dparent, dchild);
-
+ mutex_lock(&dparent->d_inode->i_mutex);
+ dchild = lookup_one_len(pstart, dparent->d_inode, len);
+ mutex_unlock(&dparent->d_inode->i_mutex);
dput(dparent);
dparent = dchild;
+ if (IS_ERR(dparent))
+ break;
len = 0;
pstart = full_path + i + 1;
full_path[i] = sep;
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
