On Wed, Jul 10, 2013 at 10:45:45AM -0700, Mark Fasheh wrote:
> Well, what do I get when I pretend I don't care any more? The little voice
> in my head says "keep plugging away". Here's another attempt at fixing this
> problem in a sane manner. Basically, this time we're adding a flag to
> s_flags which btrfs sets. Proc will see the flag and call ->getattr().
>
> This compiles, but it needs testing (which I will get to soon). It still has
> a bunch of problems in my honest opinion but maybe if we get something
> acceptable upstream we can work from there.
>
> Also, as Andrew pointed out there's more than one place which is return
> different device than from stat(2) so I probably need to update more sites
> to deal with this.
>
> Does anyone see a problem with this approach?
The approach looks ok to me, the implementation is internal to vfs and
fairly minimal. The bit that bothers me is the name of the flag, it's
completely unobvious what it means.
There are some differences to the linked suse patch:
> +static dev_t proc_get_dev_from_stat(struct inode *inode)
> +{
> + struct dentry *dentry = d_find_any_alias(inode);
This does the dentry -> inode mapping, while originally there was
&file->f_path
passing just the inode to proc_get_dev_from_stat unnecessarily drops the
available information that's about to be retrieved again.
> + struct kstat kstat;
> +
> + if (!dentry)
> + goto out_error;
> + if (inode->i_op->getattr(NULL, dentry, &kstat))
The suse patch calls vfs_getattr that in turn calls
security_inode_getattr(path->mnt, path->dentry);
That would be missing.
Plus checks for presence of the ->getattr operation. Though this is
superfluous with btrfs, I suggest to use vfs_getattr here, which will
fix all of the above.
> + goto out_error_dput;
> +
> + dput(dentry);
> + return kstat.dev;
> +
> +out_error_dput:
> + dput(dentry);
> +out_error:
> + return inode->i_sb->s_dev;
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
