Re: ncr53c8xx hack I disagree _strongly_ with

Gerard Roudier (groudier@club-internet.fr)
Sun, 18 Jul 1999 12:36:46 +0200 (MET DST)


Hi Ingo,

My post is not against your 4 macros that are probably good if we consider
them as just basic stuff. But, FYI, there are about 247 hits of your
macros in 2.2.9 and 240 of them involves jiffies.

People who want to help debugging should also send their patches to the
maintainer. They just have to look into the header of the source file.
Failing to do so may lead to unnecessary burden. For example:

- I will have to waste time fixing my incremental patch series.
- I have been an idiot each time I claimed that people could apply
patches for 2.2.9 to 2.2.10. This just occurred, by the way.

As you know, there is still some FS corruption problem that haven't been
cleared up in 2.2.10, based on reports I see at the list. Since drivers
are often suspected in that process, I had decided not to send patches
except real bug fixes until we will have a real handle on the problem.
I still appeared as a complete idiot, each time I have written that.

At the moment, I have been reported that a system has PCI parity problems
with 2.2.10-ac10, but works under 2.2.10-ac10 using drivers from 2.2.9.
The changes between these 2 versions seem not correlated with the problem,
but they add useless confusion.

I have patches for both 2.2.X and 2.3.X (large) that wait for the FS
corruption problem to be cleared up. Mixing useless changes does not help
track such a serious problem, in my opinion.

Among the diff between 2.2.9 and 2.2.10-ac10 that involve ncr/sym drivers,
only the 2 lignes that address Alpha are fixes. Other ones are not fixes
at all and so should have been defferred.

Regards,
Gérard.


On Sun, 18 Jul 1999, Ingo Molnar wrote:

> On Sun, 18 Jul 1999, Gerard Roudier wrote:
>
> > I just found the following change in the ncr53c8xx driver:
> >
> > + if (lp && time_before_eq(lp->tags_stime + 3*HZ, jiffies)) {
> > - if (lp && lp->tags_stime + (3*HZ) <= jiffies) {
> >
> > I do prefer a known bug that such a _not_ needed ambiguous construct, for
> > the reason that, each time the arguments of time_before_eq() are
> > _unfortunately_ in the reverse order we just get the opposite effect.
>
> [i did not do that change, but] i personally do not find it hard to read.
> It's the same, standard two-argument operator thing we use in many other
> cases, like memcpy():
>
> operator(x,y) := x operator y
>
> in the above case:
>
> time_before_eq(x,y) == x time_before_eq y
>
> ie. 'x is before or equal to y'. What we'd like to have is an overloaded
> '<' and '==' operator for time-type - but it's not that hard in C via
> using function-syntax either. Just like memcpy(x,y) is easier to read via
> an overloaded assignment operator, but it's not that hard to not mess it
> up in it's current form either.
>
> > The constructs I would have accepted looks like the following, for
> > example:
> >
> > Either,
> > + if (lp && time_before_jiffies_eq(lp->tags_stime + 3*HZ)) {
> > Or,
> > + if (lp && not_in_the_future(lp->tags_stime + 3*HZ)) {
> >
> > (And define appropriate macros if needed)
>
> i agree, such things makes it more readable. The two fundamental operators
> are:
>
> in_the_future()
> now()
>
> in_the_past(x) := !now(x) && !in_the_future(x)
>
> -- mingo

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/