On Wed, Mar 13, 2013 at 11:44 PM, Andrew Morton
<akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Wed, 13 Mar 2013 15:16:25 +0100 Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
>
>> From: Miklos Szeredi <mszeredi@xxxxxxx>
>>
>> Add a new inode operation i_op->dentry_open(). This is for stacked filesystems
>> that want to return a struct file from a different filesystem.
>>
>> ...
>>
>> +/**
>> + * vfs_open - open the file at the given path
>> + * @path: path to open
>> + * @filp: newly allocated file with f_flag initialized
>> + * @cred: credentials to use
>> + */
>> +int vfs_open(const struct path *path, struct file *filp,
>> + const struct cred *cred)
>> +{
>> + struct inode *inode = path->dentry->d_inode;
>> +
>> + if (inode->i_op->dentry_open)
>> + return inode->i_op->dentry_open(path->dentry, filp, cred);
>> + else {
>> + filp->f_path = *path;
>> + return do_dentry_open(filp, NULL, cred);
>> + }
>> +}
>
> This looks like it will add some overhead to dentry_open(). That long
> walk path->dentry->d_inode->i_op->dentry_open, particularly. Can we do
> anything? Using filp->f_inode might save a tad.
filp->f_inode is initialized in do_dentry_open. But we can move that
to callers.
Adding an IOP_DENTRY_OPEN flag should take care of the rest.
> It's unfortunate and a bit ugly that ->dentry_open() and
> do_dentry_open() don't have the same arguments. One would expect and
> like them to do so, and this difference raises suspicions about design
> warts.
Hmm, the basic reason for the difference is that filesystems should
not have access to the vfsmount part of the path.
Otherwise it would be trivial to make them match up.
>
> If they _did_ have the same signature then we could perhaps populate
> ->dentry_open with do_dentry_open by default and avoid the `if'.
Except there's no way to add a default i_op, AFAIK.
>
> The test of inode->i_op->dentry_open could do with an unlikely(), but
> that won't save us :(
>
>> +EXPORT_SYMBOL(vfs_open);
>
> Did you consider EXPORT_SYMBOL_GPL()?
It is not clear to me what to base that decision on. Either one is fine by me.
Thanks,
Miklos
--
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
