* Semantics of fuse_notify_delete()
@ 2023-07-26 18:08 Nikolaus Rath
2023-07-27 8:04 ` Miklos Szeredi
0 siblings, 1 reply; 15+ messages in thread
From: Nikolaus Rath @ 2023-07-26 18:08 UTC (permalink / raw)
To: fuse-devel, Linux FS Devel, miklos
Hello,
It seems to me that fuse_notify_delete
(https://elixir.bootlin.com/linux/v6.1/source/fs/fuse/dev.c#L1512) fails
with ENOTEMPTY if there is a pending FORGET request for a directory
entry within. Is that correct?
If so, what is the expected behavior for a filesystem that has just
deleted the entire tree and wants to inform the kernel of that fact?
Calling fuse_notify_delete() synchronously seems very prone to
deadlocks, and I'm not sure that the call would actually block until
FORGET has been processed.
Is the filesystem expected to wait for FORGET before it issues
fuse_notify_delete()? Or should it actually wait with the (physical)
removal of the parent directory until all child entries have zero lookup
count?
In the former case, why is this needed? In the latter case, how are
network filesystems supposed to deal with this?
Best,
-Nikolaus
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Semantics of fuse_notify_delete()
2023-07-26 18:08 Semantics of fuse_notify_delete() Nikolaus Rath
@ 2023-07-27 8:04 ` Miklos Szeredi
2023-07-27 11:37 ` [fuse-devel] " Nikolaus Rath
0 siblings, 1 reply; 15+ messages in thread
From: Miklos Szeredi @ 2023-07-27 8:04 UTC (permalink / raw)
To: fuse-devel, Linux FS Devel, miklos
On Wed, 26 Jul 2023 at 20:09, Nikolaus Rath <Nikolaus@rath.org> wrote:
>
> Hello,
>
> It seems to me that fuse_notify_delete
> (https://elixir.bootlin.com/linux/v6.1/source/fs/fuse/dev.c#L1512) fails
> with ENOTEMPTY if there is a pending FORGET request for a directory
> entry within. Is that correct?
It's bug if it does that.
The code related to NOTIFY_DELETE in fuse_reverse_inval_entry() seems
historic. It's supposed to be careful about mountpoints and
referenced dentries, but d_invalidate() should have already gotten all
that out of the way and left an unhashed dentry without any submounts
or children. The checks just seem redundant, but not harmful.
If you are managing to trigger the ENOTEMPTY case, then something
strange is going on, and we need to investigate.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [fuse-devel] Semantics of fuse_notify_delete()
2023-07-27 8:04 ` Miklos Szeredi
@ 2023-07-27 11:37 ` Nikolaus Rath
2023-07-28 8:45 ` Nikolaus Rath
2023-08-02 13:18 ` Miklos Szeredi
0 siblings, 2 replies; 15+ messages in thread
From: Nikolaus Rath @ 2023-07-27 11:37 UTC (permalink / raw)
To: Miklos Szeredi via fuse-devel; +Cc: Linux FS Devel, miklos, Miklos Szeredi
On Jul 27 2023, Miklos Szeredi via fuse-devel <fuse-devel@lists.sourceforge.net> wrote:
> On Wed, 26 Jul 2023 at 20:09, Nikolaus Rath <Nikolaus@rath.org> wrote:
>>
>> Hello,
>>
>> It seems to me that fuse_notify_delete
>> (https://elixir.bootlin.com/linux/v6.1/source/fs/fuse/dev.c#L1512) fails
>> with ENOTEMPTY if there is a pending FORGET request for a directory
>> entry within. Is that correct?
>
> It's bug if it does that.
>
> The code related to NOTIFY_DELETE in fuse_reverse_inval_entry() seems
> historic. It's supposed to be careful about mountpoints and
> referenced dentries, but d_invalidate() should have already gotten all
> that out of the way and left an unhashed dentry without any submounts
> or children. The checks just seem redundant, but not harmful.
>
> If you are managing to trigger the ENOTEMPTY case, then something
> strange is going on, and we need to investigate.
I can trigger this reliable on kernel 6.1.0-10-amd64 (Debian stable)
with this sequence of operations:
$ mkdir test
$ echo foo > test/bar
$ Trigger removal of test/bar and then test within the filesystem (not
through unlink()/rmdir() but out-of-band)
What can I do to help with the investigation?
Best,
-Nikolaus
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [fuse-devel] Semantics of fuse_notify_delete()
2023-07-27 11:37 ` [fuse-devel] " Nikolaus Rath
@ 2023-07-28 8:45 ` Nikolaus Rath
2023-07-28 8:52 ` Miklos Szeredi
2023-08-02 13:18 ` Miklos Szeredi
1 sibling, 1 reply; 15+ messages in thread
From: Nikolaus Rath @ 2023-07-28 8:45 UTC (permalink / raw)
To: Miklos Szeredi via fuse-devel, Linux FS Devel, miklos,
Miklos Szeredi
On Jul 27 2023, Nikolaus Rath <Nikolaus@rath.org> wrote:
> On Jul 27 2023, Miklos Szeredi via fuse-devel <fuse-devel@lists.sourceforge.net> wrote:
>> On Wed, 26 Jul 2023 at 20:09, Nikolaus Rath <Nikolaus@rath.org> wrote:
>>>
>>> Hello,
>>>
>>> It seems to me that fuse_notify_delete
>>> (https://elixir.bootlin.com/linux/v6.1/source/fs/fuse/dev.c#L1512) fails
>>> with ENOTEMPTY if there is a pending FORGET request for a directory
>>> entry within. Is that correct?
>>
>> It's bug if it does that.
>>
>> The code related to NOTIFY_DELETE in fuse_reverse_inval_entry() seems
>> historic. It's supposed to be careful about mountpoints and
>> referenced dentries, but d_invalidate() should have already gotten all
>> that out of the way and left an unhashed dentry without any submounts
>> or children. The checks just seem redundant, but not harmful.
>>
>> If you are managing to trigger the ENOTEMPTY case, then something
>> strange is going on, and we need to investigate.
>
> I can trigger this reliable on kernel 6.1.0-10-amd64 (Debian stable)
> with this sequence of operations:
>
> $ mkdir test
> $ echo foo > test/bar
> $ Trigger removal of test/bar and then test within the filesystem (not
> through unlink()/rmdir() but out-of-band)
>
>
> What can I do to help with the investigation?
I've pushed an instrumented snapshot to
https://github.com/s3ql/s3ql/tree/notify_delete_bug. For me, this
reliably reproduces the problem:
$ python3 setup.py build_cython build_ext --inplace
$ md bucket
$ bin/mkfs.s3ql --plain local://bucket
[...]
$ bin/mount.s3ql --fg local://bucket mnt &
[...]
$ md mnt/test; echo foo > mnt/test/bar
$ bin/s3qlrm mnt/test
fuse: writing device: Directory not empty
ERROR: Failed to submit invalidate_entry request for parent inode 1, name b'test'
Traceback (most recent call last):
File "src/internal.pxi", line 125, in pyfuse3._notify_loop
File "src/pyfuse3.pyx", line 915, in pyfuse3.invalidate_entry
OSError: [Errno 39] fuse_lowlevel_notify_delete returned: Directory not
empty
I've looked into reproducing this with e.g. example/passthrough_ll.c,
but it's non-trivial because of the need to run notify_delete in a
separate thread and giving it all the right arguments. I can look into
it more if that's needed though.
Best,
-Nikolaus
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [fuse-devel] Semantics of fuse_notify_delete()
2023-07-28 8:45 ` Nikolaus Rath
@ 2023-07-28 8:52 ` Miklos Szeredi
2023-07-31 14:12 ` Miklos Szeredi
0 siblings, 1 reply; 15+ messages in thread
From: Miklos Szeredi @ 2023-07-28 8:52 UTC (permalink / raw)
To: Miklos Szeredi via fuse-devel, Linux FS Devel, miklos,
Miklos Szeredi
On Fri, 28 Jul 2023 at 10:45, Nikolaus Rath <Nikolaus@rath.org> wrote:
> I've pushed an instrumented snapshot to
> https://github.com/s3ql/s3ql/tree/notify_delete_bug. For me, this
> reliably reproduces the problem:
>
> $ python3 setup.py build_cython build_ext --inplace
> $ md bucket
> $ bin/mkfs.s3ql --plain local://bucket
> [...]
> $ bin/mount.s3ql --fg local://bucket mnt &
> [...]
> $ md mnt/test; echo foo > mnt/test/bar
> $ bin/s3qlrm mnt/test
> fuse: writing device: Directory not empty
> ERROR: Failed to submit invalidate_entry request for parent inode 1, name b'test'
> Traceback (most recent call last):
> File "src/internal.pxi", line 125, in pyfuse3._notify_loop
> File "src/pyfuse3.pyx", line 915, in pyfuse3.invalidate_entry
> OSError: [Errno 39] fuse_lowlevel_notify_delete returned: Directory not
> empty
Thanks, will try this.
Miklos
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [fuse-devel] Semantics of fuse_notify_delete()
2023-07-28 8:52 ` Miklos Szeredi
@ 2023-07-31 14:12 ` Miklos Szeredi
2023-08-01 10:53 ` Nikolaus Rath
0 siblings, 1 reply; 15+ messages in thread
From: Miklos Szeredi @ 2023-07-31 14:12 UTC (permalink / raw)
To: Miklos Szeredi via fuse-devel, Linux FS Devel, miklos,
Miklos Szeredi
On Fri, 28 Jul 2023 at 10:52, Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Fri, 28 Jul 2023 at 10:45, Nikolaus Rath <Nikolaus@rath.org> wrote:
>
> > I've pushed an instrumented snapshot to
> > https://github.com/s3ql/s3ql/tree/notify_delete_bug. For me, this
> > reliably reproduces the problem:
> >
> > $ python3 setup.py build_cython build_ext --inplace
> > $ md bucket
> > $ bin/mkfs.s3ql --plain local://bucket
> > [...]
> > $ bin/mount.s3ql --fg local://bucket mnt &
> > [...]
> > $ md mnt/test; echo foo > mnt/test/bar
> > $ bin/s3qlrm mnt/test
> > fuse: writing device: Directory not empty
> > ERROR: Failed to submit invalidate_entry request for parent inode 1, name b'test'
> > Traceback (most recent call last):
> > File "src/internal.pxi", line 125, in pyfuse3._notify_loop
> > File "src/pyfuse3.pyx", line 915, in pyfuse3.invalidate_entry
> > OSError: [Errno 39] fuse_lowlevel_notify_delete returned: Directory not
> > empty
I get this:
root@kvm:~/s3ql# bin/s3qlrm mnt/test
WARNING: Received unknown command via control inode
ERROR: Uncaught top-level exception:
Traceback (most recent call last):
File "/root/s3ql/bin/s3qlrm", line 21, in <module>
s3ql.remove.main(sys.argv[1:])
File "/root/s3ql/src/s3ql/remove.py", line 74, in main
pyfuse3.setxattr(ctrlfile, 'rmtree', cmd)
File "src/pyfuse3.pyx", line 629, in pyfuse3.setxattr
OSError: [Errno 22] Invalid argument: 'mnt/test/.__s3ql__ctrl__'
All packages are from debian/testing, except python3-dugong, which is
from bullseye (oldstable), becase apparently it was removed from the
recent release.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [fuse-devel] Semantics of fuse_notify_delete()
2023-07-31 14:12 ` Miklos Szeredi
@ 2023-08-01 10:53 ` Nikolaus Rath
2023-08-01 12:53 ` Miklos Szeredi
0 siblings, 1 reply; 15+ messages in thread
From: Nikolaus Rath @ 2023-08-01 10:53 UTC (permalink / raw)
To: Miklos Szeredi via fuse-devel; +Cc: Linux FS Devel, miklos, Miklos Szeredi
On Jul 31 2023, Miklos Szeredi via fuse-devel <fuse-devel@lists.sourceforge.net> wrote:
> On Fri, 28 Jul 2023 at 10:52, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>
>> On Fri, 28 Jul 2023 at 10:45, Nikolaus Rath <Nikolaus@rath.org> wrote:
>>
>> > I've pushed an instrumented snapshot to
>> > https://github.com/s3ql/s3ql/tree/notify_delete_bug. For me, this
>> > reliably reproduces the problem:
>> >
>> > $ python3 setup.py build_cython build_ext --inplace
>> > $ md bucket
>> > $ bin/mkfs.s3ql --plain local://bucket
>> > [...]
>> > $ bin/mount.s3ql --fg local://bucket mnt &
>> > [...]
>> > $ md mnt/test; echo foo > mnt/test/bar
>> > $ bin/s3qlrm mnt/test
>> > fuse: writing device: Directory not empty
>> > ERROR: Failed to submit invalidate_entry request for parent inode 1, name b'test'
>> > Traceback (most recent call last):
>> > File "src/internal.pxi", line 125, in pyfuse3._notify_loop
>> > File "src/pyfuse3.pyx", line 915, in pyfuse3.invalidate_entry
>> > OSError: [Errno 39] fuse_lowlevel_notify_delete returned: Directory not
>> > empty
>
> I get this:
>
> root@kvm:~/s3ql# bin/s3qlrm mnt/test
> WARNING: Received unknown command via control inode
> ERROR: Uncaught top-level exception:
> Traceback (most recent call last):
> File "/root/s3ql/bin/s3qlrm", line 21, in <module>
> s3ql.remove.main(sys.argv[1:])
> File "/root/s3ql/src/s3ql/remove.py", line 74, in main
> pyfuse3.setxattr(ctrlfile, 'rmtree', cmd)
> File "src/pyfuse3.pyx", line 629, in pyfuse3.setxattr
> OSError: [Errno 22] Invalid argument: 'mnt/test/.__s3ql__ctrl__'
>
> All packages are from debian/testing, except python3-dugong, which is
> from bullseye (oldstable), becase apparently it was removed from the
> recent release.
This sounds like you're using s3qlrm from one version of S3QL, and
mount.s3ql from a different one.
What do you mean with "all packages are from debian/testing"? If this
includes S3QL, then it won't work: you need the one from the Git
repository above because it's instrumented to reproduce the problem
reliably.
If you want to keep the Python packages separate, the best way is to use
a virtual environment:
# mkdir ~/s3ql-python-env
# python3 -m venv --system-side-packages ~/s3ql-python-env
# ~/s3ql-python-env/bin/python -m pip install --upgrade cryptography defusedxml apsw trio pyfuse3 dugong pytest requests cython
# ~/s3ql-python-env/bin/python setup.py build_cython build_ext --inplace
# ~/s3ql-python-env/bin/python bin/mount.s3ql [...]
# ~/s3ql-python-env/bin/python bin/s3qlrm [...]
Best,
-Nikolaus
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [fuse-devel] Semantics of fuse_notify_delete()
2023-08-01 10:53 ` Nikolaus Rath
@ 2023-08-01 12:53 ` Miklos Szeredi
2023-08-01 14:40 ` Nikolaus Rath
0 siblings, 1 reply; 15+ messages in thread
From: Miklos Szeredi @ 2023-08-01 12:53 UTC (permalink / raw)
To: Miklos Szeredi via fuse-devel; +Cc: Linux FS Devel, miklos
On Tue, 1 Aug 2023 at 12:54, Nikolaus Rath <Nikolaus@rath.org> wrote:
> This sounds like you're using s3qlrm from one version of S3QL, and
> mount.s3ql from a different one.
Indeed, I forgot to checkout the debug branch.
> If you want to keep the Python packages separate, the best way is to use
> a virtual environment:
>
> # mkdir ~/s3ql-python-env
> # python3 -m venv --system-side-packages ~/s3ql-python-env
> # ~/s3ql-python-env/bin/python -m pip install --upgrade cryptography defusedxml apsw trio pyfuse3 dugong pytest requests cython
> # ~/s3ql-python-env/bin/python setup.py build_cython build_ext --inplace
> # ~/s3ql-python-env/bin/python bin/mount.s3ql [...]
> # ~/s3ql-python-env/bin/python bin/s3qlrm [...]
Here's one with the virtual env and the correct head:
root@kvm:~/s3ql# git log -1 --pretty="%h %s"
3d35f18543d9 Reproducer for notify_delete issue. To confirm:
root@kvm:~/s3ql# ~/s3ql-python-env/bin/python bin/s3qlrm mnt/test
WARNING: Received unknown command via control inode
ERROR: Uncaught top-level exception:
Traceback (most recent call last):
File "/root/s3ql/bin/s3qlrm", line 21, in <module>
s3ql.remove.main(sys.argv[1:])
File "/root/s3ql/src/s3ql/remove.py", line 72, in main
pyfuse3.setxattr(ctrlfile, 'rmtree', cmd)
File "src/pyfuse3.pyx", line 629, in pyfuse3.setxattr
OSError: [Errno 22] Invalid argument: 'mnt/test/.__s3ql__ctrl__'
Thanks,
Miklos
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [fuse-devel] Semantics of fuse_notify_delete()
2023-08-01 12:53 ` Miklos Szeredi
@ 2023-08-01 14:40 ` Nikolaus Rath
2023-08-01 14:48 ` Miklos Szeredi
0 siblings, 1 reply; 15+ messages in thread
From: Nikolaus Rath @ 2023-08-01 14:40 UTC (permalink / raw)
To: Miklos Szeredi, Martin Kaspar via fuse-devel
Cc: Linux FS Devel, Miklos Szeredi
On Tue, 1 Aug 2023, at 13:53, Miklos Szeredi via fuse-devel wrote:
> Here's one with the virtual env and the correct head:
>
> root@kvm:~/s3ql# git log -1 --pretty="%h %s"
> 3d35f18543d9 Reproducer for notify_delete issue. To confirm:
> root@kvm:~/s3ql# ~/s3ql-python-env/bin/python bin/s3qlrm mnt/test
> WARNING: Received unknown command via control inode
> ERROR: Uncaught top-level exception:
> Traceback (most recent call last):
> File "/root/s3ql/bin/s3qlrm", line 21, in <module>
> s3ql.remove.main(sys.argv[1:])
> File "/root/s3ql/src/s3ql/remove.py", line 72, in main
> pyfuse3.setxattr(ctrlfile, 'rmtree', cmd)
> File "src/pyfuse3.pyx", line 629, in pyfuse3.setxattr
> OSError: [Errno 22] Invalid argument: 'mnt/test/.__s3ql__ctrl__'
This is odd. I have never heard of anyone having this problem before and it also works fine in the CI.
I apologize that this is taking so much of your time.
I have changed the code a bit to print out what exactly it is receiving: https://github.com/s3ql/s3ql/commit/eb31f7bff4bd985d68fa20c793c2f2edf5db61a5
Would you mind updating your branch and trying again? (You'll need to fetch and reset, since I rebased on top of current master just to be sure).
I can still reproduce this every time (without any other error):
$ mkdir bucket
$ bin/mkfs.s3ql --plain local://bucket
Before using S3QL, make sure to read the user's guide, especially
the 'Important Rules to Avoid Losing Data' section.
Creating metadata tables...
Uploading metadata...
Uploading metadata...
Uploaded 1 out of ~1 dirty blocks (100%)
Calculating metadata checksum...
$ mkdir mnt
$ bin/mount.s3ql --fg local://bucket mnt &
Using 10 upload threads.
Autodetected 1048514 file descriptors available for cache entries
Using cached metadata.
Setting cache size to 315297 MB
Mounting local:///home/nikratio/in-progress/s3ql/bucket/ at /home/nikratio/in-progress/s3ql/mnt...
$ md mnt/test; echo foo > mnt/test/bar
$ bin/s3qlrm mnt/test
fuse: writing device: Directory not empty
ERROR: Failed to submit invalidate_entry request for parent inode 1, name b'test'
Traceback (most recent call last):
File "src/internal.pxi", line 125, in pyfuse3._notify_loop
File "src/pyfuse3.pyx", line 915, in pyfuse3.invalidate_entry
OSError: [Errno 39] fuse_lowlevel_notify_delete returned: Directory not empty
nikratio@vostro ~/i/s3ql (notify_delete_bug)>
Best,
-Nikolaus
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [fuse-devel] Semantics of fuse_notify_delete()
2023-08-01 14:40 ` Nikolaus Rath
@ 2023-08-01 14:48 ` Miklos Szeredi
2023-08-01 16:05 ` Nikolaus Rath
0 siblings, 1 reply; 15+ messages in thread
From: Miklos Szeredi @ 2023-08-01 14:48 UTC (permalink / raw)
To: Nikolaus Rath
Cc: Martin Kaspar via fuse-devel, Linux FS Devel, Miklos Szeredi
On Tue, 1 Aug 2023 at 16:40, Nikolaus Rath <nikolaus@rath.org> wrote:
>
> On Tue, 1 Aug 2023, at 13:53, Miklos Szeredi via fuse-devel wrote:
> > Here's one with the virtual env and the correct head:
> >
> > root@kvm:~/s3ql# git log -1 --pretty="%h %s"
> > 3d35f18543d9 Reproducer for notify_delete issue. To confirm:
> > root@kvm:~/s3ql# ~/s3ql-python-env/bin/python bin/s3qlrm mnt/test
> > WARNING: Received unknown command via control inode
> > ERROR: Uncaught top-level exception:
> > Traceback (most recent call last):
> > File "/root/s3ql/bin/s3qlrm", line 21, in <module>
> > s3ql.remove.main(sys.argv[1:])
> > File "/root/s3ql/src/s3ql/remove.py", line 72, in main
> > pyfuse3.setxattr(ctrlfile, 'rmtree', cmd)
> > File "src/pyfuse3.pyx", line 629, in pyfuse3.setxattr
> > OSError: [Errno 22] Invalid argument: 'mnt/test/.__s3ql__ctrl__'
>
> This is odd. I have never heard of anyone having this problem before and it also works fine in the CI.
>
> I apologize that this is taking so much of your time.
>
> I have changed the code a bit to print out what exactly it is receiving: https://github.com/s3ql/s3ql/commit/eb31f7bff4bd985d68fa20c793c2f2edf5db61a5
>
> Would you mind updating your branch and trying again? (You'll need to fetch and reset, since I rebased on top of current master just to be sure).
>
> I can still reproduce this every time (without any other error):
>
> $ mkdir bucket
> $ bin/mkfs.s3ql --plain local://bucket
> Before using S3QL, make sure to read the user's guide, especially
> the 'Important Rules to Avoid Losing Data' section.
> Creating metadata tables...
> Uploading metadata...
> Uploading metadata...
> Uploaded 1 out of ~1 dirty blocks (100%)
> Calculating metadata checksum...
> $ mkdir mnt
> $ bin/mount.s3ql --fg local://bucket mnt &
> Using 10 upload threads.
> Autodetected 1048514 file descriptors available for cache entries
> Using cached metadata.
> Setting cache size to 315297 MB
> Mounting local:///home/nikratio/in-progress/s3ql/bucket/ at /home/nikratio/in-progress/s3ql/mnt...
>
> $ md mnt/test; echo foo > mnt/test/bar
> $ bin/s3qlrm mnt/test
> fuse: writing device: Directory not empty
> ERROR: Failed to submit invalidate_entry request for parent inode 1, name b'test'
> Traceback (most recent call last):
> File "src/internal.pxi", line 125, in pyfuse3._notify_loop
> File "src/pyfuse3.pyx", line 915, in pyfuse3.invalidate_entry
> OSError: [Errno 39] fuse_lowlevel_notify_delete returned: Directory not empty
>
> nikratio@vostro ~/i/s3ql (notify_delete_bug)>
WARNING: Received unknown command via control inode: b"1, b'test')"
ERROR: Uncaught top-level exception:
Traceback (most recent call last):
File "/root/s3ql/bin/s3qlrm", line 21, in <module>
s3ql.remove.main(sys.argv[1:])
File "/root/s3ql/src/s3ql/remove.py", line 74, in main
pyfuse3.setxattr(ctrlfile, 'rmtree', cmd)
File "src/pyfuse3.pyx", line 629, in pyfuse3.setxattr
OSError: [Errno 22] Invalid argument: 'mnt/test/.__s3ql__ctrl__'
Thanks,
Miklos
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [fuse-devel] Semantics of fuse_notify_delete()
2023-08-01 14:48 ` Miklos Szeredi
@ 2023-08-01 16:05 ` Nikolaus Rath
2023-08-01 17:39 ` Miklos Szeredi
0 siblings, 1 reply; 15+ messages in thread
From: Nikolaus Rath @ 2023-08-01 16:05 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Martin Kaspar via fuse-devel, Linux FS Devel, Miklos Szeredi
On Aug 01 2023, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Tue, 1 Aug 2023 at 16:40, Nikolaus Rath <nikolaus@rath.org> wrote:
>>
>> On Tue, 1 Aug 2023, at 13:53, Miklos Szeredi via fuse-devel wrote:
>> > Here's one with the virtual env and the correct head:
>> >
>> > root@kvm:~/s3ql# git log -1 --pretty="%h %s"
>> > 3d35f18543d9 Reproducer for notify_delete issue. To confirm:
>> > root@kvm:~/s3ql# ~/s3ql-python-env/bin/python bin/s3qlrm mnt/test
>> > WARNING: Received unknown command via control inode
>> > ERROR: Uncaught top-level exception:
>> > Traceback (most recent call last):
>> > File "/root/s3ql/bin/s3qlrm", line 21, in <module>
>> > s3ql.remove.main(sys.argv[1:])
>> > File "/root/s3ql/src/s3ql/remove.py", line 72, in main
>> > pyfuse3.setxattr(ctrlfile, 'rmtree', cmd)
>> > File "src/pyfuse3.pyx", line 629, in pyfuse3.setxattr
>> > OSError: [Errno 22] Invalid argument: 'mnt/test/.__s3ql__ctrl__'
>>
>> This is odd. I have never heard of anyone having this problem before and it also works fine in the CI.
>>
>> I apologize that this is taking so much of your time.
>>
>> I have changed the code a bit to print out what exactly it is receiving:
>> https://github.com/s3ql/s3ql/commit/eb31f7bff4bd985d68fa20c793c2f2edf5db61a5
>>
>> Would you mind updating your branch and trying again? (You'll need to fetch and reset,
>> since I rebased on top of current master just to be sure).
>>
>> I can still reproduce this every time (without any other error):
>>
>> $ mkdir bucket
>> $ bin/mkfs.s3ql --plain local://bucket
>> Before using S3QL, make sure to read the user's guide, especially
>> the 'Important Rules to Avoid Losing Data' section.
>> Creating metadata tables...
>> Uploading metadata...
>> Uploading metadata...
>> Uploaded 1 out of ~1 dirty blocks (100%)
>> Calculating metadata checksum...
>> $ mkdir mnt
>> $ bin/mount.s3ql --fg local://bucket mnt &
>> Using 10 upload threads.
>> Autodetected 1048514 file descriptors available for cache entries
>> Using cached metadata.
>> Setting cache size to 315297 MB
>> Mounting local:///home/nikratio/in-progress/s3ql/bucket/ at /home/nikratio/in-progress/s3ql/mnt...
>>
>> $ md mnt/test; echo foo > mnt/test/bar
>> $ bin/s3qlrm mnt/test
>> fuse: writing device: Directory not empty
>> ERROR: Failed to submit invalidate_entry request for parent inode 1, name b'test'
>> Traceback (most recent call last):
>> File "src/internal.pxi", line 125, in pyfuse3._notify_loop
>> File "src/pyfuse3.pyx", line 915, in pyfuse3.invalidate_entry
>> OSError: [Errno 39] fuse_lowlevel_notify_delete returned: Directory not empty
>>
>> nikratio@vostro ~/i/s3ql (notify_delete_bug)>
>
> WARNING: Received unknown command via control inode: b"1, b'test')"
> ERROR: Uncaught top-level exception:
> Traceback (most recent call last):
> File "/root/s3ql/bin/s3qlrm", line 21, in <module>
> s3ql.remove.main(sys.argv[1:])
> File "/root/s3ql/src/s3ql/remove.py", line 74, in main
> pyfuse3.setxattr(ctrlfile, 'rmtree', cmd)
> File "src/pyfuse3.pyx", line 629, in pyfuse3.setxattr
> OSError: [Errno 22] Invalid argument: 'mnt/test/.__s3ql__ctrl__'
Thanks! It looks like the extended attribute name and value that S3QL
receives from libfuse is corrupted. What reaches S3QL as the xattr name
is actually the truncated xattr value (leading parenthesis is missing).
Is it possible that you are running into a variant of
https://github.com/libfuse/libfuse/issues/730? This was fixed in libfuse
3.14.1 and introduced in 3.13.0.
Best,
-Nikolaus
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [fuse-devel] Semantics of fuse_notify_delete()
2023-08-01 16:05 ` Nikolaus Rath
@ 2023-08-01 17:39 ` Miklos Szeredi
0 siblings, 0 replies; 15+ messages in thread
From: Miklos Szeredi @ 2023-08-01 17:39 UTC (permalink / raw)
To: Miklos Szeredi, Martin Kaspar via fuse-devel, Linux FS Devel,
Miklos Szeredi
On Tue, 1 Aug 2023 at 18:05, Nikolaus Rath <Nikolaus@rath.org> wrote:
> Is it possible that you are running into a variant of
> https://github.com/libfuse/libfuse/issues/730? This was fixed in libfuse
> 3.14.1 and introduced in 3.13.0.
Yes, that was it: a rogue libfuse3 instance installed in /usr/local/lib.
With the debian libfuse3 I get ENOTEMPTY as expected. Will look at
why this is happening tomorrow.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [fuse-devel] Semantics of fuse_notify_delete()
2023-07-27 11:37 ` [fuse-devel] " Nikolaus Rath
2023-07-28 8:45 ` Nikolaus Rath
@ 2023-08-02 13:18 ` Miklos Szeredi
2023-08-02 14:43 ` Nikolaus Rath
1 sibling, 1 reply; 15+ messages in thread
From: Miklos Szeredi @ 2023-08-02 13:18 UTC (permalink / raw)
To: Miklos Szeredi via fuse-devel; +Cc: Linux FS Devel, miklos
On Thu, 27 Jul 2023 at 13:37, Nikolaus Rath <Nikolaus@rath.org> wrote:
>
> On Jul 27 2023, Miklos Szeredi via fuse-devel <fuse-devel@lists.sourceforge.net> wrote:
> > On Wed, 26 Jul 2023 at 20:09, Nikolaus Rath <Nikolaus@rath.org> wrote:
> >>
> >> Hello,
> >>
> >> It seems to me that fuse_notify_delete
> >> (https://elixir.bootlin.com/linux/v6.1/source/fs/fuse/dev.c#L1512) fails
> >> with ENOTEMPTY if there is a pending FORGET request for a directory
> >> entry within. Is that correct?
> >
> > It's bug if it does that.
> >
> > The code related to NOTIFY_DELETE in fuse_reverse_inval_entry() seems
> > historic. It's supposed to be careful about mountpoints and
> > referenced dentries, but d_invalidate() should have already gotten all
> > that out of the way and left an unhashed dentry without any submounts
> > or children. The checks just seem redundant, but not harmful.
> >
> > If you are managing to trigger the ENOTEMPTY case, then something
> > strange is going on, and we need to investigate.
>
> I can trigger this reliable on kernel 6.1.0-10-amd64 (Debian stable)
> with this sequence of operations:
>
> $ mkdir test
> $ echo foo > test/bar
> $ Trigger removal of test/bar and then test within the filesystem (not
> through unlink()/rmdir() but out-of-band)
Issue is that "test/.__s3ql__ctrl__" is still positive. I.e. the
directory is *really* not empty.
I thought that that's okay, and d_invalidate will recursively unhash
dentries, but that's not the case. d_invalidate removes submounts
but only unhashes the root of the subtree, leaving the rest intact.
So the solution here is to invoke NOTIFY_DELETE on
"test/.__s3ql__ctrl__" before doing it on "test" itself.
I don't think NOTIFY_DELETE was ever meant to be a recursive delete.
This is indicated by the simple_empty() check, which will fail if any
positive child dentries remain.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [fuse-devel] Semantics of fuse_notify_delete()
2023-08-02 13:18 ` Miklos Szeredi
@ 2023-08-02 14:43 ` Nikolaus Rath
2023-08-02 17:48 ` Miklos Szeredi
0 siblings, 1 reply; 15+ messages in thread
From: Nikolaus Rath @ 2023-08-02 14:43 UTC (permalink / raw)
To: Miklos Szeredi via fuse-devel; +Cc: Miklos Szeredi, Linux FS Devel, miklos
On Aug 02 2023, Miklos Szeredi via fuse-devel <fuse-devel@lists.sourceforge.net> wrote:
> On Thu, 27 Jul 2023 at 13:37, Nikolaus Rath <Nikolaus@rath.org> wrote:
>>
>> On Jul 27 2023, Miklos Szeredi via fuse-devel <fuse-devel@lists.sourceforge.net> wrote:
>> > On Wed, 26 Jul 2023 at 20:09, Nikolaus Rath <Nikolaus@rath.org> wrote:
>> >>
>> >> Hello,
>> >>
>> >> It seems to me that fuse_notify_delete
>> >> (https://elixir.bootlin.com/linux/v6.1/source/fs/fuse/dev.c#L1512) fails
>> >> with ENOTEMPTY if there is a pending FORGET request for a directory
>> >> entry within. Is that correct?
>> >
>> > It's bug if it does that.
>> >
>> > The code related to NOTIFY_DELETE in fuse_reverse_inval_entry() seems
>> > historic. It's supposed to be careful about mountpoints and
>> > referenced dentries, but d_invalidate() should have already gotten all
>> > that out of the way and left an unhashed dentry without any submounts
>> > or children. The checks just seem redundant, but not harmful.
>> >
>> > If you are managing to trigger the ENOTEMPTY case, then something
>> > strange is going on, and we need to investigate.
>>
>> I can trigger this reliable on kernel 6.1.0-10-amd64 (Debian stable)
>> with this sequence of operations:
>>
>> $ mkdir test
>> $ echo foo > test/bar
>> $ Trigger removal of test/bar and then test within the filesystem (not
>> through unlink()/rmdir() but out-of-band)
>
> Issue is that "test/.__s3ql__ctrl__" is still positive. I.e. the
> directory is *really* not empty.
>
> I thought that that's okay, and d_invalidate will recursively unhash
> dentries, but that's not the case. d_invalidate removes submounts
> but only unhashes the root of the subtree, leaving the rest intact.
>
> So the solution here is to invoke NOTIFY_DELETE on
> "test/.__s3ql__ctrl__" before doing it on "test" itself.
Ah, thanks a lot for your help! I did not think of the potential
connection to this pseudo-file.
Will think about how to best fix that. The problem is that LOOKUP for
the ctrl file name always succeeds (no matter the directory), so we'd
have to issue an additional NOTIFY_DELETE for every directory *and*
there'd still be a race condition with LOOKUP(ctrl_file) being called
in-between.
Best,
-Nikolaus
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [fuse-devel] Semantics of fuse_notify_delete()
2023-08-02 14:43 ` Nikolaus Rath
@ 2023-08-02 17:48 ` Miklos Szeredi
0 siblings, 0 replies; 15+ messages in thread
From: Miklos Szeredi @ 2023-08-02 17:48 UTC (permalink / raw)
To: Miklos Szeredi via fuse-devel; +Cc: Linux FS Devel, miklos
On Wed, 2 Aug 2023 at 16:43, Nikolaus Rath <Nikolaus@rath.org> wrote:
> Will think about how to best fix that. The problem is that LOOKUP for
> the ctrl file name always succeeds (no matter the directory), so we'd
> have to issue an additional NOTIFY_DELETE for every directory *and*
> there'd still be a race condition with LOOKUP(ctrl_file) being called
> in-between.
Only need to issue the NOTIFY_DELETE if the ctrl_file has actually
been looked up. If it has, then yes, an extra NOTIFY_DELETE needs to
be done. After having done NOTIFY_DELETE(ctrl_file) the
LOOKUP(ctrl_file) can just fail with -ENOENT.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-08-02 17:50 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-26 18:08 Semantics of fuse_notify_delete() Nikolaus Rath
2023-07-27 8:04 ` Miklos Szeredi
2023-07-27 11:37 ` [fuse-devel] " Nikolaus Rath
2023-07-28 8:45 ` Nikolaus Rath
2023-07-28 8:52 ` Miklos Szeredi
2023-07-31 14:12 ` Miklos Szeredi
2023-08-01 10:53 ` Nikolaus Rath
2023-08-01 12:53 ` Miklos Szeredi
2023-08-01 14:40 ` Nikolaus Rath
2023-08-01 14:48 ` Miklos Szeredi
2023-08-01 16:05 ` Nikolaus Rath
2023-08-01 17:39 ` Miklos Szeredi
2023-08-02 13:18 ` Miklos Szeredi
2023-08-02 14:43 ` Nikolaus Rath
2023-08-02 17:48 ` Miklos Szeredi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).