On Sun, Aug 3, 2008 at 7:56 AM, Neil Brown <neilb@xxxxxxx> wrote: > On Saturday August 2, bfields@xxxxxxxxxxxx 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 -- 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
