We haven't been sending much email out, but we have been working away.
We just finished the VFS work, and will send a patch out on Monday.
akpm asked for a bullet list of things suggested on lkml as issues for
inclusion.
There are some things that I would like akpm to confirm represent the
official opinion/consensus as opposed to just someone posting an opinion
and perhaps not being right. I assure you that all points made by
commenters were considered carefully, and I thank all of them for their
time.
If we lose every remaining point of this list, we can generate a patch
in a few days, because the VFS work was the only substantive (in coding
hours) task, and it is done. Do I remember right that the submission
deadline is a week from Monday for 2.6.14 inclusion?
This is supposed to be a bullet list, so I don't list here the line by
line minor code improvements sent to us, most of which were
incorporated, but let me take a moment to thanks those who donated them.
Hans
1. pseudo files or "...." files
disabled. It remains a point of (extraordinary) contention as to whether it can be fixed, we want to keep the code around until we can devote proper resources into proving it can be (or until we fail to prove it can be and remove it). We don't want to delay the rest of the code for that proof, but we still think it can be done (by several different ways of which we need to select one and make it work.) Let us postpone contention on this until the existence of a patch that cannot crash makes contention purposeful, shall we?
2. dependency on 4k stack turned off
removed as requested
3. remove conditional variable code, use wait queues instead.
not done. There are times when reduced functionality aids debugging. kcond is (literally) textbook code.
We don't care enough to fight much for it, but akpm, what is your opinion? Will remove if akpm asks us to.
4. remove reiser4_drop_inode
done.
5. remove undesired abstraction layer right below reiser4 VFS hooks.
done. This was a big job just completed.
6. remove type safe lists and type safe hash queues.
not done, it is not clear that the person asking for this represents a unified consensus of lkml. Other persons instead asked that it just be moved out of reiser4 code into the generic kernel code, which implies they did not object to it. There are many who like being type safe. Akpm, what do you yourself think?
7. remove fs/reiser4/lib.h:/div64_32.
is being replaced by the linux one.
8. Remove all assertions because they clutter the code and make it hard to read
We think this person was not an experienced security specialist, and that what we do is considered by professional code auditors to be laudable practice. We can supply an emacs macro that makes them greyed out. I myself found the assertions to be distracting at first (though functional and especially necessary for a DARPA funded project), and then after time got used to them, and now I understand that it was just my inexperience that caused the discomfort. I now have a more sophisticated subconscious that is not discomforted or distracted by them. People debugging find them very useful. Defense branches tear their hair out at how difficult it is to get the commercial software they use coded this way, and I think they are right to be frustrated. Linux kernel folks, those DoD guys actually know quite a few things about security, maybe its ok that they taught me something and the code reflects that? We will conform on this if requested to by akpm, but somehow I doubt he will quite honestly. akpm?
On Fri, Sep 09, 2005 at 10:36:06AM -0700, Hans Reiser wrote:
> If we lose every remaining point of this list, we can generate a patch
> in a few days, because the VFS work was the only substantive (in coding
> hours) task, and it is done. Do I remember right that the submission
> deadline is a week from Monday for 2.6.14 inclusion?
No. 14 days from release date of 2.6.13, which was 8/29, so deadline
is 9/12, this coming Monday.
-chris
On Fri, Sep 09, 2005 at 10:36:06AM -0700, Hans Reiser wrote:
> 1. pseudo files or "...." files
>
> disabled. It remains a point of (extraordinary) contention as to whether it can be fixed, we want to keep the code around until we can devote proper resources into proving it can be (or until we fail to prove it can be and remove it). We don't want to delay the rest of the code for that proof, but we still think it can be done (by several different ways of which we need to select one and make it work.) Let us postpone contention on this until the existence of a patch that cannot crash makes contention purposeful, shall we?
please remove all unused code from the tree. You're free to leave stubs
in so you don't have to redo all common code, but don't leave unused files
in the tree.
> 6. remove type safe lists and type safe hash queues.
>
> not done, it is not clear that the person asking for this represents a unified consensus of lkml. Other persons instead asked that it just be moved out of reiser4 code into the generic kernel code, which implies they did not object to it. There are many who like being type safe. Akpm, what do you yourself think?
I think quite a few people complained ;-) It's huge CPP abuse which we
generally don't want - at least for the hash case we already had something
similar (linux/ghash.h and got rid of it). That beeing said a generic hash
abstraction without too much CPP abuse might be really useful, but the list
code should certainly go.
>
> 8. Remove all assertions because they clutter the code and make it hard to read
>
> We think this person was not an experienced security specialist,
please stop attacking people personally all the time. You're certainly not
what I'd call an experienced security specialist either, but fortunately that
doesn't matter for this case at all.
Removing all assertations certainly doesn't make sense, we have them all
over the tree. Whether your own assert macros makes sense is a different
question, but given that you use something similar in reiser3 and lots
of other drivers have their own things built around BUG() I won't complain
to loudly.
What should go on the other hand are useless assertation, like one that
asserts that something is non-NULL just before dereferencing it - the
latter gives a backtrace just as nice.
As additional requirements please give people time to actually audit the
codebase. I've started but it's quite a pain with all the plugin
indirections right now.
One major item I found is that you're using your own code for the
read/write file operations, which not only duplicates core code but
also lacks features (vectored I/O, AIO, direct I/O) from the common code
and is totally buggy (it has no chance on working on architectures that
have completely separate address spaces for user/kernel like s390, please
check how they define PAGE_OFFSET). Depending on your requirements you
might not use the complete generic code, but you should at least use
the most important fragments, ala XFS or ocfs, and improve them where
needed.
A very annoying small thing that comes to mind is the usage of
reiser4_internal. Please remove it, all but exported symbol are
module-private.
On Fri, Sep 09, 2005 at 07:57:40PM +0100, Christoph Hellwig wrote:
> A very annoying small thing that comes to mind is the usage of
> reiser4_internal. Please remove it, all but exported symbol are
> module-private.
Oh, one other things that's totally annoying and makes reading the
code a pain are the line-length and comment style. Please wrap
lines after 80 chars and use normal
/* foobar, blagg */
or
/*
* fvsfafsfasdrfasddddddddddddddddddddd
* fdsfsfdsfsdf
* sdfsdfs
*/
comments, thanks
Chris Shoemaker wrote:
>On Fri, Sep 09, 2005 at 10:36:06AM -0700, Hans Reiser wrote:
>
>
>>If we lose every remaining point of this list, we can generate a patch
>>in a few days, because the VFS work was the only substantive (in coding
>>hours) task, and it is done. Do I remember right that the submission
>>deadline is a week from Monday for 2.6.14 inclusion?
>>
>>
>
>No. 14 days from release date of 2.6.13, which was 8/29, so deadline
>is 9/12, this coming Monday.
>
>-chris
>
>
>
>
IIRC, Linus announced that since he was going on vacation for 7 days,
this release only it would be 3 weeks, so it is a week from Monday that
we must submit by, yes?
Hans
On 9/9/05, Hans Reiser <[email protected]> wrote:
> Chris Shoemaker wrote:
>
> >On Fri, Sep 09, 2005 at 10:36:06AM -0700, Hans Reiser wrote:
> >
> >
> >>If we lose every remaining point of this list, we can generate a patch
> >>in a few days, because the VFS work was the only substantive (in coding
> >>hours) task, and it is done. Do I remember right that the submission
> >>deadline is a week from Monday for 2.6.14 inclusion?
> >>
> >>
> >
> >No. 14 days from release date of 2.6.13, which was 8/29, so deadline
> >is 9/12, this coming Monday.
> >
> >-chris
> >
> >
> >
> >
> IIRC, Linus announced that since he was going on vacation for 7 days,
> this release only it would be 3 weeks, so it is a week from Monday that
> we must submit by, yes?
According to his mail (http://lkml.org/lkml/2005/9/8/208):
"As per the new merge policies that were discussed during LKS in Ottawa
earlier during the summer, I'm going to accept new stuff for 2.6.14 only
during the first two weeks after 2.6.13 was released.
That release was ten days ago, so you've got four more days before I don't
want any big merges."
Thanks,
Nish
On Fri, 9 Sep 2005, Hans Reiser wrote:
> Chris Shoemaker wrote:
>
> >On Fri, Sep 09, 2005 at 10:36:06AM -0700, Hans Reiser wrote:
> >
> >
> >>If we lose every remaining point of this list, we can generate a patch
> >>in a few days, because the VFS work was the only substantive (in coding
> >>hours) task, and it is done. Do I remember right that the submission
> >>deadline is a week from Monday for 2.6.14 inclusion?
> >>
> >>
> >
> >No. 14 days from release date of 2.6.13, which was 8/29, so deadline
> >is 9/12, this coming Monday.
> >
> >-chris
> >
> >
> IIRC, Linus announced that since he was going on vacation for 7 days,
> this release only it would be 3 weeks, so it is a week from Monday that
> we must submit by, yes?
Please read
http://marc.theaimsgroup.com/?l=linux-kernel&m=112620765730112&w=2
summary: on SEPT-08, Linus wrote:
"you've got four more days before I don't want any big merges."
=> until SEPT-12.
--
~Randy
On Fri, Sep 09, 2005 at 01:42:19PM -0700, Hans Reiser wrote:
> Chris Shoemaker wrote:
>
> >On Fri, Sep 09, 2005 at 10:36:06AM -0700, Hans Reiser wrote:
> >
> >
> >>If we lose every remaining point of this list, we can generate a patch
> >>in a few days, because the VFS work was the only substantive (in coding
> >>hours) task, and it is done. Do I remember right that the submission
> >>deadline is a week from Monday for 2.6.14 inclusion?
> >>
> >>
> >
> >No. 14 days from release date of 2.6.13, which was 8/29, so deadline
> >is 9/12, this coming Monday.
> >
> >-chris
> >
> >
> >
> >
> IIRC, Linus announced that since he was going on vacation for 7 days,
> this release only it would be 3 weeks, so it is a week from Monday that
> we must submit by, yes?
>
> Hans
On Thu Sep 08 2005 - 12:26:28 EST, Linus warned:
> That release was ten days ago, so you've got four more days before I
> don't want any big merges.
Surely you know how to search LKML archives for any mail from Linus
since the release of 2.6.13. It's only 2 mouse clicks if you know
where to find the archives. (3 if you start from kernel.org) Getting
lazy?
-chris
Christoph Hellwig wrote:
> 6. remove type safe lists and type safe hash queues.
>
>> not done, it is not clear that the person asking for this represents a unified consensus of lkml. Other persons instead asked that it just be moved out of reiser4 code into the generic kernel code, which implies they did not object to it. There are many who like being type safe. Akpm, what do you yourself think?
>>
>>
>
>I think quite a few people complained ;-)
>
I remember that you complained.
> It's huge CPP abuse
>
can you define what that means? and how abuse differs from cleverness?
This code was not my idea, but it seemed more cleverness than abuse to
me when I read it.
> which we
>generally don't want - at least for the hash case we already had something
>similar (linux/ghash.h and got rid of it). That beeing said a generic hash
>abstraction without too much CPP abuse might be really useful, but the list
>code should certainly go.
>
>
>
>>8. Remove all assertions because they clutter the code and make it hard to read
>>
>> We think this person was not an experienced security specialist,
>>
>>
>
>please stop attacking people personally all the time.
>
It is not an attack, fewer than 1% of programmers are security
specialists, and I go on to say that until I was educated by some such
specialists, I would have agreed with him, so I don't think you can view
it as an attack. I hardly expect most people to hang out with the DoD.
> You're certainly not
>what I'd call an experienced security specialist either, but fortunately that
>doesn't matter for this case at all.
>
>Removing all assertations certainly doesn't make sense, we have them all
>over the tree. Whether your own assert macros makes sense is a different
>question, but given that you use something similar in reiser3 and lots
>of other drivers have their own things built around BUG() I won't complain
>to loudly.
>
>What should go on the other hand are useless assertation, like one that
>asserts that something is non-NULL just before dereferencing it - the
>latter gives a backtrace just as nice.
>
>
I'll be happy to accept point of code improvements.
>
>As additional requirements please give people time to actually audit the
>codebase.
>
9 months time given so far....
> I've started but it's quite a pain with all the plugin
>indirections right now.
>
>
Here I sympathize with you. If anyone has ideas for how to comment it,
etc., to make it trivial to follow the indirections, I am all ears. It
annoys me also.
>One major item I found is that you're using your own code for the
>read/write file operations,
>
As I remember, there were things that did not code right using the
generic code, and I told the guys to code it right. I no longer
remember what those things were, quite honestly, but I remember they
affected performance. The whole idea of vfs operations is that each
file system gets to do them its own way, and please remember that we
code for more than just use on Linux.
> which not only duplicates core code but
>also lacks features (vectored I/O, AIO, direct I/O) from the common code
>
>
in time....
>and is totally buggy (it has no chance on working on architectures that
>have completely separate address spaces for user/kernel like s390, please
>check how they define PAGE_OFFSET).
>
zam, please look at that on Tuesday.
> Depending on your requirements you
>might not use the complete generic code, but you should at least use
>the most important fragments, ala XFS or ocfs, and improve them where
>needed.
>
>
Oh we do try to do that where it does what we need. However, you should
keep in mind that reiser4 is not made just for Linux. Our code needs to
be at least moderately self-contained.
>A very annoying small thing that comes to mind is the usage of
>reiser4_internal. Please remove it, all but exported symbol are
>module-private.
>
>
Here I mostly agree. Edward, can you work on a patch to fix it? Thanks.
>
>
>
Christoph, thanks kindly for the donation of your time to improving our
work.
Hans Reiser <[email protected]> wrote:
>
> ...
>
> Do I remember right that the submission
> deadline is a week from Monday for 2.6.14 inclusion?
Next week, supposedly.
But something like a brand new filesystem can go in pretty much any time,
as long as it compiles. Because it can't break anyone's current setup.
Brand new filesystems which also require monkeying with the core VFS aren't
quite that straightforward though.
> This is supposed to be a bullet list, so I don't list here the line by
> line minor code improvements sent to us, most of which were
> incorporated, but let me take a moment to thanks those who donated them.
Guys, you should know by now not to send 1021-column emails :(
> 1. pseudo files or "...." files
>
> disabled. It remains a point of (extraordinary) contention as to
> whether it can be fixed, we want to keep the code around until we can
> devote proper resources into proving it can be (or until we fail to prove
> it can be and remove it). We don't want to delay the rest of the code for
> that proof, but we still think it can be done (by several different ways of
> which we need to select one and make it work.) Let us postpone contention
> on this until the existence of a patch that cannot crash makes contention
> purposeful, shall we?
I'd prefer that unused code simply not be present in the tree, sorry.
>
>
> 2. dependency on 4k stack turned off
>
> removed as requested
So it all runs OK with 4k stacks now?
> 3. remove conditional variable code, use wait queues instead.
>
> not done. There are times when reduced functionality aids debugging.
> kcond is (literally) textbook code. We don't care enough to fight much for
> it, but akpm, what is your opinion? Will remove if akpm asks us to.
kcond is only used in a couple of places. One looks like it could use
complete() and the other is a standard wait-for-something-to-do kernel
thread loop, which we open-code without any fuss in lots of places
(kjournald, loop, pdflush, etc). So yes, I'd be inclined to remove kcond
please.
Also, it would be better to use the kthread API rather than open-coding
kernel_thread() calls. If you think that reiser4 needs additional ways of
controlling kernel threads then feel free to enhance the kthread API.
> 6. remove type safe lists and type safe hash queues.
>
> not done, it is not clear that the person asking for this represents a
> unified consensus of lkml. Other persons instead asked that it just be
> moved out of reiser4 code into the generic kernel code, which implies they
> did not object to it. There are many who like being type safe. Akpm, what
> do you yourself think?
The type-unsafety of existing list_heads gives me conniptions too. Yes,
it'd be nice to have a type-safe version available.
That being said, I don't see why such a thing cannot be a wrapper around
the existing list_head functions. Yes, there will be some ghastly
C-templates-via-CPP stuff, best avoided by not looking at the file ;)
We should aim for a complete 1:1 relationship between list_heads and
type-safe lists. So people know what they're called, know how they work,
etc. We shouldn't go adding things called rx_event_list_pop_back() when
everyone has learned the existing list API.
Of course, it would have been better to do this work as a completely
separate kernel feature rather than bundling it with a filesystem. If this
isn't a thing your team wants to take on now then yes, I'd be inclined to
switch reiser4 to list_heads.
>
> 7. remove fs/reiser4/lib.h:/div64_32.
>
> is being replaced by the linux one.
>
> 8. Remove all assertions because they clutter the code and make it hard to read
>
>
> We think this person was not an experienced security specialist,
We think this person didn't submit a patch to remove the 124 assertions
from ext3 ;) Keep the assertions.
Andrew Morton wrote:
>
>
>
>>1. pseudo files or "...." files
>>
>>
>>
>
>
>
>> disabled. It remains a point of (extraordinary) contention as to
>>whether it can be fixed, we want to keep the code around until we can
>>devote proper resources into proving it can be (or until we fail to prove
>>it can be and remove it). We don't want to delay the rest of the code for
>>that proof, but we still think it can be done (by several different ways of
>>which we need to select one and make it work.) Let us postpone contention
>>on this until the existence of a patch that cannot crash makes contention
>>purposeful, shall we?
>>
>>
>
>I'd prefer that unused code simply not be present in the tree, sorry.
>
>
Ok, edward will remove.
>
>
>>2. dependency on 4k stack turned off
>>
>> removed as requested
>>
>>
>
>So it all runs OK with 4k stacks now?
>
>
vs will answer this.
>
>
>>3. remove conditional variable code, use wait queues instead.
>>
>>not done. There are times when reduced functionality aids debugging.
>>kcond is (literally) textbook code. We don't care enough to fight much for
>>it, but akpm, what is your opinion? Will remove if akpm asks us to.
>>
>>
>
>kcond is only used in a couple of places. One looks like it could use
>complete() and the other is a standard wait-for-something-to-do kernel
>thread loop, which we open-code without any fuss in lots of places
>(kjournald, loop, pdflush, etc). So yes, I'd be inclined to remove kcond
>please.
>
>
ok, zam will do so.
>Also, it would be better to use the kthread API rather than open-coding
>kernel_thread() calls. If you think that reiser4 needs additional ways of
>controlling kernel threads then feel free to enhance the kthread API.
>
>
>
>>6. remove type safe lists and type safe hash queues.
>>
>>not done, it is not clear that the person asking for this represents a
>>unified consensus of lkml. Other persons instead asked that it just be
>>moved out of reiser4 code into the generic kernel code, which implies they
>>did not object to it. There are many who like being type safe. Akpm, what
>>do you yourself think?
>>
>>
>
>The type-unsafety of existing list_heads gives me conniptions too. Yes,
>it'd be nice to have a type-safe version available.
>
>That being said, I don't see why such a thing cannot be a wrapper around
>the existing list_head functions. Yes, there will be some ghastly
>C-templates-via-CPP stuff, best avoided by not looking at the file ;)
>
>We should aim for a complete 1:1 relationship between list_heads and
>type-safe lists. So people know what they're called, know how they work,
>etc. We shouldn't go adding things called rx_event_list_pop_back() when
>everyone has learned the existing list API.
>
>Of course, it would have been better to do this work as a completely
>separate kernel feature rather than bundling it with a filesystem. If this
>isn't a thing your team wants to take on now then yes, I'd be inclined to
>switch reiser4 to list_heads.
>
>
Ok, Edward, this task is yours. Edward, how big you make the task is up
to you so long as it is done by Monday. If you run low on time, get
help with it.
On Sep 9, 2005, at 17:21:30, Hans Reiser wrote:
>> It's huge CPP abuse
> can you define what that means? and how abuse differs from cleverness?
> This code was not my idea, but it seemed more cleverness than abuse to
> me when I read it.
"Debugging is twice as hard as writing the code in the first place.
Therefore, if you write the code as cleverly as possible, you are, by
definition, not smart enough to debug it."
-- Brian Kernighan
Sometimes cleverness can be even worse than ordinary abuse :-D. If the
code gets added to the kernel, then it should be debuggable (or at least
easily comprehensible) by a significant chunk of kernel developers, or
it will cause more problems than it solves. I think that a type-safe
list system _would_ be a good addition, but make sure you comment it
heavily enough to make it really obvious, even to those of us who are
less than brilliant (like myself).
Cheers,
Kyle Moffett
--
Simple things should be simple and complex things should be possible
-- Alan Kay
Kyle wrote:
> Sometimes cleverness can be even worse than ordinary abuse
I have a habit of commenting the code spots where it is not obvious
what is going on.
Someday I am going to learn that everytime I feel the compulsion
to write a "Watch out !" or "Beware !" or "Must !" code comment,
it probably means that I am coding crap.
I usually end up ripping out the code, after some bug has whacked
my upside the head.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.925.600.0401
On 9/9/05, Hans Reiser <[email protected]> wrote:
> 8. Remove all assertions because they clutter the code and make it hard to read
I don't think anyone suggested to remove _all_ assertions. I did,
however, suggested to tone down some of the ones that seem overly
defensive and clutter the code. If that is unacceptable to you and
maintainers can live with that then I have no problem with it either.
Pekka
Andrew Morton wrote:
>
>
>The type-unsafety of existing list_heads gives me conniptions too. Yes,
>it'd be nice to have a type-safe version available.
>
>That being said, I don't see why such a thing cannot be a wrapper around
>the existing list_head functions. Yes, there will be some ghastly
>C-templates-via-CPP stuff, best avoided by not looking at the file ;)
>
>We should aim for a complete 1:1 relationship between list_heads and
>type-safe lists. So people know what they're called, know how they work,
>etc. We shouldn't go adding things called rx_event_list_pop_back() when
>everyone has learned the existing list API.
>
>Of course, it would have been better to do this work as a completely
>separate kernel feature rather than bundling it with a filesystem. If this
>isn't a thing your team wants to take on now then yes, I'd be inclined to
>switch reiser4 to list_heads.
>
>
>
Vladimir spent 24 hours straight unsafing the lists in reiser4, and
didn't finish yet. We need 1-2 more days to address this before we can
submit reiser4. I hope this delay will not be too much of a problem.
Hans
Hans Reiser <[email protected]> wrote:
> Andrew Morton wrote:
> >The type-unsafety of existing list_heads gives me conniptions too. Yes,
> >it'd be nice to have a type-safe version available.
[...]
> Vladimir spent 24 hours straight unsafing the lists in reiser4, and
> didn't finish yet. We need 1-2 more days to address this before we can
> submit reiser4. I hope this delay will not be too much of a problem.
It won't go in before 2.6.15 or so anyway, so...
--
Dr. Horst H. von Brand User #22616 counter.li.org
Departamento de Informatica Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria +56 32 654239
Casilla 110-V, Valparaiso, Chile Fax: +56 32 797513