Re: [PATCH] powerpc/xmon: Fix an unexpected xmon onoff state change

[Guilherme G. Piccoli]

On 02/17/2017 07:30 AM, Pan Xinhui wrote:
> 
> 
> 在 2017/2/17 14:05, Michael Ellerman 写道:
>> Pan Xinhui <xinhui@xxxxxxxxxxxxxxxxxx> writes:
>>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>>> index 9c0e17c..f6e5c3d 100644
>>> --- a/arch/powerpc/xmon/xmon.c
>>> +++ b/arch/powerpc/xmon/xmon.c
>>> @@ -76,6 +76,7 @@ static int xmon_gate;
>>>   #endif /* CONFIG_SMP */
>>>
>>>   static unsigned long in_xmon __read_mostly = 0;
>>> +static int xmon_off = !IS_ENABLED(CONFIG_XMON_DEFAULT);
>>
>> I think the logic would probably clearer if we invert this to become
>> xmon_on.
>>
> yep, make sense.
> 
>>> @@ -3266,16 +3269,16 @@ static int __init setup_xmon_sysrq(void)
>>>   __initcall(setup_xmon_sysrq);
>>>   #endif /* CONFIG_MAGIC_SYSRQ */
>>>
>>> -static int __initdata xmon_early, xmon_off;
>>> +static int __initdata xmon_early;
>>>
>>>   static int __init early_parse_xmon(char *p)
>>>   {
>>>   	if (!p || strncmp(p, "early", 5) == 0) {
>>>   		/* just "xmon" is equivalent to "xmon=early" */
>>> -		xmon_init(1);
>>>   		xmon_early = 1;
>>> +		xmon_off = 0;
>>>   	} else if (strncmp(p, "on", 2) == 0)
>>> -		xmon_init(1);
>>> +		xmon_off = 0;
>>
>> You've just changed the timing of when xmon gets enabled for the above
>> two cases, from here which is called very early, to xmon_setup() which
>> is called much later in boot.
>>
>> That effectively disables xmon for most of the boot, which we do not
>> want to do.
>>
> Although it is not often that kernel got stucked during boot. Yes, the behavior changed anyway. Will fix that in v3.

Pan/Michael, I'm working my patches on top of Pan's. So, I sent his V2
on my series, as patch #1.

Guess the workflow is better/easier if we can work the patches on the
series exclusively, since each time Pan's patch is changed, I need to
refactor my patches.

Pan, if possible send your V3 to me, I'll refactor my series on top of
it and send again on the list. Or if you or Michael have better
suggestions of workflow, let me know.

Thanks,


Guilherme

> 
>> cheers
>>
>