[lkml]   [2012]   [May]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRE: [PATCH 1/3] xen/mce: Add mcelog support for Xen platform (v2)
Borislav Petkov wrote:
> On Thu, May 24, 2012 at 10:10:34AM +0000, Liu, Jinsong wrote:
>>>> Signed-off-by: Liu, Jinsong <>
>>>> Signed-off-by: Ke, Liping <>
>>>> Signed-off-by: Jiang, Yunhong <>
>>>> Signed-off-by: Jeremy Fitzhardinge <>
>>> If you're sending the patch, you need to be the last on the
>>> SOB-chain and the SOB-chain has to show the proper path the patch
>>> took. See <Documentation/SubmittingPatches>.
>> Thanks! I will update it. But I'm not quite clear 'the SOB-chain has
>> to show the proper path the patch took', would you elaborate more?
> Section 12 in the SubmittingPatches readme has more or less an example
> about it, here's what I mean:
> Signed-off-by: Initial Patch Author <>
> Signed-off-by: Second Patch Author <>
> Signed-off-by: Third Patch Author <>
> ...
> Signed-off-by: Patch Submitter <>
> Some of the lines above may or may not be present depending on your
> case. If you're sending the patch, you're the last in chain so your
> SOB should be last.
> That's what I mean with "proper path" the patch took, i.e. the SOB
> chain should show how exactly this patch was created and who handled
> it on its way upstream.

Oh, I see your meaning now. Thanks for telling me kernel habit / culture about SOB. I will update accordingly.

>>>> -static struct miscdevice mce_chrdev_device = {
>>>> +struct miscdevice mce_chrdev_device = {
>>>> "mcelog",
>>>> &mce_chrdev_ops,
>>> You're still reusing those - pls, define your own 'struct miscdevice
>>> mce_chrdev_device' in drivers/xen/ or somewhere convenient and
>>> your own mce_chrdev_ops. The only thing you should be touching in
>>> arch/x86/.../mcheck/ is the export of MISC_MCELOG_MINOR.
>>> Thanks.
>> I'm *not* reuse native code.
>> I have defined 'struct miscdevice xen_mce_chrdev_device' in
>> drivers/xen, and I also implement xen_mce_chrdev_ops, they are all
>> xen-self-contained.
>> The patch just redirect native mce_chrdev_device to
>> xen_mce_chrdev_device when running under xen environment.
>> It didn't change any native code (except just cancel
>> mce_chrdev_device 'static'), and will not break native logic.
> Why are you doing that?
> Why don't you do
> misc_register(&xen_mce_chrdev_device);
> in xen_early_init_mcelog() ?
> This way there'll be no arch/x86/ dependencies at all.

The reason is, if we do so, it would be covered by native misc_register(&mce_chrdev_device) later when native kernel init (xen init first and then start native kernel).
Under such case, if linux running under xen platform, /dev/mcelog point to vcpu, that's pointless since it cannot get any mcelog from physical cpu (which owned by xen).

Yes, we can use another misc device like /dev/xen-mcelog, w/ another device minor like 226, but that's not good for userspace mcelog tools. As far as I know, Novell mcelog use unified /dev/mcelog interface for linux running under either bare metal or xen platform.

This patch just do redirection at xen code path, and that would not hurt anything to native kernel.


 \ /
  Last update: 2012-05-24 20:02    [W:0.221 / U:5.432 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site