linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@transmeta.com>
To: Alexander Viro <viro@math.psu.edu>
Cc: <linux-kernel@vger.kernel.org>
Subject: Re: [CFT][PATCH] ramfs/tmpfs readdir()
Date: Sun, 4 Nov 2001 09:55:46 -0800 (PST)	[thread overview]
Message-ID: <Pine.LNX.4.33.0111040943160.6919-100000@penguin.transmeta.com> (raw)
In-Reply-To: <Pine.GSO.4.21.0111041046040.21449-100000@weyl.math.psu.edu>


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


  reply	other threads:[~2001-11-04 17:59 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-11-04 15:52 [CFT][PATCH] ramfs/tmpfs readdir() Alexander Viro
2001-11-04 17:55 ` Linus Torvalds [this message]
2001-11-04 18:20   ` Alexander Viro
2001-11-04 18:27 Manfred Spraul
2001-11-05 13:05 ` Trond Myklebust

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Pine.LNX.4.33.0111040943160.6919-100000@penguin.transmeta.com \
    --to=torvalds@transmeta.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@math.psu.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).