[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [pvops-dom0] Adding MCA logging support in pv_ops
On 08/06/09 19:32, Ke, Liping wrote: > Hi, Jeremy and all > > This is the new formated patch and I have tried linux patch format check > tools on it. > Thanks. I have a couple of little comments: * when referring to Xen hg changesets, use the hex changeset ID rather than the number, since the number only makes sense locally (ie, your change 902 may not match anyone else's) * don't put "dom0" in the driver filename; drivers/xen/mce.c is enough. Same with the various symbols and messages mentioning dom0. * don't use "#if define" if #ifdef would do; but try to avoid #ifdef anyway. In particular, this *definitely* shouldn't be duplicating code between the #if and #else blocks. At the very least, you should just need one #ifdef to re-add the MCE/MCA flags back into the mask. +#if !defined(CONFIG_XEN_MCE) cpuid_leaf1_edx_mask = ~((1 << X86_FEATURE_MCE) | /* disable MCE */ (1 << X86_FEATURE_MCA) | /* disable MCA */ (1 << X86_FEATURE_PAT) | /* disable PAT */ (1 << X86_FEATURE_ACC)); /* thermal monitoring */ - +#else + cpuid_leaf1_edx_mask = + ~((1 << X86_FEATURE_PAT) | /* disable PAT */ + (1 << X86_FEATURE_ACC)); /* thermal monitoring */ +#endif Also, if you include the patch inline it makes it easier to review and comment on. Thanks, J _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |