NFS exporting

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

NFS exporting

Cool Frood
Hi,

I apologise in advance for the highly-technical nature
of this email. :-)

I am trying to implement proper NFS support for FUSE
by implementing my own FUSE user-space library that
can support nodeid-based lookups.  To get this
support, I have implemented two export_operations:
get_dentry() and get_parent() which look something
like this:

struct inode* fuse_ilookup(struct super_block *sb,
unsigned long nodeid)
{
    struct fuse_conn *fc = get_fuse_conn_super(sb);
    struct fuse_req *req = fuse_get_request(fc);
    struct fuse_entry_out outarg;
    struct inode* inode = NULL;
    int err;
    if (!req)
        return 0;
    req->in.h.opcode = FUSE_ILOOKUP;
    req->in.h.nodeid = nodeid;
    req->inode = NULL;
    req->in.numargs = 0;
    req->out.numargs = 1;
    req->out.args[0].size = sizeof(struct
fuse_entry_out);
    req->out.args[0].value = &outarg;

    request_send(fc, req);

    err = req->out.h.error;
    if (!err) {
        inode = fuse_iget(sb, nodeid,
outarg.generation, &outarg.attr);
        if (!inode) {
            fuse_send_forget(fc, req, outarg.nodeid,
1);
            return ERR_PTR(-ENOMEM);
        }
    }
    fuse_put_request(fc, req);
    return inode;
}

static struct dentry *fuse_get_dentry(struct
super_block *sb, void *vobjp)
{
    __u32 *objp = vobjp;
    unsigned long nodeid = objp[0];
    __u32 generation = objp[1];
    struct inode *inode;
    struct dentry *entry;

    if (nodeid == 0)
        return ERR_PTR(-ESTALE);

    inode = ilookup5(sb, nodeid, fuse_inode_eq,
&nodeid);
    if (!inode) {
        inode = fuse_ilookup(sb, nodeid);
        if (IS_ERR(inode))
            return ERR_PTR(PTR_ERR(inode));
    }
    if (!inode)
        return ERR_PTR(-ESTALE);
    if (inode->i_generation != generation) {
        iput(inode);
        return ERR_PTR(-ESTALE);
    }

    entry = d_alloc_anon(inode);
    if (!entry) {
        iput(inode);
        return ERR_PTR(-ENOMEM);
    }
    return entry;
}

extern int fuse_lookup_iget(struct inode* dir, struct
dentry *entry, struct inode **inodep);

static struct dentry* fuse_get_parent(struct dentry*
child)
{
    struct inode *inode = NULL;
    struct dentry *parent;

    struct dentry dotdot;
    dotdot.d_name.name = "..";
    dotdot.d_name.len = 2;

    int err = fuse_lookup_iget(child->d_inode,
&dotdot, &inode);
    if (err)
        return ERR_PTR(err);
    if (!inode)
        return ERR_PTR(-EACCES);
    parent = d_alloc_anon(inode);
    if (!parent) {
        iput(inode);
        parent = ERR_PTR(-ENOMEM);
    }
    return parent;
}


Essentially, I modified get_dentry to do an "ilookup"
if the inode is not in the cache.  Also, I added
get_parent.  I based these on implementation in other
filesystems.

The problem I have is with this set of operations for
NFS mount on /mnt:

mkdir -p /mnt/a/b/c/
echo foo > /mnt/a/b/c/foo
cd /mnt/a/b/c
cat foo
#stop NFS server
#unmount FUSE
#mount FUSE
#start NFS server
cat foo

This causes get_dentry() and get_parent() to be
exercised.  I don't get ESTALE as with original FUSE
implementation.  However, on unmount, I get busy
inodes for all the parent directories of "foo".

Any idea what I might be doing wrong for the refcounts
in these functions?  All other filesystems implement
them in similar ways.

Thanks,
Akshat


               
__________________________________
Yahoo! Mail - PC Magazine Editors' Choice 2005
http://mail.yahoo.com


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
fuse-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: NFS exporting

Miklos Szeredi
> Any idea what I might be doing wrong for the refcounts
> in these functions?  All other filesystems implement
> them in similar ways.

I don't see that you increase ->nlookup after a successful ILOOKUP
operation.  Maybe that's causing the problem.

Miklos


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
fuse-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: NFS exporting

Cool Frood


--- Miklos Szeredi <[hidden email]> wrote:

> > Any idea what I might be doing wrong for the
> refcounts
> > in these functions?  All other filesystems
> implement
> > them in similar ways.
>
> I don't see that you increase ->nlookup after a
> successful ILOOKUP
> operation.  Maybe that's causing the problem.
>

the ILOOKUP operation uses fuse_iget after we get the
attributes from userspace.  fuse_iget will increase
nlookup.

One difference between other filesystems and FUSE is
that FUSE is not device-backed.  Could that be an
issue?

> Miklos
>

-Akshat


               
__________________________________
Yahoo! Mail - PC Magazine Editors' Choice 2005
http://mail.yahoo.com


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
fuse-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: NFS exporting

Miklos Szeredi
> > I don't see that you increase ->nlookup after a
> > successful ILOOKUP
> > operation.  Maybe that's causing the problem.
> >
>
> the ILOOKUP operation uses fuse_iget after we get the
> attributes from userspace.  fuse_iget will increase
> nlookup.

Right, I missed that.

> One difference between other filesystems and FUSE is
> that FUSE is not device-backed.  Could that be an
> issue?

Lots of other filesystems are not device-backed: all network
filesystems, ramfs, tmpfs, proc, etc.  But some of them surely support
NFS exporting.

I don't know what is causing leaked inodes, but the key is probably in
those export methods.

Miklos


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
fuse-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: NFS exporting

Cool Frood


--- Miklos Szeredi <[hidden email]> wrote:


>
> > One difference between other filesystems and FUSE
> is
> > that FUSE is not device-backed.  Could that be an
> > issue?
>
> Lots of other filesystems are not device-backed: all
> network
> filesystems, ramfs, tmpfs, proc, etc.  But some of
> them surely support
> NFS exporting.
>

There's a difference between exporting and "exporting
properly", meaning that you provide proper
implementations for export operations that would let
you create an inode based on the inode number offered
by the client and connect it back into the dentry tree
by walking up.

ramfs, tmpfs, and proc aren't even persistent, so an
inode-number based lookup is a moot point.  I don't
know if any of the network filesystems can be
re-exported though.  I looked through the code, and
nfs client-side does not seem to provide export
operations.

> I don't know what is causing leaked inodes, but the
> key is probably in
> those export methods.
>
> Miklos
>

-Akshat


               
__________________________________
Yahoo! Mail - PC Magazine Editors' Choice 2005
http://mail.yahoo.com


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
fuse-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: NFS exporting

Cool Frood
In reply to this post by Miklos Szeredi
Miklos,

I believe I have tracked down the problem that I'm
getting.  fuse_lookup calls d_find_alias for a
directory to check if an alias is being created.
There is a check:

if (alias && !(alias->d_flags & DCACHE_DISCONNECTED))

which, if true, dputs the alias and iputs the inode
and returns.  However, in my case, with NFS, alias is
present but the other condition is false, so this
alias is never put back and there is an extra
reference for the dentry.  If I change the code to:

if (alias && !(alias->d_flags & DCACHE_DISCONNECTED))
{
            dput(alias);
            iput(inode);
            return ERR_PTR(-EIO);
} else if (alias) {
            dput(alias);
}

this seems to fix the problem.  Is this fix correct?

-Akshat



--- Miklos Szeredi <[hidden email]> wrote:

> > > I don't see that you increase ->nlookup after a
> > > successful ILOOKUP
> > > operation.  Maybe that's causing the problem.
> > >
> >
> > the ILOOKUP operation uses fuse_iget after we get
> the
> > attributes from userspace.  fuse_iget will
> increase
> > nlookup.
>
> Right, I missed that.
>
> > One difference between other filesystems and FUSE
> is
> > that FUSE is not device-backed.  Could that be an
> > issue?
>
> Lots of other filesystems are not device-backed: all
> network
> filesystems, ramfs, tmpfs, proc, etc.  But some of
> them surely support
> NFS exporting.
>
> I don't know what is causing leaked inodes, but the
> key is probably in
> those export methods.
>
> Miklos
>
>
>
-------------------------------------------------------
> This SF.Net email is sponsored by:
> Power Architecture Resource Center: Free content,
> downloads, discussions,
> and more.
> http://solutions.newsforge.com/ibmarch.tmpl
> _______________________________________________
> fuse-devel mailing list
> [hidden email]
>
https://lists.sourceforge.net/lists/listinfo/fuse-devel
>



               
__________________________________
Yahoo! Mail - PC Magazine Editors' Choice 2005
http://mail.yahoo.com


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
fuse-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: NFS exporting

Cool Frood
Here's a patch against the latest cvs:

Signed-off-by:  Akshat Aranya <[hidden email]>

Index: kernel/dir.c
===================================================================
RCS file: /cvsroot/fuse/fuse/kernel/dir.c,v
retrieving revision 1.87
diff -u -r1.87 dir.c
--- kernel/dir.c    23 Sep 2005 13:34:44 -0000  1.87
+++ kernel/dir.c    29 Sep 2005 21:15:18 -0000
@@ -821,10 +821,14 @@
    if (inode && S_ISDIR(inode->i_mode)) {
        /* Don't allow creating an alias to a
directory  */
        struct dentry *alias = d_find_alias(inode);
-       if (alias && !(alias->d_flags &
DCACHE_DISCONNECTED)) {
-           dput(alias);
-           iput(inode);
-           return ERR_PTR(-EIO);
+       if (alias) {
+           if (alias->d_flags & DCACHE_DISCONNECTED)
{
+               dput(alias);
+           } else {
+               dput(alias);
+               iput(inode);
+               return ERR_PTR(-EIO);
+           }
        }
    }
    return d_splice_alias(inode, entry);

-Akshat

--- Cool Frood <[hidden email]> wrote:

> Miklos,
>
> I believe I have tracked down the problem that I'm
> getting.  fuse_lookup calls d_find_alias for a
> directory to check if an alias is being created.
> There is a check:
>
> if (alias && !(alias->d_flags &
> DCACHE_DISCONNECTED))
>
> which, if true, dputs the alias and iputs the inode
> and returns.  However, in my case, with NFS, alias
> is
> present but the other condition is false, so this
> alias is never put back and there is an extra
> reference for the dentry.  If I change the code to:
>
> if (alias && !(alias->d_flags &
> DCACHE_DISCONNECTED))
> {
>             dput(alias);
>             iput(inode);
>             return ERR_PTR(-EIO);
> } else if (alias) {
>             dput(alias);
> }
>
> this seems to fix the problem.  Is this fix correct?
>
> -Akshat
>
>
>
> --- Miklos Szeredi <[hidden email]> wrote:
>
> > > > I don't see that you increase ->nlookup after
> a
> > > > successful ILOOKUP
> > > > operation.  Maybe that's causing the problem.
> > > >
> > >
> > > the ILOOKUP operation uses fuse_iget after we
> get
> > the
> > > attributes from userspace.  fuse_iget will
> > increase
> > > nlookup.
> >
> > Right, I missed that.
> >
> > > One difference between other filesystems and
> FUSE
> > is
> > > that FUSE is not device-backed.  Could that be
> an
> > > issue?
> >
> > Lots of other filesystems are not device-backed:
> all
> > network
> > filesystems, ramfs, tmpfs, proc, etc.  But some of
> > them surely support
> > NFS exporting.
> >
> > I don't know what is causing leaked inodes, but
> the
> > key is probably in
> > those export methods.
> >
> > Miklos
> >
> >
> >
>
-------------------------------------------------------

> > This SF.Net email is sponsored by:
> > Power Architecture Resource Center: Free content,
> > downloads, discussions,
> > and more.
> > http://solutions.newsforge.com/ibmarch.tmpl
> > _______________________________________________
> > fuse-devel mailing list
> > [hidden email]
> >
>
https://lists.sourceforge.net/lists/listinfo/fuse-devel

> >
>
>
>
>
> __________________________________
> Yahoo! Mail - PC Magazine Editors' Choice 2005
> http://mail.yahoo.com
>
>
>
-------------------------------------------------------
> This SF.Net email is sponsored by:
> Power Architecture Resource Center: Free content,
> downloads, discussions,
> and more.
> http://solutions.newsforge.com/ibmarch.tmpl
> _______________________________________________
> fuse-devel mailing list
> [hidden email]
>
https://lists.sourceforge.net/lists/listinfo/fuse-devel
>



               
__________________________________
Yahoo! Mail - PC Magazine Editors' Choice 2005
http://mail.yahoo.com


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
fuse-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: NFS exporting

Miklos Szeredi
In reply to this post by Cool Frood
> I believe I have tracked down the problem that I'm
> getting.  fuse_lookup calls d_find_alias for a
> directory to check if an alias is being created.
> There is a check:
>
> if (alias && !(alias->d_flags & DCACHE_DISCONNECTED))
>
> which, if true, dputs the alias and iputs the inode
> and returns.  However, in my case, with NFS, alias is
> present but the other condition is false, so this
> alias is never put back and there is an extra
> reference for the dentry.  If I change the code to:
>
> if (alias && !(alias->d_flags & DCACHE_DISCONNECTED))
> {
>             dput(alias);
>             iput(inode);
>             return ERR_PTR(-EIO);
> } else if (alias) {
>             dput(alias);
> }
>
> this seems to fix the problem.  Is this fix correct?

Yes, great find!  Committed to CVS.

Just a minor nit: the 'if (alias)' is not needed, because dput checks
for NULL argument.

Could this be the cause of the lockups too?

Thanks,
Miklos


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
fuse-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: NFS exporting

Cool Frood


--- Miklos Szeredi <[hidden email]> wrote:

> > I believe I have tracked down the problem that I'm
> > getting.  fuse_lookup calls d_find_alias for a
> > directory to check if an alias is being created.
> > There is a check:
> >
> > if (alias && !(alias->d_flags &
> DCACHE_DISCONNECTED))
> >
> > which, if true, dputs the alias and iputs the
> inode
> > and returns.  However, in my case, with NFS, alias
> is
> > present but the other condition is false, so this
> > alias is never put back and there is an extra
> > reference for the dentry.  If I change the code
> to:
> >
> > if (alias && !(alias->d_flags &
> DCACHE_DISCONNECTED))
> > {
> >             dput(alias);
> >             iput(inode);
> >             return ERR_PTR(-EIO);
> > } else if (alias) {
> >             dput(alias);
> > }
> >
> > this seems to fix the problem.  Is this fix
> correct?
>
> Yes, great find!  Committed to CVS.
>
> Just a minor nit: the 'if (alias)' is not needed,
> because dput checks
> for NULL argument.
>
> Could this be the cause of the lockups too?
>

I doubt if this is related to the lockup problem,
because the lockup scenario is simple WRITE calls
which would not use the lookup code, but if Franco can
test it out, that would be great.  I will get to
investigating that eventually.

> Thanks,
> Miklos
>
-Akshat


               
__________________________________
Yahoo! Mail - PC Magazine Editors' Choice 2005
http://mail.yahoo.com


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
fuse-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: NFS exporting

Miklos Szeredi
> > Could this be the cause of the lockups too?
> >
>
> I doubt if this is related to the lockup problem,
> because the lockup scenario is simple WRITE calls
> which would not use the lookup code,

Traces from Franco indicated that it's in locking up in
readdir/getattr.  But it was not OOM, so the dentry leak doesn't fit
the picture too well, I admit.

Miklos


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
fuse-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/fuse-devel