lkml.org 
[lkml]   [2012]   [May]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [Xen-devel] [PATCH 1/3] xen/mce: Add mcelog support for Xen platform (v2)
Date
Konrad Rzeszutek Wilk wrote:
> On Fri, May 25, 2012 at 05:56:56PM +0000, Liu, Jinsong wrote:
>> Konrad Rzeszutek Wilk wrote:
>>>>>>>> -static struct miscdevice mce_chrdev_device = {
>>>>>>>> +struct miscdevice mce_chrdev_device = {
>>>>>>>> MISC_MCELOG_MINOR,
>>>>>>>> "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).
>>>
>>> Won't the second registration (so the original one) of the major
>>> fail? So the mce_log would just error out since somebody already
>>> registered?
>>
>> No, that would be device confliction, the 2nd register return as
>> -EBUSY and un-predicetable result.
>
> And the existing code does not actually check the 'misc_register'
> return value? Ah yes. Perhaps then a fix to
> arch/x86/kernel/cpu/mcheck/mce.c to do the proper de-registration if
> 'misc_register' fails?

It's weird. From code point of view, it indeed not check return value so it should go silently. mce.c didn't do misc_deregister.

>
> Or just set 'mce_disabled=1' in the bootup of Xen, similar to
> how lguest.c does it?
>
>> I test it in your way, mcelog fail to fetch any error log.
>
> And that is b/c of? What exactly?

I don't know exactly what's the reason, but test again still fail.

Thanks,
Jinsong




\
 
 \ /
  Last update: 2012-05-25 21:41    [W:0.178 / U:1.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site