Re: Possible readdir regression with BTRFS

[Filipe Manana <fdmanana@kernel.org>]

On Sat, Sep 09, 2023 at 08:27:03AM +0100, Filipe Manana wrote:
> On Fri, Sep 08, 2023 at 09:07:10PM -0400, Ian Johnson wrote:
> > I was debugging an issue recently and came across what seems to be a
> > change in the behavior of opendir and readdir in Btrfs seemingly
> > starting with 9b378f6ad48cfa195ed868db9123c09ee7ec5ea2, specifically
> > when the contents of a directory are changed between the call to opendir
> > and the initial call to readdir. The following C program demonstrates
> > the behavior:
> > 
> > #import <dirent.h>
> > #import <stdio.h>
> > 
> > int main(void) {
> >  DIR *dir = opendir("test");
> > 
> >  FILE *file;
> >  file = fopen("test/1", "w");
> >  fwrite("1", 1, 1, file);
> >  fclose(file);
> > 
> >  file = fopen("test/2", "w");
> >  fwrite("2", 1, 1, file);
> >  fclose(file);
> > 
> >  /* rewinddir(dir); */
> > 
> >  struct dirent *entry;
> >  while ((entry = readdir(dir))) {
> >    printf("%s\n", entry->d_name);
> >  }
> >  closedir(dir);
> >  return 0;
> > }
> > 
> > Running this program with an initially empty test directory will print
> > entry 1 but not entry 2 on Btrfs, while on Ext4 it prints both entries.
> > 
> > Evidently this particular result is not an issue, as POSIX states:
> > 
> > > 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_r() returns an entry for that file is unspecified.
> 
> Yes, I can modify it to be more "logical" and don't return any new entry
> after the opendir() call. As it is now, it always returns only the first
> new entry after opendir().
> 
> > 
> > However, the same program with rewinddir uncommented above yields the
> > same behavior, seemingly in contradiction of POSIX:
> > 
> > > It [rewinddir] shall also cause the directory stream to refer to the
> > > current state of the corresponding directory, as a call to opendir()
> > > would have done.
> 
> That seems to make sense.
> I'll prepare a patch to ensure that behaviour and let you know when it's
> ready for you to test.

Here it is (I CC'ed you, but just to link the thread):

https://lore.kernel.org/linux-btrfs/cover.1694260751.git.fdmanana@suse.com/

> 
> Thanks.
> 
> > 
> > I know that the functions used here are part of the C library and not
> > the kernel, but given this seems to be a recent change, I wanted to
> > bring this up. I looked over the man pages for getdents and lseek and
> > didn't see any mention of guarantees (or lack thereof) in the face of
> > concurrent directory updates, so it's quite possible that this is within
> > the realm of expected/allowed behavior.
> > 
> > 
> >