|| ||Christoph Hellwig <hch-AT-infradead.org>|
|| ||Andrew Morton <akpm-AT-linux-foundation.org>|
|| ||Re: 2.6.29 -mm merge plans|
|| ||Tue, 6 Jan 2009 17:57:44 -0500|
|| ||Article, Thread
On Mon, Jan 05, 2009 at 12:43:00AM -0800, Andrew Morton wrote:
Just implementing this behaviour for x86 and uml is extremly
counter-productive. Please fix up all architectures as this
behaviour needs to be the same on all ports (at least those
with a MMU)
Btw, this code is still not quite right. We really need to call
->setattr instead of vmtruncate here. Complex filesystem need
transaction to properly free blocks, and those transactions are in
->setattr not inside vmtruncate where ->truncate doesn't even have
a chance to get the handle to the transaction passed.
As these patches don't make it worse this is not a NACK, but more of
a heads up.
I'm not sure this is a good idea. Concurrent syncs are a bad idea
to start with and we should just synchronyze do_sync completely.
sync_filesystems as one of the main components of do_sync already
is synchronized in that way, and taking that to a higher level would
get rid of all the worries about concurrent syncs.
Why is this in procfs?
By using lookup_one_len on an arbitrary underlying filesystem this
breaks the assumption that a nameidata is always available. Please
redo to use proper lookup helpers. Also whole code feels rather
fragile from a 10000 feet view, but beeing plsit in so many
patches it's almost impossible to properly review it anywya.
I'm pretty sure we already had a version better than that in your
tree on the list. But I've lost track and we should just restart
the review cycle on -fsdevel.
looks generally good, but the comments should get a little loving.
Please remove the stupid filenames that always get out of sync in
the top of file comments, and make the documentation of exported
symbols kernel-doc instead of it's weird own format.
Does checkpatch.pl still not catch these things?
Also the ioctl certainly should be an unlocked_ioctl and not the
old BKL-locked variant. The !uarg checks in the ioctls can go,
copy_to/from_users does this automatically.
pps.h shoulkd be split into one header only defining the
kernel<->userspace ABI, and a kernel-internal one. That way
also the conditional includes can go away.
Once again this stuff is in and utterly wrong place where it can't
easily be package for distros. ppsfind belongs into util-linux and
needs a proper mangage, ppsldisc is not nessecary but ldattach in
util-linux needs to grow support for N_PPS instead, and ppstest
should probably go into util-linux in a more polished version, too.
This one is utterly wrong. It provides what should be a userspace
library as inlines in a kernel header.
Please do a proper libpps library package.
> Add a kernel-wide swap() macro, use it. Merge.
Hmm, must have missed this when it went to lkml. Having this helper
generic is a good idea, but swap is far too generic for something
in kernel.h as indicated by the various renaming patches. How about
> Dunno. Has this been reviewed enough?
No. I might eventually take a look, but looking into btrfs has a little
higher priority right now, and split into gazillions of
non-selfcontained patches certainly doesn't help reviewing it.
BTW, the current influx of higher-complexity filesystems certainly worries
me a little.
to post comments)