2011/8/26 Jeff Layton <jlayton@xxxxxxxxxx>:
...
>> +static struct dentry *
>> +cifs_alloc_root(struct super_block *sb)
>> +{
>> + struct qstr q;
>> +
>> + q.name = "/";
>
> I don't think you want a separator in the name. That seems
> like it should be "".
Ok, I will try.
>
>> + q.len = 1;
>> + q.hash = full_name_hash(q.name, q.len);
>> + return d_alloc_pseudo(sb, &q);
>> +}
>> +
...
>> - if (filldir(direntry, "..", 2, file->f_pos,
>> - parent_ino(file->f_path.dentry), DT_DIR) < 0) {
>> + if (!file->f_path.dentry->d_parent->d_inode) {
>> + cFYI(1, "parent dir is negative, filling as current");
>> + ino = file->f_path.dentry->d_inode->i_ino;
>> + } else
>> + ino = parent_ino(file->f_path.dentry);
>> + if (filldir(direntry, "..", 2, file->f_pos, ino, DT_DIR) < 0) {
>> cERROR(1, "Filldir for parent dir failed");
>> rc = -ENOMEM;
>> break;
>>
> (cc'ing David Howells since he did the sb sharing code for NFS)
>
> This doesn't seem quite right to me...
>
> This implies that the parent in this situation would be the parent
> dentry on the server, but that shouldn't be the case right? Shouldn't
> this be the mountpoint's parent?
>
> For instance, suppose you have //server/share/d1/d2/d3 mounted
> on /mnt/cifs. During the mount, d1 and d2 are instantiated as negative
> dentries. Here, you try to fill in the i_ino for d2 as the inode number
> for "..", but that doesn't seem correct -- shouldn't it be the inode
> number for /mnt?
I think that doesn't seem correct too, but I looked through another
file systems code and found the same things.
>
> I don't have a great feel for this sort of dcache trickery, but I tend
> to think we probably ought to follow NFS' example here. It has sort of
> an "opportunistic" mechanism for filling in the dcache for a particular
> superblock. This is detailed in the comments on commit 54ceac45.
As I understand, NFS does the following:
1) requests fattr from the server
error = server->nfs_client->rpc_ops->getroot(server, mntfh, &fsinfo);
2) lookup an inode with ino got from the server
inode = nfs_fhget(sb, mntfh, fsinfo.fattr);
3) set dummy root
error = nfs_superblock_set_dummy_root(sb, inode);
4) get dentry for the inode
ret = d_obtain_alias(inode);
So, it lookup an existing dentry according to the ino got from the
server. As for CIFS, we can't do it because we may end up using a
different inode numbers for the same path (noserverino cases).
Thoughts?
--
Best regards,
Pavel Shilovsky.
--
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
