[zfs-fuse] zfs_socket hangs with http://rainemu.swishparty.co.uk/git/zfs/

[Tino]

Using http://rainemu.swishparty.co.uk/git/zfs/ master (with some
changes to let compile it, see below) I get a hang of following
command:

while echo -n .; do zpool status >/dev/null; done

This runs a time successfully (a few seconds) and then it hangs (no
more dots). Any command talking to /var/run/zfs/zfs_socket then does
no more work, too, so the socket is dead. Note that my machine is SMP
(two cores). Also note that the zfs filesystem stays online and
accessible, so only the socket serving routine is affected.

Can somebody confirm this or is my problem local only? Thanks.

Diff attached. Notes to the diff:

I changed ZFS_IOC because the numbering of commands might differ
between ZFS versions. So if you try out several zfs versions in
parallel is too dangerous to perhaps invoke the wrong zpool command to
a different running zfs-fuse daemon. I recommend others trying out
different zfs versions in parallel to give each version tested an
unique ZFS_IOC like me, to prevent a wrong zpool to be able to talk to
zfs-fuse successfully.

Also ENOATTR is only present in xfs; Grepping the Internet I got the
impression that the correct errno should be EDOM but I really do not
know. And attr/xattr.h is at sys/xattr.h at my side.

zoo:~/src/zfs/bleedingedge/zfs/src# cat /etc/debian_version
5.0.4

zoo:~/src/zfs/bleedingedge/zfs/src# uname -a
Linux zoo 2.6.26-2-686 #1 SMP Wed Nov 4 20:45:37 UTC 2009 i686 GNU/
Linux

zoo:~/src/zfs/bleedingedge/zfs/src# git diff master
diff --git a/.gitignore b/.gitignore
index ddea9d6..3706540 100644
--- a/.gitignore
+++ b/.gitignore
@@ -4,6 +4,8 @@
*.swp
*.orig
*.sconsign.dblite
+*.lo
+*.la

src/zfs-fuse/zfs-fuse
src/cmd/zdb/zdb
@@ -11,3 +13,18 @@ src/cmd/zfs/zfs
src/cmd/zpool/zpool
src/cmd/ztest/ztest

+/src/cmd/zstreamdump/zstreamdump
+/src/lib/libumem/.deps/
+/src/lib/libumem/.libs/
+/src/lib/libumem/Doxyfile
+/src/lib/libumem/Makefile
+/src/lib/libumem/config.h
+/src/lib/libumem/config.log
+/src/lib/libumem/config.status
+/src/lib/libumem/libtool
+/src/lib/libumem/stamp-h1
+/src/lib/libumem/umem.spec
+/src/lib/libumem/umem_test
+/src/lib/libumem/umem_test2
+/src/lib/libumem/umem_test3
+
diff --git a/src/lib/libzfscommon/include/sys/fs/zfs.h b/src/lib/
libzfscommon/include/sys/fs/zfs.h
index 82cc2c9..08d62d4 100644
--- a/src/lib/libzfscommon/include/sys/fs/zfs.h
+++ b/src/lib/libzfscommon/include/sys/fs/zfs.h
@@ -666,7 +666,7 @@ typedef struct ddt_histogram {
/*
* /dev/zfs ioctl numbers.
*/
-#define ZFS_IOC ('Z' << 8)
+#define ZFS_IOC (('Z'+2) << 8)

typedef enum zfs_ioc {
ZFS_IOC_POOL_CREATE = ZFS_IOC,
diff --git a/src/zfs-fuse/zfs_operations.c b/src/zfs-fuse/
zfs_operations.c
index 260adbb..3448181 100644
--- a/src/zfs-fuse/zfs_operations.c
+++ b/src/zfs-fuse/zfs_operations.c
@@ -34,7 +34,7 @@
#include <sys/zfs_vfsops.h>
#include <sys/zfs_znode.h>
#include <sys/mode.h>
-#include <attr/xattr.h>
+#include <sys/xattr.h>
#include <sys/fcntl.h>

#include <string.h>
@@ -374,7 +374,7 @@ static void zfsfuse_getxattr(fuse_req_t req,
fuse_ino_t ino, const char *name,
vnode_t *new_vp = NULL;
error = VOP_LOOKUP(vp, (char *) name, &new_vp, NULL, 0, NULL,
&cred, NULL, NULL, NULL);
if (error) {
- error = ENOATTR;
+ error = EDOM;
goto out;
}
VN_RELE(vp);
@@ -445,7 +445,7 @@ out:
VN_RELE(dvp);
ZFS_EXIT(zfsvfs);
if (error == ENOENT)
- error = ENOATTR;
+ error = EDOM;
fuse_reply_err(req,error);
}

--
To post to this group, send email to zfs-...@googlegroups.com
To visit our Web site, click on http://zfs-fuse.net/

[Emmanuel Anne]

1. I tried your command, and stopped after I got a terminal full of
dots, so I guess it works here. I'll try later with my smp laptop but
I don't think it will make any difference.
2. the zfs/zpool commands go with a precise version of zfs-fuse,
changing the ioc numbers just to make some tests seems crazy. If you
want to do some tests, use the patch_debug (I posted it some time ago
to the list, might eventually put it in contrib dir, I'll attach it
once more to this mail, it just changes the location of the socket +
the lock file to allow to run 2 instances of zfs-fuse together, one
for testing/debuging + the main one, very useful if like me you have
your home on zfs and still want to do some debuging stuff).
3. Your patch is mostly unreadable because it's merged into the
message instead of being a real attachement, but anyway, xattr.h comes
from libattr1-dev you don't seem to have looked for it for very long.

2010/5/14 Tino <valenti...@googlemail.com>:

--
zfs-fuse git repository :
http://rainemu.swishparty.co.uk/cgi-bin/gitweb.cgi?p=zfs;a=summary

[sgheeren]

On 05/14/2010 11:23 AM, Emmanuel Anne wrote:
> 1. I tried your command, and stopped after I got a terminal full of
> dots, so I guess it works here. I'll try later with my smp laptop but
> I don't think it will make any difference.
>

Same here

> 2. the zfs/zpool commands go with a precise version of zfs-fuse,
> changing the ioc numbers just to make some tests seems crazy.

+1 Exactly: running multiple zfs-fuse in parallel is simply not
supported. If you use a hacked version to allow this, any bugs reported
against that version are automatically tainted as inaccurate
information... Please reproduce with a vanilla version and resubmit...

> If you
> want to do some tests, use the patch_debug (I posted it some time ago
> to the list, might eventually put it in contrib dir, I'll attach it
> once more to this mail, it just changes the location of the socket +
> the lock file to allow to run 2 instances of zfs-fuse together, one
> for testing/debuging + the main one, very useful if like me you have
> your home on zfs and still want to do some debuging stuff).
> 3. Your patch is mostly unreadable because it's merged into the
> message instead of being a real attachement, but anyway, xattr.h comes
> from libattr1-dev you don't seem to have looked for it for very long.
>

But we could (presumably) consider the idea that the package hath
changed in that specific version of debian. It would be most helpful to
list the output of dpkg --status for the packages mentioned on the
zfs-fuse.net home page in that respect.

[Emmanuel Anne]

Well by the way, since xattrs are currently disabled it's not a big issue...
+ if I remember correctly, I use only ENOATTR from this file so it
could even be hard defined somewhere...
But really installing this should not be an issue, looking for xattr.h
in packages.debian.org would find this package very quickly.

2010/5/14 sgheeren <sghe...@hotmail.com>:

--
zfs-fuse git repository :
http://rainemu.swishparty.co.uk/cgi-bin/gitweb.cgi?p=zfs;a=summary

[Tino]

Thank you for all notes and replies. Sorry that it took some time to
cross check.

Even in a clean room environment I can observe the hang.

The symptom prints a line in /var/log/daemon.log saying
"zfs-fuse: WARNING: Error creating ioctl thread."
(typed in, I have no copy&paste in the VM currently.)

To check this again I created a VM (vmware-server) from scratch with
following parameters:

- Linux 32 bit
- 1024 MB RAM
- 2 SMP processors (2.67 GHz btw.)
- Debian Lenny version 5.0.4 minimal (nothing, not even SSHd, only
console)

Then I downloaded following script and ran it as root (sorry, I use
Google Groups, no way to attach something here properly):

http://hydra.geht.net/zfs-build.sh

This does everything, it updates Linux, installs the Dev environment,
checks out zfs-fuse, compiles it and runs the test.

This test fails. It still hangs after a while. It took half a screen
full of dots to hang at my side, though.

So I am really out of clues.

Key properties which might be needed to see this problem, too:

- 32 bit
- Debian Lenny
- Enough RAM
- Fast enough processor
- SMP

But I might be wrong as I did not do many tests with different VM
parameters yet.

Note that it looks like the bigger the pool size the faster you find
the problem. However it even shows up with no pool at all.

-Tino

On 14 Mai, 14:11, Emmanuel Anne <emmanuel.a...@gmail.com> wrote:
> Well by the way, since xattrs are currently disabled it's not a big issue...
> + if I remember correctly, I use only ENOATTR from this file so it
> could even be hard defined somewhere...
> But really installing this should not be an issue, looking for xattr.h
> in packages.debian.org would find this package very quickly.
>

> 2010/5/14 sgheeren <sghee...@hotmail.com>:

> >> 2010/5/14 Tino <valentin.hil...@googlemail.com>:
>
> >>> Usinghttp://rainemu.swishparty.co.uk/git/zfs/master (with some

> >>> To visit our Web site, click onhttp://zfs-fuse.net/

>
> > --
> > To post to this group, send email to zfs-...@googlegroups.com

> > To visit our Web site, click onhttp://zfs-fuse.net/

>
> --
> zfs-fuse git repository :http://rainemu.swishparty.co.uk/cgi-bin/gitweb.cgi?p=zfs;a=summary
>
> --
> To post to this group, send email to zfs-...@googlegroups.com

> To visit our Web site, click onhttp://zfs-fuse.net/

[Emmanuel Anne]

Limitation of your virtual machine related to threads maybe ?
See the man page of pthread_create : they say the only cases where it can fails is either because of a lack of memory or because you have more than the allowed maximum number of threads (which is high enough).
Anyway what I could do is add something to handle the error case when the thread creation fails, but I can't make miracles if your virtual machine does not allow the thread creation it will stop here even if it does not crash...

2010/5/15 Tino <valenti...@googlemail.com>

[Emmanuel Anne]

Ok, just committed a change to better handle this case of error,thanks for the log message, now it will try to correctly return the error code to the caller, which should fix the crash (but it won't fix your problem, it will just avoid the crash, not the error).

2010/5/15 Emmanuel Anne <emmanu...@gmail.com>

[Tino]

Sorry to stress again, but I think I found a first workaround of this
problem.
I am not completely convinced that my solution is correct, though.

After observing that pthread_create() returned ENOMEM (undoc.) at my
side, Google spew out following URL
> http://www.parkes.atnf.csiro.au/observing/documentation/computing_notes/linux_threads.html

The workaround is to create the thread in detached state, as it looks
like it is never joined. This prevents that the created threads
consume internal memory in the pthread library, thus preventing the
ENOMEM.

As I cannot attach anything in Google Groups, the diff is at
http://hydra.geht.net/zfs.1.patch.txt

After applying this patch I was unable to observe the problem again.

Thanks.
-Tino
PS: uuencoded patch (HTH):

> > >>> Usinghttp://rainemu.swishparty.co.uk/git/zfs/master(with some

[sgheeren]

Confirmed

I filed a bug for this: http://zfs-fuse.net/issues/40

I suppose it was introduced by 8b6b9db619d8944d69760b2efd71faa34111e85b which creates new threads but possibly does not clean them up correctly. I'll use your patch to create detached trheads, as I feel it might be just the fix desired.

Please subscribe to the bug if you continue interest in this issue, I will not always corsspost (time limited)

[sgheeren]

Ok, the stack-size patch is the culprit. The default is clearly not good for some systems.

See bug for details: http://zfs-fuse.net/issues/40

I propose to
(a) check that default behaviour is really unlimited ?
(b) otherwise change default to 32m. 8m is not working on this type of test system.

$0.04

[sgheeren]

On 05/15/2010 12:07 PM, sgheeren wrote:
> Ok, the stack-size patch is the culprit. The default is clearly not good
> for some systems.
>
> See bug for details: http://zfs-fuse.net/issues/40
>
> I propose to
> (a) check that default behaviour is really unlimited ?
> (b) otherwise change default to 32m. 8m is not working on this type of
> test system.
>

Oh and
(c) see if http://hydra.geht.net/zfs.1.patch.txt is a good idea

diff --git a/src/zfs-fuse/cmd_listener.c b/src/zfs-fuse/cmd_listener.c
index d0f0999..b7abbe4 100644
--- a/src/zfs-fuse/cmd_listener.c
+++ b/src/zfs-fuse/cmd_listener.c
@@ -124,6 +124,7 @@ static void start_ioctl_thread(int sock, zfsfuse_cmd_t *cmd) {
pthread_attr_init(&attr);
if (stack_size)
pthread_attr_setstacksize(&attr,stack_size);
+ pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
if(pthread_create(&ioctl_thread, &attr, cmd_ioctl_thread, (void *) &init) != 0)
cmn_err(CE_WARN, "Error creating ioctl thread.");

[Emmanuel Anne]

LOL !!!
When I think about all the beging to get the default stack size to finally get this result !!!
I knew 8M of stack with 150 stacks, even unmapped could not possibly do some good.

But on the other hand, I admit I was not confident with these detached threads either, I knew there was probably a better way to handle them, but didn't know which one.

Quite funny anyway, glad to have kept my 32k stack size !
Ok, I'll look at this patch and I will probably merge it then...

2010/5/15 sgheeren <sghe...@hotmail.com>

[Emmanuel Anne]

Yeah ok, patch committed and pushed.
Quite funny that you have to explicitely create these threads as detached, there should be something to automatically free them if no process waits to join them... anyway !

2010/5/15 sgheeren <sghe...@hotmail.com>

[sgheeren]

On 05/15/2010 12:16 PM, Emmanuel Anne wrote:
> Yeah ok, patch committed and pushed.
> Quite funny that you have to explicitely create these threads as detached,
> there should be something to automatically free them if no process waits to
> join them... anyway !
>

Ok, I've posted more embarrassing results from my own server

Will retest with your latest push.

Off for the swimming pool now :)

[sgheeren]

sooo.... "it seems to help with memory consumption, particularly in
virtual machines"

this info is based on...?

I'd very much like to know what other info you have received to draw
this conclusion, since I don't understand this piece

On 05/15/2010 12:16 PM, Emmanuel Anne wrote:

[sgheeren]

To be fair, it wasn't really introduced by any of this. It was really a side-effect of having many more threads (since the multiple threads of ioctl handling). Read my later response http://zfs-fuse.net/issues/40/6 for 'proof' and figures.

It turns out no mount of stack tweaking was going to fix any of this. It was simply raising the ceiling on which zfs-fuse would choke.

Anyways, testing is now fixed for this, thanks Tino and Emmanuel

 would find this package very quickly.

2010/5/14 sgheeren <sghee...@hotmail.com> <sghee...@hotmail.com>:






 On 05/14/2010 11:23 AM, Emmanuel Anne wrote:



 1. I tried your command, and stopped after I got a terminal full of
dots, so I guess it works here. I'll try later with my smp laptop but
I don't think it will make any difference.




       Same here



 2. the zfs/zpool commands go with a precise version of zfs-fuse,
changing the ioc numbers just to make some tests seems crazy.



 +1 Exactly: running multiple zfs-fuse in parallel is simply not
supported. If you use a hacked version to allow this, any bugs reported
against that version are automatically tainted as inaccurate
information... Please reproduce with a vanilla version and resubmit...



 If you
want to do some tests, use the patch_debug (I posted it some time ago
to the list, might eventually put it in contrib dir, I'll attach it
once more to this mail, it just changes the location of the socket +
the lock file to allow to run 2 instances of zfs-fuse together, one
for testing/debuging + the main one, very useful if like me you have
your home on zfs and still want to do some debuging stuff).
3. Your patch is mostly unreadable because it's merged into the
message instead of being a real attachement, but anyway, xattr.h comes
from libattr1-dev you don't seem to have looked for it for very long.




       But we could (presumably) consider the idea that the package hath
changed in that specific version of debian. It would be most helpful to
list the output of dpkg --status for the packages mentioned on thezfs-fuse.net home page in that respect.



 2010/5/14 Tino <valentin.hil...@googlemail.com> <valentin.hil...@googlemail.com>:




        Usinghttp://rainemu.swishparty.co.uk/git/zfs/master (with some
changes to let compile it, see below) I get a hang of following
command:




        while echo -n .; do zpool status >/dev/null; done




        This runs a time successfully (a few seconds) and then it hangs (no
more dots).  Any command talking to /var/run/zfs/zfs_socket then does
no more work, too, so the socket is dead.  Note that my machine is SMP
(two cores).  Also note that the zfs filesystem stays online and
accessible, so only the socket serving routine is affected.




        Can somebody confirm this or is my problem local only?  Thanks.




        Diff attached.  Notes to the diff:




        I changed ZFS_IOC because the numbering of commands might differ
between ZFS versions.  So if you try out several zfs versions in
parallel is too dangerous to perhaps invoke the wrong zpool command to
a different running zfs-fuse daemon.  I recommend others trying out
different zfs versions in parallel to give each version tested an
unique ZFS_IOC like me, to prevent a wrong zpool to be able to talk to
zfs-fuse successfully.




        Also ENOATTR is only present in xfs; Grepping the Internet I got the
impression that the correct errno should be EDOM but I really do not
know.  And attr/xattr.h is at sys/xattr.h at my side.




        zoo:~/src/zfs/bleedingedge/zfs/src# cat /etc/debian_version
5.0.4




        zoo:~/src/zfs/bleedingedge/zfs/src# uname -a
Linux zoo 2.6.26-2-686 #1 SMP Wed Nov 4 20:45:37 UTC 2009 i686 GNU/
Linux




        zoo:~/src/zfs/bleedingedge/zfs/src# git diff master
diff --git a/.gitignore b/.gitignore
index ddea9d6..3706540 100644
--- a/.gitignore
+++ b/.gitignore
@@ -4,6 +4,8 @@
 *.swp
 *.orig
 *.sconsign.dblite
+*.lo
+*.la




         src/zfs-fuse/zfs-fuse
 src/cmd/zdb/zdb
@@ -11,3 +13,18 @@ src/cmd/zfs/zfs
 src/cmd/zpool/zpool
 src/cmd/ztest/ztest




        +/src/cmd/zstreamdump/zstreamdump
+/src/lib/libumem/.deps/
+/src/lib/libumem/.libs/
+/src/lib/libumem/Doxyfile
+/src/lib/libumem/Makefile
+/src/lib/libumem/config.h
+/src/lib/libumem/config.log
+/src/lib/libumem/config.status
+/src/lib/libumem/libtool
+/src/lib/libumem/stamp-h1
+/src/lib/libumem/umem.spec
+/src/lib/libumem/umem_test
+/src/lib/libumem/umem_test2
+/src/lib/libumem/umem_test3
+
diff --git a/src/lib/libzfscommon/include/sys/fs/zfs.h b/src/lib/
libzfscommon/include/sys/fs/zfs.h
index 82cc2c9..08d62d4 100644
--- a/src/lib/libzfscommon/include/sys/fs/zfs.h
+++ b/src/lib/libzfscommon/include/sys/fs/zfs.h
@@ -666,7 +666,7 @@ typedef struct ddt_histogram {
 /*
 * /dev/zfs ioctl numbers.
 */
-#define        ZFS_IOC         ('Z' << 8)
+#define        ZFS_IOC         (('Z'+2) << 8)




         typedef enum zfs_ioc {
       ZFS_IOC_POOL_CREATE = ZFS_IOC,
diff --git a/src/zfs-fuse/zfs_operations.c b/src/zfs-fuse/
zfs_operations.c
index 260adbb..3448181 100644
--- a/src/zfs-fuse/zfs_operations.c
+++ b/src/zfs-fuse/zfs_operations.c
@@ -34,7 +34,7 @@
 #include <sys/zfs_vfsops.h>
 #include <sys/zfs_znode.h>
 #include <sys/mode.h>
-#include <attr/xattr.h>
+#include <sys/xattr.h>
 #include <sys/fcntl.h>




         #include <string.h>
@@ -374,7 +374,7 @@ static void zfsfuse_getxattr(fuse_req_t req,
fuse_ino_t ino, const char *name,
    vnode_t *new_vp = NULL;
    error = VOP_LOOKUP(vp, (char *) name, &new_vp, NULL, 0, NULL,
&cred, NULL, NULL, NULL);
    if (error) {
-       error = ENOATTR;
+       error = EDOM;
       goto out;
    }
    VN_RELE(vp);
@@ -445,7 +445,7 @@ out:
    VN_RELE(dvp);
    ZFS_EXIT(zfsvfs);
       if (error == ENOENT)
-               error = ENOATTR;
+               error = EDOM;
    fuse_reply_err(req,error);
 }




        --
To post to this group, send email to zfs-...@googlegroups.com

[Emmanuel Anne]

lower memory consumption is probably not the right way to say it, it's a fix for a memory leak and it was my fault, but it's normal I am still a beginner with pthreads, so thanks Tino for your search !

2010/5/15 sgheeren <sghe...@hotmail.com>