Re: [PATCH 02/16] GFS2: Core locking interface

[Ingo Molnar]

* Jan Engelhardt <jengelh@xxxxxxxxxxxxxxx> wrote:

> I suppose so. If they were initialized statically, this function could 
> possibly be dropped.
> 
> >+typedef void lm_lockspace_t;
> >+typedef void lm_lock_t;
> >+typedef void lm_fsdata_t;
> 
> Try to avoid typedefs for
> - simple types like these (int/void/etc.)
> - structures

yeah. If we dont want to expose a type externally, we forward declare 
the structure, and pointers to it can then be used and passed around. 
That's also more type-safe (and obviously more readable) than a typedef 
to void.

> >+		error = glock_wait_internal(gh);
> >+		if (error == GLR_CANCELED) {
> >+			msleep(100);
> 
> msleep is a busy-waiter IIRC. Really want to do that - what about some 
> schedulling?

no. mdelay() is the busy-waiter - msleep() is scheduling based.

> >+			borked = 1;
> >+			serious = error;
> 
> This got me a laugh :)

me too - the hidden joys of code review :-)

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