Hi Matthew, Thanks for the reply. Sorry if it was unclear in my earlier email. I'm the engineer working on cifs.ko, who's now trying to understand what's the "standard" way of handling this which the VFS expects from the underlying filesystem. And it sounds to me like there's no such standard way. I read this codepath in a couple of popular filesystems, and each one seems to have it's own way of handling this. I wanted to reconfirm that the main issue is in the implementation of the rm command on this distro, and the way it's using libc. Regards, Shyam On Wed, Oct 21, 2020 at 7:51 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > On Tue, Oct 20, 2020 at 11:14:11AM +0530, Shyam Prasad N wrote: > > A summary of the issue: > > With alpine linux containers (which uses the musl implementation of > > libc), the "rm -Rf" command could fail depending upon the dir size. > > The Linux cifs client filesystem behaviour is compared against ext4 > > behaviour. > [...] > > Now the question is whether cifs.ko is doing anything wrong? > > @Steve French pointed me to this readdir documentation: > > https://pubs.opengroup.org/onlinepubs/9699919799/functions/readdir_r.html > > > > 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. > > > > So I guess the documents don't specify the behaviour in this case. > > Or rather, your implementation of 'rm' is relying on unspecified > behaviour. If it's doing rm -rf, it can keep calling readdir() [1] > but before it tries to unlink() the directory, it should rewinddir() > and see if it can find any more entries. It shouldn't rely on the kernel > to fix this up. ie: > > DIR *dirp = opendir(n); > bool first = true; > > for (;;) { > struct dirent *de = readdir(dirp); > > if (!de) { > if first) > break; > rewinddir(dirp); > continue; > } > first = false; > unlink(de.d_name); > } > unlink(n); > > ... only with error checking and so on. > > [1] Use readdir() rather than readdir_r() -- see the glibc 2.24+ > documentation for details. -- -Shyam
