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

From: Rusty Russell
Date: Mon Oct 25 2004 - 01:09:47 EST


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

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