Hello
On Tue, 2005-06-28 at 06:58, Andi Kleen wrote:
> Hans Reiser <[email protected]> writes:
>
> > * metafiles should be disabled until we can present code that works
> > right. Half the list thinks we cannot solve the cycles problem ever.
> > Disable metafiles and postpone problem until working code, or the
> > failure to produce it, makes it possible to do more than rant at each
> > other. This is currently already done in the -mm patches, but is
> > mentioned lest someone think it forgotten.
> >
> > * update the locking documentation
> >
> > Probably I forget something.
>
> These are all big picture issues, but I think some low level attention to
> the individual code is still needed.
>
> Some stuff that stood out from a very quick look:
>
> I would like for the custom spin lock debugging (spin_macros.h) and
> profiling code to be removed (prof.[ch], spinprof.[ch]). Such code shouldn't
> be in specific subsystems.
>
sorry, Andi, I guess you are looking at something old. Reiser4 does not
have neither prof.[ch], nor spinprof.[ch] and we removed already some
debugging code from spin_macros.h.
> The division functions in lib.h are useless IMHO, both callers seem
> to use divide by a power of two. And gcc supports shift in 64bit
> fine in the kernel. Can you remove that please?
>
ok
> statcnt.h: This is completely useless because you don't align
> the individual fields for cache lines - so you will still
> have false sharing everywhere. Also using NR_CPUS is nasty
> because it can be very big - num_possible_cpus() is better.
> It should use the new dynamic per cpu allocator.
>
statcnt.h is already removed.
> Best you just remove it for now and use atomic_t and readd properly
> when you do real SMP tuning with measurements.
>
> debug.[ch]: A lot of these functions like "schedulable" are name space
> space polluting.
> reiser4_kmalloc() such wrappers are deprecated. Please remove.
ok
> xmemset et.al should be replaced with the normal functions everywhere
>
done.
> Best would be probably to remove most of these files for submission.
>
> What is reiser4_internal? Can't you just use static like
> everybody else?
ok
> status_flags.c: Please remove that CONFIG_FRAME_POINTER code.
> In general i think it would be better if you removed that
> "private mini crashdumping".
ok
>
> Is there any reason you can't just use wait queues like everybody
> else instead of these reimplemented condition variables in kcond.[ch]?
>
I will investigate that
> In general it would be good if someone experienced not from the reiser team
> would read the whole source and looks for obvious problems
> (I didn't, just mentioning stuff I from a quick look at some support
> files)
>
> -Andi
>
>
-
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]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
