On Thu, Jun 17, 2010 at 06:54:29PM +0100, Jamie Lokier wrote: > Valerie Aurora wrote: > >> Who needs d_ino anyway? I am running a kernel with this patch - > >> Gnome, a browser, IRC, kernel compile, etc. and everything works. > > I'm running a kernel with the below patch and everything still works. > > Apparently "ls -i" is still using the bogus d_ino performance > > improvement mentioned here because it returns all 1's for inode > > number. > > I'm surprised at "ls -i", as a patch to change that has been submitted: > > http://marc.info/?l=linux-kernel&m=125181054102075 > http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/17887 I was surprised too. I guess people still want to optimize ls -i, even at the cost of wrong results. > > Use of d_ino without the corresponding st_dev is always buggy in the > > presence of submounts, bind mounts, and union mounts. E.g., the d_ino > > of a mountpoint will be the inode number of the directory under the > > mountpoint, not the mounted directory. > > It's not surprising everything seems to work. > > It can be useful as a performance hint, which you probably didn't test. I'm afraid I wasn't entirely serious with that patch. :) But it was an interesting exercise. > I strongly disagree that correct code must call stat(). Correct code > can check against the list of mountpoints in /proc/mounts, because it > is strictly only mountpoints where the number doesn't agree with > stat() -- prior to your patch :-) If you are assuming that the application is parsing /proc/mounts (does anyone actually do this?), then the application can also learn about union mounts and not trust d_ino in any directory below the union mount point. :) > Anyway, maybe your patch is not allowed by POSIX :-) as follows > (posted to linux-kernel some time ago): > > http://marc.info/?l=linux-kernel&m=125181054102075 > http://www.gossamer-threads.com/lists/linux/kernel/1124140 > > The POSIX readdir spec says this: > > The structure dirent defined in the <dirent.h> header describes a > directory entry. The value of the structure's d_ino member shall be set > to the file serial number of the file named by the d_name member. > > The description for sys/stat.h makes the connection between > "file serial number" and the stat.st_ino member: > > The <sys/stat.h> header shall define the stat structure, which shall > include at least the following members: > ... > ino_t st_ino File serial number. > > Returning the covered inode's number at a mountpoint is apparently not > POSIX compliant either, but is widespread. (I.e. all unixes except > Cygwin apparently.) > > > Gosh, maybe it would help to patch the currently used readdir instead > > of just old_readdir() (thanks, Arnd). And return 1 instead of 0 so ls > > doesn't think all files are deleted (thanks, Andreas). > > It's not just ls. Bash 3.0 ignores entries for completion if d_ino == 0. > > > I'm running a kernel with the below patch and everything still works. > > Apparently "ls -i" is still using the bogus d_ino performance > > improvement mentioned here because it returns all 1's for inode > > number. > > > > http://www.mail-archive.com/bug-findutils@xxxxxxx/msg02531.html > > I'm intrigued by the mentioned in that report that Linux bind mounts > return the covering inode number in d_ino, not the covered inode number. > > If true, that means mounts are already being checked when returning d_ino, > and suggests that doing it for all mounts isn't expensive. This surprises me too. I will check into it further. -VAL -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
