On Saturday August 2, [email protected] wrote:
>
> Though really I can't see any great objection to just moving xfs's hack
> up into nfsd. It may not do everything, but it seems like an
> incremental improvement.
Because it is a hack, and hacks have a tendency to hide deeper
problems, and not be ever get cleaned up and generally to become a
burden to future generations.
However if you do go down that path, can I suggest:
1/ get rid of the word "hack" throughout the code. If you think it
is sensible, make it appear sensible.
2/ drop the "retry malloc of a smaller size" thing.
In fact, you can probably use one of the set of pages that has
been reserved for the request. It is very rare that a readdir
request will be as big as the largest read.
3/ Make the new way unconditional. That gives it broader test
coverage which can only be a good thing. And what is good for the
goose is good for the gander... (not that I'm calling anyone a
goose).
But I still prefer the O_READDIRPLUS approach.
NeilBrown
On Sun, Aug 3, 2008 at 7:56 AM, Neil Brown <[email protected]> wrote:
> On Saturday August 2, [email protected] wrote:
>>
>> Though really I can't see any great objection to just moving xfs's hack
>> up into nfsd. It may not do everything, but it seems like an
>> incremental improvement.
>
> Because it is a hack, and hacks have a tendency to hide deeper
> problems, and not be ever get cleaned up and generally to become a
> burden to future generations.
Agreed that maintainability is an important concern.
However, I don't see that what David suggests in general is hiding a
deeper problem, but is rather exposing it. Can you explain what you
think is the issue?
> However if you do go down that path, can I suggest:
>
> 1/ get rid of the word "hack" throughout the code. If you think it
> is sensible, make it appear sensible.
Yes, if we're going to codify this method of handling readdir, let's
document it properly and treat it as a first class API.
> 2/ drop the "retry malloc of a smaller size" thing.
> In fact, you can probably use one of the set of pages that has
> been reserved for the request. It is very rare that a readdir
> request will be as big as the largest read.
Well, many Linux clients support reading directories only a page at a
time (this limitation may have been lifted recently). But other
clients often ask to read much more.
Again it appears that a Linux NFS client is not going to be the real
acid test here. Solaris and FreeBSD would probably be the best to
try, I think.
> 3/ Make the new way unconditional. That gives it broader test
> coverage which can only be a good thing. And what is good for the
> goose is good for the gander... (not that I'm calling anyone a
> goose).
As Bruce also suggested, and I agree with this (not the goose part,
the unconditionality and test coverage parts).
--
"Alright guard, begin the unnecessarily slow-moving dipping mechanism."
--Dr. Evil