RE: [PATCH v3 1/3] aerdrv: Trace Event for AER

From: Ortiz, Lance E
Date: Mon Dec 03 2012 - 16:14:36 EST


>
> Maybe we should ask Yanmin who added those strings in
> 6c2b374d74857e892080ee726184ec1d15e7d4e4; CCed.
>
> Also, it would make a lot of sense to have those string definitions
> at one place and then reuse them instead of define them again in the
> tracepoint header and they get out of sync and have needless
> duplication
> in the kernel, etc, etc.
>
> So, instead, you could define some macros which can generate your
> strings above like this:
>
> #define aer_uncor_err_str(bitnum) \
> { BIT(bitnum), aer_uncorrectable_error_string(bitnum) }
>
> and use that macro for the __print_flags flag_array argument.
>
> Or something to that effect.
>
> Even better would it be if the error strings in
> <drivers/pci/pcie/aer/aerdrv_errprint.c> could be shared with the
> tracepoint ones. That would require a bit more changes though but
> something like using an array of trace_print_flags instead an array of
> strings could be doable.
>
> Then, when you need the string, you do uncor_err_array[i].name and so
> on.

This a fine idea. I'm thinking we might want to implement that in a later patch since it is a bigger change and we still need to see about adding additional strings. I can send out one more version of the patch with the comment header changes you suggested, and the comments Joe had and maybe we can move forward with that.

Lance

N‹§²æìr¸›yúèšØb²X¬¶ÇvØ^–)Þ{.nÇ+‰·¥Š{±‘êçzX§¶›¡Ü}©ž²ÆzÚ&j:+v‰¨¾«‘êçzZ+€Ê+zf£¢·hšˆ§~†­†Ûiÿûàz¹®w¥¢¸?™¨è­Ú&¢)ßf”ù^jÇy§m…á@A«a¶Úÿ 0¶ìh®å’i