lkml.org 
[lkml]   [2013]   [Mar]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 1/9] vfs: add i_op->dentry_open()
From
On Wed, Mar 13, 2013 at 11:44 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Wed, 13 Mar 2013 15:16:25 +0100 Miklos Szeredi <miklos@szeredi.hu> wrote:
>
>> From: Miklos Szeredi <mszeredi@suse.cz>
>>
>> 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


\
 
 \ /
  Last update: 2013-03-14 13:01    [W:0.181 / U:0.224 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site