On Sun, Jun 29, 2008 at 05:05:31AM +0530, Balaji Rao wrote:
> Hello,
>
> Here's a quick implementation of NFS support for btrfs. It works mostly. But
> its still a bit buggy. I suspect it has to do something with inode locking.
> When I unmount and mount a couple of times, it stops working i.e, when I do an
> 'ls', it keeps waiting indefinitely. This is seen when accessing the filesystem
> from a local mount point as well.
>
> Can anybody please review and tell me what stupid mistake I'm committing ?
>
You may want to send things in patch form. I'd recommend using the hg quilt
plugin, its nice for adding patches easily.
>
> #include <linux/fs.h>
> #include <linux/types.h>
> #include "ctree.h"
> #include "disk-io.h"
> #include "btrfs_inode.h"
> #include "print-tree.h"
> #include "export.h"
>
> static int btrfs_encode_fh(struct dentry *dentry, u32 *fh_in, int *max_len,
> int connectable)
> { struct inode *inode = dentry->d_inode;
> int len = *max_len;
> u64 objectid, root_objectid;
> u32 generation;
> __le32 *fh = (__force __le32 *) fh_in;
>
> if ((len < 5 )|| (connectable && len < 8)) {
> return 255;
> }
Magic numbers are a no-no.
>
> objectid = BTRFS_I(inode)->location.objectid;
> root_objectid = BTRFS_I(inode)->root->objectid;
> generation = inode->i_generation;
>
> len = 5;
> fh[0] = cpu_to_le32((u32)(objectid >> 32));
> fh[1] = cpu_to_le32((u32)(objectid & 0xffffffff));
> fh[2] = cpu_to_le32((u32)(root_objectid >> 32));
> fh[3] = cpu_to_le32((u32)(root_objectid & 0xffffffff));
> fh[4] = cpu_to_le32(generation);
>
> if (connectable && !S_ISDIR(inode->i_mode)) {
> struct inode *parent;
>
> spin_lock(&dentry->d_lock);
>
> parent = dentry->d_parent->d_inode;
> objectid = BTRFS_I(parent)->location.objectid;
> generation = parent->i_generation;
>
> fh[5] = cpu_to_le32((u32)(objectid >> 32));
> fh[6] = cpu_to_le32((u32)(objectid & 0xffffffff));
> fh[7] = cpu_to_le32(generation);
>
> spin_unlock(&dentry->d_lock);
>
> len += 3;
there is no need to take the d_lock here.
> }
>
> *max_len = len;
> return len;
>
> }
>
> static struct dentry * btrfs_get_dentry(struct super_block *sb,
> u64 objectid, u64 root_objectid, u32 generation)
> {
> struct inode *inode;
> struct dentry *result;
>
> inode = btrfs_ilookup(sb, objectid, root_objectid);
> if (IS_ERR(inode))
> return (void *)inode;
>
> if(generation != inode->i_generation) {
> iput(inode);
> return ERR_PTR(-ESTALE);
> }
>
> result = d_alloc_anon(inode);
> if(!result) {
> iput(inode);
> return ERR_PTR(-ENOMEM);
> }
>
> return result;
> }
>
> static struct dentry *btrfs_fh_to_parent(struct super_block *sb,
> struct fid *fid, int fh_len, int fh_type)
> {
> u64 objectid, root_objectid;
> u32 generation;
>
> if (fh_len < 8 || fh_type != 8) {
> return NULL;
> }
>
>
> root_objectid = (u64)le32_to_cpu(fid->raw[2]) << 32;
> root_objectid |= (u64)le32_to_cpu(fid->raw[3]);
> objectid = (u64)le32_to_cpu(fid->raw[5]) << 32;
> objectid |= (u64)le32_to_cpu(fid->raw[6]);
> generation = le32_to_cpu(fid->raw[7]);
>
> return btrfs_get_dentry(sb, objectid, root_objectid, generation);
> }
>
> static struct dentry *btrfs_fh_to_dentry(struct super_block *sb,
> struct fid *fid, int fh_len, int fh_type)
> {
> u64 objectid, root_objectid;
> u32 generation;
>
> if (fh_len < 5 || fh_type > 5) {
> return NULL;
> }
>
> objectid = (u64)le32_to_cpu(fid->raw[0]) << 32;
> objectid |= (u64)le32_to_cpu(fid->raw[1]);
> root_objectid = (u64)le32_to_cpu(fid->raw[2]) << 32;
> root_objectid |= (u64)le32_to_cpu(fid->raw[3]);
> generation = le32_to_cpu(fid->raw[4]);
>
> return btrfs_get_dentry(sb, objectid, root_objectid, generation);
> }
>
> static struct dentry *btrfs_get_parent(struct dentry *child)
> {
> struct inode *dir = child->d_inode;
> struct inode *inode;
> struct dentry *parent;
> struct btrfs_root *root = BTRFS_I(dir)->root;
> struct btrfs_key key;
> struct btrfs_path *path;
> struct extent_buffer *leaf;
> u32 nritems;
> int slot;
> u64 objectid;
> int ret;
>
> path = btrfs_alloc_path();
>
> key.objectid = dir->i_ino;
> btrfs_set_key_type(&key, BTRFS_INODE_REF_KEY);
> key.offset = 0;
> ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
> BUG_ON(ret == 0);
> ret = 0;
>
> leaf = path->nodes[0];
> slot = path->slots[0];
> nritems = btrfs_header_nritems(leaf);
> if (slot >= nritems)
> return ERR_PTR(-EINVAL);
>
> btrfs_item_key_to_cpu(leaf, &key, slot);
> if (key.objectid != dir->i_ino ||
> key.type != BTRFS_INODE_REF_KEY) {
> return ERR_PTR(-EINVAL);
> }
>
in both of the above error cases you don't free the path, you need to do that.
> btrfs_free_path(path);
> objectid = key.offset;
> inode = btrfs_iget_locked(root->fs_info->sb, objectid, root);
>
> parent = d_alloc_anon(inode);
> if (!parent) {
> iput(inode);
> parent = ERR_PTR(-ENOMEM);
> }
>
> return parent;
> }
>
> const struct export_operations btrfs_export_ops = {
> .encode_fh = btrfs_encode_fh,
> .fh_to_dentry = btrfs_fh_to_dentry,
> .fh_to_parent = btrfs_fh_to_parent,
> .get_parent = btrfs_get_parent,
> };
I don't know much about NFS so I can't really comment on much of anything else
other than the obvious problems, but that should point you in a general
direction.
Josef
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
