On Mon, 31 Jul 2006 10:26:55 +0100
"Denis Vlasenko" <[email protected]> wrote:
> The reiser4 thread seem to be longer than usual.
Meanwhile here's poor old me trying to find another four hours to finish
reviewing the thing.
The writeout code is ugly, although that's largely due to a mismatch between
what reiser4 wants to do and what the VFS/MM expects it to do. If it
works, we can live with it, although perhaps the VFS could be made smarter.
I'd say that resier4's major problem is the lack of xattrs, acls and
direct-io. That's likely to significantly limit its vendor uptake. (As
might the copyright assignment thing, but is that a kernel.org concern?)
The plugins appear to be wildly misnamed - they're just an internal
abstraction layer which permits later feature additions to be added in a
clean and safe manner. Certainly not worth all this fuss.
Could I suggest that further technical critiques of reiser4 include a
file-and-line reference? That should ease the load on vger.
Thanks.
Andrew Morton wrote:
>On Mon, 31 Jul 2006 10:26:55 +0100
>"Denis Vlasenko" <[email protected]> wrote:
>
>
>
>>The reiser4 thread seem to be longer than usual.
>>
>>
>
>Meanwhile here's poor old me trying to find another four hours to finish
>reviewing the thing.
>
>
Thanks Andrew.
>The writeout code is ugly, although that's largely due to a mismatch between
>what reiser4 wants to do and what the VFS/MM expects it to do.
>
I agree --- both with it being ugly, and that being part of why.
> If it
>works, we can live with it, although perhaps the VFS could be made smarter.
>
>
I would be curious regarding any ideas on that. Next time I read
through that code, I will keep in mind that you are open to making VFS
changes if it improves things, and I will try to get clever somehow and
send it by you. Our squalloc code though is I must say the most
complicated and ugliest piece of code I ever worked on for which every
cumulative ugliness had a substantive performance advantage requiring us
to keep it. If you spare yourself from reading that, it is
understandable to do so.
>I'd say that resier4's major problem is the lack of xattrs, acls and
>direct-io. That's likely to significantly limit its vendor uptake. (As
>might the copyright assignment thing, but is that a kernel.org concern?)
>
>
Thanks to you and the batch write code, direct io support will now be
much easier to code, and it probably will get coded the soonest of those
features. acls are on the todo list, but doing them right might require
solving a few additional issues (finishing the inheritance code, etc.)
>The plugins appear to be wildly misnamed - they're just an internal
>abstraction layer which permits later feature additions to be added in a
>clean and safe manner. Certainly not worth all this fuss.
>
>Could I suggest that further technical critiques of reiser4 include a
>file-and-line reference? That should ease the load on vger.
>
>Thanks.
>
>
>
>
Hello
On Mon, 2006-07-31 at 20:18 -0600, Hans Reiser wrote:
> Andrew Morton wrote:
> >The writeout code is ugly, although that's largely due to a mismatch between
> >what reiser4 wants to do and what the VFS/MM expects it to do.
Yes. reiser4 writeouts atoms. Most of pages get into atoms via
sys_write. But pages dirtied via shared mapping do not. They get into
atoms in reiser4's writepages address space operation. That is why
reiser4_sync_inodes has two steps: on first one it calls
generic_sync_sb_inodes to call writepages for dirty inodes to capture
pages dirtied via shared mapping into atoms. Second step flushes atoms.
> >
> I agree --- both with it being ugly, and that being part of why.
>
> > If it
> >works, we can live with it, although perhaps the VFS could be made smarter.
> >
> >
> I would be curious regarding any ideas on that. Next time I read
> through that code, I will keep in mind that you are open to making VFS
> changes if it improves things, and I will try to get clever somehow and
> send it by you. Our squalloc code though is I must say the most
> complicated and ugliest piece of code I ever worked on for which every
> cumulative ugliness had a substantive performance advantage requiring us
> to keep it. If you spare yourself from reading that, it is
> understandable to do so.
>
> >I'd say that resier4's major problem is the lack of xattrs, acls and
> >direct-io. That's likely to significantly limit its vendor uptake.
xattrs is really a problem.
On Tue, Aug 01, 2006 at 01:31:04AM -0700, Andrew Morton wrote:
> On Mon, 31 Jul 2006 10:26:55 +0100
> "Denis Vlasenko" <[email protected]> wrote:
>
> > The reiser4 thread seem to be longer than usual.
> I'd say that resier4's major problem is the lack of xattrs, acls and
> direct-io.
I though the namesys folks finally switched to the generic read/write
code? With that implementing direct I/O should become rather simple -
they need a get_block routines that deals with these writes aswell as the
reads mostly.
> The plugins appear to be wildly misnamed - they're just an internal
> abstraction layer which permits later feature additions to be added in a
> clean and safe manner. Certainly not worth all this fuss.
That because the real plugins are long gone. It's just that neither the
complainers nor the fanboys in this thread ever read the code or generally
had any clue of their own.
Andrew Morton wrote:
> On Mon, 31 Jul 2006 10:26:55 +0100
> "Denis Vlasenko" <[email protected]> wrote:
>
>
>>The reiser4 thread seem to be longer than usual.
>
>
> Meanwhile here's poor old me trying to find another four hours to finish
> reviewing the thing.
>
> The writeout code is ugly, although that's largely due to a mismatch between
> what reiser4 wants to do and what the VFS/MM expects it to do. If it
> works, we can live with it, although perhaps the VFS could be made smarter.
>
> I'd say that resier4's major problem is the lack of xattrs, acls and
> direct-io. That's likely to significantly limit its vendor uptake. (As
> might the copyright assignment thing, but is that a kernel.org concern?)
>
> The plugins appear to be wildly misnamed - they're just an internal
> abstraction layer which permits later feature additions to be added in a
> clean and safe manner. Certainly not worth all this fuss.
>
> Could I suggest that further technical critiques of reiser4 include a
> file-and-line reference? That should ease the load on vger.
I haven't really reviewed it, but when I grepped through it last, I
found a few alarming things, like use of __put_page, trying to remove
pages from pagecache (duplicating parts of vmscan.c, plus bugs), and
taking tree_lock.
Mostly didn't look like big problems to fix, but should be fixed for
mm/ maintainers' sanity. Maybe it's better now, though.
--
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com
On Tue, 01 Aug 2006 15:24:37 +0400
"Vladimir V. Saveliev" <[email protected]> wrote:
> > >The writeout code is ugly, although that's largely due to a mismatch between
> > >what reiser4 wants to do and what the VFS/MM expects it to do.
>
> Yes. reiser4 writeouts atoms. Most of pages get into atoms via
> sys_write. But pages dirtied via shared mapping do not. They get into
> atoms in reiser4's writepages address space operation.
It think you mean ->writepage - reiser4 desn't implement ->writepages().
I assume you considered hooking into ->set_page_dirty() to do the
add-to-atom thing earlier on?
We'll merge mm-tracking-shared-dirty-pages.patch into 2.6.19-rc1, which
would make that approach considerably more successful, I expect.
->set_page_dirty() is a bit awkward because it can be called under
spinlock.
Maybe comething could also be gained from the new
vm_operations_struct.page_mkwrite(), although that's less obvious...
> That is why
> reiser4_sync_inodes has two steps: on first one it calls
> generic_sync_sb_inodes to call writepages for dirty inodes to capture
> pages dirtied via shared mapping into atoms. Second step flushes atoms.
>
> > >
> > I agree --- both with it being ugly, and that being part of why.
> >
> > > If it
> > >works, we can live with it, although perhaps the VFS could be made smarter.
> > >
> > >
> > I would be curious regarding any ideas on that. Next time I read
> > through that code, I will keep in mind that you are open to making VFS
> > changes if it improves things, and I will try to get clever somehow and
> > send it by you. Our squalloc code though is I must say the most
> > complicated and ugliest piece of code I ever worked on for which every
> > cumulative ugliness had a substantive performance advantage requiring us
> > to keep it. If you spare yourself from reading that, it is
> > understandable to do so.
> >
> > >I'd say that resier4's major problem is the lack of xattrs, acls and
> > >direct-io. That's likely to significantly limit its vendor uptake.
>
> xattrs is really a problem.
That's not good. The ability to properly support SELinux is likely to be
important.
On Tue, 01 Aug 2006 21:52:03 +1000
Nick Piggin <[email protected]> wrote:
> Andrew Morton wrote:
> > On Mon, 31 Jul 2006 10:26:55 +0100
> > "Denis Vlasenko" <[email protected]> wrote:
> >
> >
> >>The reiser4 thread seem to be longer than usual.
> >
> >
> > Meanwhile here's poor old me trying to find another four hours to finish
> > reviewing the thing.
> >
> > The writeout code is ugly, although that's largely due to a mismatch between
> > what reiser4 wants to do and what the VFS/MM expects it to do. If it
> > works, we can live with it, although perhaps the VFS could be made smarter.
> >
> > I'd say that resier4's major problem is the lack of xattrs, acls and
> > direct-io. That's likely to significantly limit its vendor uptake. (As
> > might the copyright assignment thing, but is that a kernel.org concern?)
> >
> > The plugins appear to be wildly misnamed - they're just an internal
> > abstraction layer which permits later feature additions to be added in a
> > clean and safe manner. Certainly not worth all this fuss.
> >
> > Could I suggest that further technical critiques of reiser4 include a
> > file-and-line reference? That should ease the load on vger.
>
> I haven't really reviewed it, but when I grepped through it last, I
> found a few alarming things, like use of __put_page, trying to remove
> pages from pagecache (duplicating parts of vmscan.c, plus bugs), and
> taking tree_lock.
__put_page() has gone.
A lot of the VM duplication has gone.
tree_lock is still there a bit. For two reasons:
a) A presently-unneeded duplication of the generic
__set_page_dirty_nobuffers() and
b) Manipulation of the fake inode (I think).
iirc the base problem here is that for whatever reason, the usual
blockdev inode which filesystems use for metadata (ie: the pagecache
which backs sb_bread(), etc) isn't suitable. reiser4 wants to get a
hold of its address_space ops, so it needs to create its own
covers-all-the-partition, identify-mapped address_space.
> Mostly didn't look like big problems to fix, but should be fixed for
> mm/ maintainers' sanity. Maybe it's better now, though.
It's better. More reviewing help is always appreciated ;)
Hello
On Tue, 2006-08-01 at 07:33 -0700, Andrew Morton wrote:
> On Tue, 01 Aug 2006 15:24:37 +0400
> "Vladimir V. Saveliev" <[email protected]> wrote:
>
> > > >The writeout code is ugly, although that's largely due to a mismatch between
> > > >what reiser4 wants to do and what the VFS/MM expects it to do.
> >
> > Yes. reiser4 writeouts atoms. Most of pages get into atoms via
> > sys_write. But pages dirtied via shared mapping do not. They get into
> > atoms in reiser4's writepages address space operation.
>
> It think you mean ->writepage - reiser4 desn't implement ->writepages().
>
no.
there is one. It is reiser4/plugin/file/file.c:writepages_unix_file().
reiser4_writepage just kicks kernel thread (entd) which works similar to
reiser4_sync_inodes() (described earlier) and waits until several pages
(including the one reiser4_writepage is called with) are written.
> I assume you considered hooking into ->set_page_dirty() to do the
> add-to-atom thing earlier on?
>
Currently, add-to-atom is not simple. It may require memory allocations
and disk i/o-s. I guess these are not supposed to be called in
->set_page_dirty(). That is why in reiser4_set_page_dirty we just mark
the page in mapping's tree and dealy adding to atoms until flush time.
> We'll merge mm-tracking-shared-dirty-pages.patch into 2.6.19-rc1, which
> would make that approach considerably more successful, I expect.
> ->set_page_dirty() is a bit awkward because it can be called under
> spinlock.
>
> Maybe comething could also be gained from the new
> vm_operations_struct.page_mkwrite(), although that's less obvious...
>
> > That is why
> > reiser4_sync_inodes has two steps: on first one it calls
> > generic_sync_sb_inodes to call writepages for dirty inodes to capture
> > pages dirtied via shared mapping into atoms. Second step flushes atoms.
> >
> > > >
> > > I agree --- both with it being ugly, and that being part of why.
> > >
> > > > If it
> > > >works, we can live with it, although perhaps the VFS could be made smarter.
> > > >
> > > >
> > > I would be curious regarding any ideas on that. Next time I read
> > > through that code, I will keep in mind that you are open to making VFS
> > > changes if it improves things, and I will try to get clever somehow and
> > > send it by you. Our squalloc code though is I must say the most
> > > complicated and ugliest piece of code I ever worked on for which every
> > > cumulative ugliness had a substantive performance advantage requiring us
> > > to keep it. If you spare yourself from reading that, it is
> > > understandable to do so.
> > >
> > > >I'd say that resier4's major problem is the lack of xattrs, acls and
> > > >direct-io. That's likely to significantly limit its vendor uptake.
> >
> > xattrs is really a problem.
>
> That's not good. The ability to properly support SELinux is likely to be
> important.
>
Do you think that if reiser4 supported xattrs - it would increase its
chances on inclusion?
PS: what exactly did you refer to when you said that writeout code is
ugly?
Vladimir V. Saveliev wrote:
> Do you think that if reiser4 supported xattrs - it would increase its
> chances on inclusion?
Probably the opposite.
If I understand it right, the original Reiser4 model of file metadata is
the file-as-directory stuff that caused such a furor the last big push
for inclusion (search for "Silent semantic changes in Reiser4"):
foo.mp3/.../rwx # permissions
foo.mp3/.../artist # part of the id3 tag
So I suspect xattrs would just be a different interface to this stuff,
maybe just a subset of it (to prevent namespace collisions):
foo.mp3/.../xattr/ # contains files representing attributes
Of course, you'd be able to use the standard interface for
getting/setting these. The point is, I don't think Hans/Namesys wants
to do this unless they're going to do it right, especially because they
already have the file-as-dir stuff somewhat done. Note that these are
neither mutually exclusive nor mutually dependent -- you don't have to
enable file-as-dir to make xattrs work.
I know it's not done yet, though. I can understand Hans dragging his
feet here, because xattrs and traditional acls are examples of things
Reiser4 is supposed to eventually replace.
Anyway, if xattrs were done now, the only good that would come of it is
building a userbase outside the vanilla kernel. I can't see it as doing
anything but hurting inclusion by introducing more confusion about
"plugins".
I could be entirely wrong, though. I speak for neither
Hans/Namesys/reiserfs nor LKML. Talk amongst yourselves...
On 8/1/06, Andrew Morton <[email protected]> wrote:
> On Tue, 01 Aug 2006 15:24:37 +0400
> "Vladimir V. Saveliev" <[email protected]> wrote:
>
> > > >The writeout code is ugly, although that's largely due to a mismatch between
> > > >what reiser4 wants to do and what the VFS/MM expects it to do.
> >
> > Yes. reiser4 writeouts atoms. Most of pages get into atoms via
> > sys_write. But pages dirtied via shared mapping do not. They get into
> > atoms in reiser4's writepages address space operation.
>
> It think you mean ->writepage - reiser4 desn't implement ->writepages().
>
> I assume you considered hooking into ->set_page_dirty() to do the
> add-to-atom thing earlier on?
>
> We'll merge mm-tracking-shared-dirty-pages.patch into 2.6.19-rc1, which
> would make that approach considerably more successful, I expect.
> ->set_page_dirty() is a bit awkward because it can be called under
> spinlock.
>
> Maybe comething could also be gained from the new
> vm_operations_struct.page_mkwrite(), although that's less obvious...
>
> > That is why
> > reiser4_sync_inodes has two steps: on first one it calls
> > generic_sync_sb_inodes to call writepages for dirty inodes to capture
> > pages dirtied via shared mapping into atoms. Second step flushes atoms.
> >
> > > >
> > > I agree --- both with it being ugly, and that being part of why.
> > >
> > > > If it
> > > >works, we can live with it, although perhaps the VFS could be made smarter.
> > > >
> > > >
> > > I would be curious regarding any ideas on that. Next time I read
> > > through that code, I will keep in mind that you are open to making VFS
> > > changes if it improves things, and I will try to get clever somehow and
> > > send it by you. Our squalloc code though is I must say the most
> > > complicated and ugliest piece of code I ever worked on for which every
> > > cumulative ugliness had a substantive performance advantage requiring us
> > > to keep it. If you spare yourself from reading that, it is
> > > understandable to do so.
> > >
> > > >I'd say that resier4's major problem is the lack of xattrs, acls and
> > > >direct-io. That's likely to significantly limit its vendor uptake.
> >
> > xattrs is really a problem.
>
> That's not good. The ability to properly support SELinux is likely to be
> important.
i disagreee that it will be difficult. unfortunately, the patch that
I am working on right now, which fixes the various reiser4 specific
functions to avoid using VFS data structures unless needed, is a
prerequisite to enabling xattrs. creating it is a time of tedium for
me, and it will cause a bit of internal churn (1000 lines and
counting). it's all in the fs/reiser4 directory though, and it should
cause minimal disruption, as far as runtime bugs introduced.
once that's taken care of, i will be delighted to enable xattr support
in a way that will make selinux and beagle and such run as expected,
and will have the added advantage of some major scalability
improvements for certain lookup and update operations.
NATE
On 8/1/06, David Masover <[email protected]> wrote:
> Vladimir V. Saveliev wrote:
>
> > Do you think that if reiser4 supported xattrs - it would increase its
> > chances on inclusion?
>
> Probably the opposite.
>
> If I understand it right, the original Reiser4 model of file metadata is
> the file-as-directory stuff that caused such a furor the last big push
> for inclusion (search for "Silent semantic changes in Reiser4"):
>
> foo.mp3/.../rwx # permissions
> foo.mp3/.../artist # part of the id3 tag
>
> So I suspect xattrs would just be a different interface to this stuff,
> maybe just a subset of it (to prevent namespace collisions):
>
> foo.mp3/.../xattr/ # contains files representing attributes
>
> Of course, you'd be able to use the standard interface for
> getting/setting these. The point is, I don't think Hans/Namesys wants
> to do this unless they're going to do it right, especially because they
> already have the file-as-dir stuff somewhat done. Note that these are
> neither mutually exclusive nor mutually dependent -- you don't have to
> enable file-as-dir to make xattrs work.
>
> I know it's not done yet, though. I can understand Hans dragging his
> feet here, because xattrs and traditional acls are examples of things
> Reiser4 is supposed to eventually replace.
>
> Anyway, if xattrs were done now, the only good that would come of it is
> building a userbase outside the vanilla kernel. I can't see it as doing
> anything but hurting inclusion by introducing more confusion about
> "plugins".
>
> I could be entirely wrong, though. I speak for neither
> Hans/Namesys/reiserfs nor LKML. Talk amongst yourselves...
i should clarify things a bit here. yes, hans' goal is for there to
be no difference between the "xattr" namespace and the "readdir" one.
unfortunately, this is not feasible with the current VFS, and some
major work would have to be done to enable this without some
pathological cases cropping up. some very smart people think that it
cannot be done at all.
xattr is a seperate VFS interface, which avoids those problems by
defining certain restrictions on how the 'files' which live in that
namespace can be manupulated. for instance, hard links are
non-existent, and the 'mv' command cannot move a file between
different xattr namespaces.
enabling xattr would have no connection to the file-as-directory
stuff, and (without extra work) would not even allow access to the
things reiser4 defined in the '...' directory. also enabling xattr in
the way i intend would in no way compromise hans' long-term vision.
HOWEVER, i *need* to point out that hans and i disagree somewhat on
the specifics here, and so i should say adamently "i don't speak here
on behalf of hans or namesys".
that won't stop me from submitting my own patch though :)
NATE
Andrew Morton <[email protected]> writes:
> On Mon, 31 Jul 2006 10:26:55 +0100
> "Denis Vlasenko" <[email protected]> wrote:
>
> > The reiser4 thread seem to be longer than usual.
>
> Meanwhile here's poor old me trying to find another four hours to finish
> reviewing the thing.
I took a quick look at it and I must say that most of the things
that tripped me up when I first looked at it a long time ago
are gone now.
I guess there could be still quite a lot of cleanup, but it already
looks much better.
-Andi
Nate Diller wrote:
> On 8/1/06, David Masover <[email protected]> wrote:
>> Vladimir V. Saveliev wrote:
>> I could be entirely wrong, though. I speak for neither
>> Hans/Namesys/reiserfs nor LKML. Talk amongst yourselves...
>
> i should clarify things a bit here. yes, hans' goal is for there to
> be no difference between the "xattr" namespace and the "readdir" one.
> unfortunately, this is not feasible with the current VFS, and some
> major work would have to be done to enable this without some
> pathological cases cropping up. some very smart people think that it
> cannot be done at all.
But an xattr interface should work just fine, even if the rest of the
system is inaccessible (no readdir interface) -- preventing all these
pathological problems, except the one where Hans implements it the way
I'm thinking, and kernel people hate it.
On Tue, Aug 01, 2006 at 11:55:57AM -0500, David Masover wrote:
>
> If I understand it right, the original Reiser4 model of file metadata is
> the file-as-directory stuff that caused such a furor the last big push
> for inclusion (search for "Silent semantic changes in Reiser4"):
The furor was caused by concerns Al Viro expressed about
locking/deadlock issues that reiser4 introduced.
The bigger issue with xattr support is two-fold. First of all, there
are the progams that are expecting the existing extended attribute
interface, and not implementing it will simply mean that as far as
Samba, and other applications, Reiser4 simply will not haved xattr
support.
More importantly are the system-level extended attributes, such as
those used by SELINUX, which by definition are not supposed to be
visible to the user at all, but only are supposed to be there for the
SELINUX (or some other kernel layer, in general) to access.
Not supporting xattrs means that those distro's that use SELINUX by
default (i.e., RHEL, Fedora, etc.) won't want to use reiser4, because
SELINUX won't work on reiser4 filesytstems.
Whether or not Hans cares about this is up to him....
- Ted
Theodore Tso wrote:
> On Tue, Aug 01, 2006 at 11:55:57AM -0500, David Masover wrote:
>> If I understand it right, the original Reiser4 model of file metadata is
>> the file-as-directory stuff that caused such a furor the last big push
>> for inclusion (search for "Silent semantic changes in Reiser4"):
>
> The furor was caused by concerns Al Viro expressed about
> locking/deadlock issues that reiser4 introduced.
Which, I believe, was about file-as-dir. Which also had problems with
things like directory loops. That's sort of a disk space memory leak.
> The bigger issue with xattr support is two-fold. First of all, there
> are the progams that are expecting the existing extended attribute
> interface,
Yeah...
> More importantly are the system-level extended attributes, such as
> those used by SELINUX, which by definition are not supposed to be
> visible to the user at all,
I don't see why either of these are issues. The SELINUX stuff can be a
plugin which doesn't necessarily have a user-level interface.
Cryptocompress, for instance, exists independent of its user-level
interface (probably the file-as-dir stuff), and will probably be
implemented in some sort of stable form as a system-wide default for new
files.
So, certainly metadata (xattrs) as a plugin could be implemented with no
UI at all, or any given UI.
... Anyway, I still see no reason why these cannot be implemented in
Reiser4, other than the possibility that if it uses "plugins", I
guarantee that at least one or two people will hate the implementation
for that reason alone.
> Not supporting xattrs means that those distro's that use SELINUX by
> default (i.e., RHEL, Fedora, etc.) won't want to use reiser4, because
> SELINUX won't work on reiser4 filesytstems.
Right. So they will be implemented, eventually.
> Whether or not Hans cares about this is up to him....
He does, or he should. Reiser4 needs every bit of acceptance it can get
right now, as long as it can get them without compromising its goals or
philosophy. Extended attributes only compromise these because it
provides less incentive to learn any other metadata interface that
Reiser4 provides. But that's irrelevant if Reiser4 doesn't gain enough
acceptance due to lack of xattr support, anything it has will be
irrelevant anyway.
So just as we provide the standard interface to Unix permissions (even
though we intend to implement things like acls and views, and even
though there was a file/.pseudo/rwx interface), we should provide the
standard xattr interface, and the standard direct IO interface, and
anything else that's practical. Be a good, standard filesystem first,
and an innovative filesystem second.