Hello
Christoph Hellwig wrote:
> more trivial review comments ontop of the previous one, after looking
> at things:
>
> - please never use list_for_each in new code but list_for_each_entry
done
> - never use kernel_thread in new code but kthread_*
done
> - do_sendfile duplicates the common sendfile code. why aren't you
> using the generic code?
fixed to use generic code.
> - there's tons of really useless assertation of the category
> discussed in the last thread
> - there's tons of deep pagecache messing in there. normally this
> shouldn't be a filesystem, and if this breaks because of VM changes you'll
> have to fix it, don't complain..
we are also not very happy with it. We will not complain, but try to simplify it.
> - you still do your plugin mess in ->readpage. honsetly could you
> please explain why mpage_readpage{,s} don't work for you?
The reason is performance. Reiser4 uses a search through the filesystem tree to
access metadata of a file.
If reiser4 implemented its read/write via generic functions it would have to
repeat the search for every page being read/written.
It is especially disappointing because reiser4 uses extents with which it is
able in most cases to find all file pointers to data blocks with only one search
through the tree.
Another reason is multithread load.
Tree search includes complex part of providing correct concurrent read/write
access to the tree including deadlock avoidance algorithm. On multithread
filesystem load minimizing of a number of tree searches should have improved
filesystem throughput.
These are on todo list yet.
> - (issues with the read/write path already addresses in the previous thread)
> - looking at ->d_count in ->release is wrong
> - still has security plugin stuff that duplicates LSM
> - why do underlying attributes change when VFS inode doesn't change?
> if not please rip out most of getattr_common
> - link_common S_ISDIR doesn't make sense, VFS takes care of it
> - please use the generic_readlink infrastructure
>
> additinoal comment is that the code is very messy, very different
> from normal kernel style, full of indirections and thus hard to read.
> real review will take some time.
We hope to improve that with your help
Thanks
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
| |
 |