linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: robin.murphy@arm.com (Robin Murphy)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] arm64: fix show_regs fallout from KERN_CONT changes
Date: Fri, 21 Oct 2016 14:48:55 +0100	[thread overview]
Message-ID: <00e54ff9-e4da-c96f-47dd-f7930ac12a40@arm.com> (raw)
In-Reply-To: <20161021125439.GE16630@leverpostej>

On 21/10/16 13:54, Mark Rutland wrote:
> Hi,
> 
> On Fri, Oct 21, 2016 at 12:34:11PM +0100, Robin Murphy wrote:
>>> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
>>> index ddce61b..3f31cf93 100644
>>> --- a/arch/arm64/kernel/process.c
>>> +++ b/arch/arm64/kernel/process.c
>>> @@ -187,10 +187,19 @@ void __show_regs(struct pt_regs *regs)
>>>  	printk("pc : [<%016llx>] lr : [<%016llx>] pstate: %08llx\n",
>>>  	       regs->pc, lr, regs->pstate);
>>>  	printk("sp : %016llx\n", sp);
>>> -	for (i = top_reg; i >= 0; i--) {
>>> +
>>> +	i = top_reg;
>>> +
>>> +	while (i >= 0) {
>>>  		printk("x%-2d: %016llx ", i, regs->regs[i]);
>>> -		if (i % 2 == 0)
>>> -			printk("\n");
>>> +		i--;
>>> +
>>> +		if (i % 2 == 0) {
>>> +			pr_cont("x%-2d: %016llx ", i, regs->regs[i]);
>>> +			i--;
>>> +		}
>>> +
>>> +		pr_cont("\n");
>>>  	}
>>
>> Might it be nicer to simply do this (or thereabouts)?
> 
> I don't think so; top_reg is either 12 (for compat), or 29 (for native),
> so for the compat case, with the existing code the first line should be
> one register (r12), with r1; r0 on the final line.

Heh, the "or thereabouts" was intended to acknowledge the act of
copy-pasting patch context around in an email client without even
looking at the original code (or how it's called) ;)

The main idea was that it looks feasible to avoid pr_cont() and the
yucky floating "\n"s altogether.

>> 	for (i = top_reg; i > 1; i -= 2)
>> 		printk("x%-2d: %016llx x%-2d: %016llx\n", i-1,
>> 			regs->regs[i-1], i, regs->regs[i]);
> 
> ... whereas here the first line would be two (r12 and r11) ... 
> 
>> 	if (i > 0)
>> 		printk("x%-2d: %016llx\n", i-1, regs->regs[i-1]);
> 
> ... and then r0 on its own.
> 
> Perhaps that's fine, but it would differ from the existing behaviour,
> and make native and compat noticeably different.
>
> We could try fixing up the first line prior to the loop, but that still
> requires duplicating the format string thrice, manipulation of i, etc.
> 
> It looks like Will's taken my patch as-is, but if we can clean this up
> further it would certainly be nice.

Sure, getting it un-broken for 4.9 is the important thing. I'll take a
proper look into the refactoring idea next time I get bored.

Robin.

> 
> Thanks,
> Mark.
> 

      reply	other threads:[~2016-10-21 13:48 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-20 11:23 [PATCH] arm64: fix show_regs fallout from KERN_CONT changes Mark Rutland
2016-10-21 11:34 ` Robin Murphy
2016-10-21 12:54   ` Mark Rutland
2016-10-21 13:48     ` Robin Murphy [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=00e54ff9-e4da-c96f-47dd-f7930ac12a40@arm.com \
    --to=robin.murphy@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).