[regression] getdents() does not list entries created after opening the directory

[regression] getdents() does not list entries created after opening the directory

[Linux regression tracking (Thorsten Leemhuis)]

Hi, Thorsten here, the Linux kernel's regression tracker.

yangerkun, I noticed a report about a regression in bugzilla.kernel.org
that appears to be caused by the following change of yours:

64a7ce76fb901b ("libfs: fix infinite directory reads for offset dir")
[merged via: "Merge tag 'vfs-6.11-rc4.fixes' of
git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs"; v6.11-rc4]

As many (most?) kernel developers don't keep an eye on the bug tracker,
I decided to write this mail. To quote from
https://bugzilla.kernel.org/show_bug.cgi?id=219285:

> below program illustrates the problem. Expected output should include line "entry: after", actual output does not:
> ```
> entry: .
> entry: ..
> entry: before
> ```
> Program:
> 
> ```c
> #include <unistd.h>
> #include <dirent.h>
> #include <stdlib.h>
> #include <sys/stat.h>
> #include <stdio.h>
> #include <fcntl.h>
> 
> int main() {
> 	system("rm -rf /tmp/dirent-problems-test-dir");
> 	if (mkdir("/tmp/dirent-problems-test-dir", 0755)) {
> 		abort();
> 	}
> 
> 	int fd = creat("/tmp/dirent-problems-test-dir/before", 0644);
> 	if (fd < 0) {
> 		abort();
> 	}
> 	close(fd);
> 
> 	DIR* dir = opendir("/tmp/dirent-problems-test-dir");
> 
> 	fd = creat("/tmp/dirent-problems-test-dir/after", 0644);
> 	if (fd < 0) {
> 		abort();
> 	}
> 	close(fd);
> 
> 	struct dirent* entry;
> 	while ((entry = readdir(dir))) {
> 		printf("entry: %s\n", entry->d_name);
> 	}
> 
> 	closedir(dir);
> 	return 0;
> }
> ```
> 
> Affected kernel version: 6.10.10.
> Filesystem: ext4.
> Distribution: Arch Linux.

> On Linux 6.6.51 it works as expected.

> Regression first appeared in 6.10.7, 6.10.6 was good. I will further
> bisect tomorrow.

> 6.11 is still affected.

See the ticket for more details. Reporter ist CCed. I made no judgement
if the code provided is sane, I'm just assumed forwarding the issue was
a good idea.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

P.S.: let me use this mail to also add the report to the list of tracked
regressions to ensure it's doesn't fall through the cracks:

#regzbot introduced: 64a7ce76fb901bf9f9c36cf5d681328fc0fd4b5a
#regzbot title: libfs: getdents() does not list entries created after
opening the directory
#regzbot from: Krzysztof Małysa <varqox@gmail.com>
#regzbot duplicate: https://bugzilla.kernel.org/show_bug.cgi?id=219285
#regzbot ignore-activity

Re: [regression] getdents() does not list entries created after opening the directory

[Matthew Wilcox]

On Tue, Oct 01, 2024 at 01:29:09PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote:
> > 	DIR* dir = opendir("/tmp/dirent-problems-test-dir");
> > 
> > 	fd = creat("/tmp/dirent-problems-test-dir/after", 0644);

"If a file is removed from or added to the directory after the most
recent call to opendir() or rewinddir(), whether a subsequent call to
readdir() returns an entry for that file is unspecified."

https://pubs.opengroup.org/onlinepubs/007904975/functions/readdir.html

That said, if there's an easy fix here, it'd be a nice improvement to
QoI to do it, but the test-case as written is incorrect.

Re: [regression] getdents() does not list entries created after opening the directory

[Linux regression tracking (Thorsten Leemhuis)]

On 01.10.24 14:18, Matthew Wilcox wrote:
> On Tue, Oct 01, 2024 at 01:29:09PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote:
>>> 	DIR* dir = opendir("/tmp/dirent-problems-test-dir");
>>>
>>> 	fd = creat("/tmp/dirent-problems-test-dir/after", 0644);
> 
> "If a file is removed from or added to the directory after the most
> recent call to opendir() or rewinddir(), whether a subsequent call to
> readdir() returns an entry for that file is unspecified."
> 
> https://pubs.opengroup.org/onlinepubs/007904975/functions/readdir.html
> 
> That said, if there's an easy fix here, it'd be a nice improvement to
> QoI to do it, but the test-case as written is incorrect.

Many thx Willy!

Which leads to a question:

Krzysztof, how did you find the problem? Was there a practical use case
(some software or workload) with this behavior that broke and made your
write that test-case? Or is that a test-program older and part of your
CI tests or something like that?

Ciao, Thorsten

Re: [regression] getdents() does not list entries created after opening the directory

[Christian Heusel]

[-- Attachment #1: Type: text/plain, Size: 2509 bytes --]

On 24/10/01 02:49PM, Linux regression tracking (Thorsten Leemhuis) wrote:
> On 01.10.24 14:18, Matthew Wilcox wrote:
> > On Tue, Oct 01, 2024 at 01:29:09PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote:
> >>> 	DIR* dir = opendir("/tmp/dirent-problems-test-dir");
> >>>
> >>> 	fd = creat("/tmp/dirent-problems-test-dir/after", 0644);
> > 
> > "If a file is removed from or added to the directory after the most
> > recent call to opendir() or rewinddir(), whether a subsequent call to
> > readdir() returns an entry for that file is unspecified."
> > 
> > https://pubs.opengroup.org/onlinepubs/007904975/functions/readdir.html
> > 
> > That said, if there's an easy fix here, it'd be a nice improvement to
> > QoI to do it, but the test-case as written is incorrect.
> 
> Many thx Willy!
> 
> Which leads to a question:
> 
> Krzysztof, how did you find the problem? Was there a practical use case
> (some software or workload) with this behavior that broke and made your
> write that test-case? Or is that a test-program older and part of your
> CI tests or something like that?

The above message and the mentioned patch reminded me of an [old
issue][0] that is bothering us in the Arch Linux Infrastructure Team
which makes files vanish if modified during an rsync transaction (which
breaks our mirror infrastructure because it makes the package sync
databases [go missing][1]).

The issue was previously discussed with the BTRFS developers after they
implemented a [similar patch][2] (atleast judging from the title of
both) for their filesystem who also pointed to the standards compliance
after we have complained.

The workload and the issue with it (and how the new behaviour breaks
rsync for our usecase) was [nicely explained][3] by one of the BTRFS
developers.

So going back to the initial question: There could be a practical
usecase this causes a regression for, atleast if the patch has the same
implications as the BTRFS patch has. While we will have to sort out our
issue separately with the BTRFS folks I thought I'd still leave this
information in this thread.

> Ciao, Thorsten

Cheers,
Chris

[0]: https://lore.kernel.org/linux-btrfs/00ed09b9-d60c-4605-b3b6-f4e79bf92fca@foutras.com/
[1]: https://gitlab.archlinux.org/archlinux/infrastructure/-/issues/585
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=9b378f6ad48c
[3]: https://lore.kernel.org/linux-btrfs/ZP8AWKMVYOY0mAwq@debian0.Home/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [regression] getdents() does not list entries created after opening the directory

[Krzysztof Małysa]

wt., 1 paź 2024 o 14:49 Linux regression tracking (Thorsten Leemhuis)
<regressions@leemhuis.info> napisał(a):
>
> On 01.10.24 14:18, Matthew Wilcox wrote:
> > On Tue, Oct 01, 2024 at 01:29:09PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote:
> >>>     DIR* dir = opendir("/tmp/dirent-problems-test-dir");
> >>>
> >>>     fd = creat("/tmp/dirent-problems-test-dir/after", 0644);
> >
> > "If a file is removed from or added to the directory after the most
> > recent call to opendir() or rewinddir(), whether a subsequent call to
> > readdir() returns an entry for that file is unspecified."
> >
> > https://pubs.opengroup.org/onlinepubs/007904975/functions/readdir.html
> >
> > That said, if there's an easy fix here, it'd be a nice improvement to
> > QoI to do it, but the test-case as written is incorrect.
>
> Many thx Willy!
>
> Which leads to a question:
>
> Krzysztof, how did you find the problem? Was there a practical use case
> (some software or workload) with this behavior that broke and made your
> write that test-case? Or is that a test-program older and part of your
> CI tests or something like that?
>
> Ciao, Thorsten

I have a unit test (
https://github.com/varqox/sim-project/blob/889bcee60af56fa28613aaf52d27f3dd2c32a079/subprojects/simlib/test/directory.cc
) in my project’s test suite where I create a temporary directory,
populate it with files and then list its contents using the handle
obtained during creation of the directory. And it started to list the
directory empty, since this patch was introduced.

While listing the temporary dir one is using is unlikely in this case,
we will see if any issue in other software will emerge after the 6.11
will be released as LTS kernel.

Thanks,
Krzysztof