Re: [CFT][PATCH] ramfs/tmpfs readdir()

Linus Torvalds (torvalds@transmeta.com)
Sun, 4 Nov 2001 09:55:46 -0800 (PST)


On Sun, 4 Nov 2001, Alexander Viro wrote:
>
> Please, help with review and testing. Linus, if you have problems with
> that approach - please, tell.

The approach looks solid, although the fact that telldir/seekdir will
still be random is a bummer. And means that it _still_ won't be POSIX-
compatible..

(POSIX specifies that you can seek to any previously reported telldir
location - but not any random location).

This is better than what we have, though, so I wouldn't object to the
patch. I wonder why you export the internal dcache functions, though? The
only thing that _should_ need exporting is "dcache_dir_ops", no? We don't
want other filesystems mucking with the internals of this, as far as I can
tell.

As to telldir/seekdir - the approach will never fix them. The only thing
that will fix those would be to add a "d_offset" to the "struct dentry"
itself, and have the start of "filldir()" walk the chain until it finds
the first offset larger than the current one.

Then, dcache file creation would be something like

new_dentry->d_offset = 0;
if (!list_entry(parent->d_child))
new_dentry->d_offset = list_dentry(parent->d_child.prev)->d_offset + 1;

(or something similar), and we'd have to make sure that "d_add()" always
adds to the end - which it doesn't seem to do right now. Lots of small
details, and not as clean as your patch, but I don't see any better ways
to _really_ fix this.

Note that other filesystems would already enjoy having a d_offset in the
dentry: it allows for various other optimizations (ie making "unlink()" a
O(1) operation, by not having to search the directory).

So quite frankly I'd prefer the d_offset approach, and fix the directory
behaviour once and for all, not just the "linear traversal" case.

Admittedly linear traversal is the _common_ case, and arguably the much
more important of the two. However, right is right, and a true quality
implementation gets seekdir/telldir right too.

Have you looked at how nasty the d_offset thing would be?

Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/