Re: PATCH for 2.3.29: block device setup cleanup.

Andries.Brouwer@cwi.nl
Fri, 3 Dec 1999 22:07:28 +0100 (MET)


> I hope you understand my intentions a bit better now?

Yes, I see what you want.
But I am unhappy with your work. Look again at

> - read_ahead[MAJOR(dev)] = arg;
> + blk_dev[MAJOR(dev)].read_ahead = arg;

If the final goal is

dev->majordev->read_ahead = arg;

or so, then it seems to me you do not make progress.
On the other hand, if you do

- read_ahead[MAJOR(dev)] = arg;
+ READ_AHEAD(dev) = arg;

then you can have today

#define READ_AHEAD(x) read_ahead[MAJOR(x)]

and tomorrow

#define READ_AHEAD(x) (x)->majordev->read_ahead

(And this may be further hidden inside functions get_read_ahead()
and set_read_ahead(), so that generic versions of such functions
can be replaced by more interesting ones for certain devices, etc.)

Such changes are real progress, because (i) they generate the same
code as we have today, so no errors can be introduced, not even
any inefficiencies, and (ii) using a 1-line change this READ_AHEAD()
suddenly gets its new meaning.

It is this same mechanism that allows one today to switch between

#define MINOR(x) ((x) & MINORMASK)

and

#define MINOR(x) ((x)->minor)

In other words, your current approach requires changes all over
the place to reach an intermediate stage, and then again changes
all over for the final work. While the version I would suggest
requires a change only once, and people running the kernel with
one set of defines can coexist with people running the kernel
with another set of defines.

This allows one to play with several possible ways to setup
device structs for free, without having to do major editing.

And Linus might accept such a change even during code-freeze
times since the generated code remains the same.

Andries

-
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/