Re: [PATCH linux-2.6.12-rc2-mm3] smc91c92_cs: Reduce stack usage in smc91c92_event()

[Denis Vlasenko <vda@port.imtp.ilyichevsk.odessa.ua>]

On Saturday 23 April 2005 03:12, Jörn Engel wrote:

> > 1) struct is unnamed and local to function
> > 2) Variables do not change their type, the just sit in local-> now.
> >    I can just add 'local->' to each affected variable,
> >    without "it was an object, now it is a pointer" changes.
> >    No need to replace . with ->, remove &, etc.
> 
> I'd have proposed the same, before reading further down in the patch.
> Basically, the driver is full of duplication, so the exact same struct
> can be used several times.  Therefore, the downsides of your approach
> seem to prevail.

What downsides? I must admit I do not understand your answer here.

Let me stay on the subject of converting large stack onjects
to kmalloc()ed ones, without reference to current state
of code in a particular module.

Basically, these objects are local to function. We do not
introduce struct as an 'object'. It merely aggregates
all locals we decided to move to kmalloc space,
only because it's easier that way to allocate and reference
all of them in C.

Thus, even if there are many functions with similar
set of locals, it makes little sense to declare common struct.
IOW, I wouldn't do this:

struct big {
	large_t	large;
	huge_t huge;
};

int f() {
	struct big *local;
	local = kmalloc(sizeof(big),...);
	...
}

int g() {
	struct big *local;
	local = kmalloc(sizeof(big),...);
	...
}

For one, what will happen when you will need to add
another local in one function only?

Instead, I'd do it like I described in previous mail:

int f() {
	struct {
		large_t	large;
		huge_t huge;
	} *local;
	local = kmalloc(sizeof(*local),...);
	...
}

int g() {
	struct {
		large_t	large;
		huge_t huge;
	} *local;
	local = kmalloc(sizeof(*local),...);
	...
}
--
vda

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