Re: [RFC/PATCH] Per-device parameter support (13/16)

[Rusty Russell <rusty@rustcorp.com.au>]

On Sat, 2004-10-23 at 13:31 +0900, Tejun Heo wrote:
> +/* For some reason, gcc aligns structures which contain other
> +   structures to 32-byte boundary even when __alignof__() the strucure
> +   is smaller than that.  As we're gonna assemble paramset array
> +   manually, we need to set alignment explicitly.  Also, we put all
> +   paramset array elements into .data.1 to avoid intervening variables
> +   of different types (kparam_string, kparam_array for now). */

You need __attribute__((aligned(sizeof(void *))): see moduleparam.h.
The padding between structures is arch-dependent (x86-64 found out the
hard way when we changed kernel_param previously).  Not sure why you're
using a separate section here anyway.

> +/* Bit flag */
> +#define __DEVICE_PARAM_FLAG(Name, Field, Flag, Dfl, Inv, Perm, Desc)	\
> +	param_check_uint(__devparam_data(Name),				\
> +			 &(((__devparam_type *)0)->Field));		\
> +	static struct kparam_flag __devparam_data(__param_flag_##Name)	\
> +	__devparam_extra_section = { 0, Flag, Inv };			\
> +	__DEVICE_PARAM_CALL_RANGED(Name, param_set_flag, param_get_flag,\
> +		&__devparam_data(__param_flag_##Name),			\
> +		sizeof(struct kparam_flag), 1, 0, Dfl, 0, Perm, Desc,	\
> +		offsetof(struct kparam_flag, pflags),			\
> +		offsetof(__devparam_type, Field), -1, -1)
> +
> +#define DEVICE_PARAM_FLAG(Name, Field, Flag, Dfl, Perm, Desc)		\
> +	__DEVICE_PARAM_FLAG(Name, Field, Flag, Dfl, 0, Perm, Desc)

Haven't read closely, but why is FLAG special?

I think your macros might be better off always having the range, which
would reduce the number of functions..  I like including the description
in the macros: that's an improvement.

I always disliked the _NAMED versions (if it's a good name for users,
it's usually a good name for the variable and vice versa), but you might
want to consider always having it if that's the common case.

Cheers,
Rusty.
-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman