Date: Thu, 29 Jun 2017 15:52:42 -0700 (PDT)
From: Palmer Dabbelt <palmer@dabbelt.com>
To: james.hogan@imgtec.com
CC: peterz@infradead.org
CC: mingo@redhat.com
CC: mcgrof@kernel.org
CC: viro@zeniv.linux.org.uk
CC: sfr@canb.auug.org.au
CC: nicolas.dichtel@6wind.com
CC: rmk+kernel@armlinux.org.uk
CC: msalter@redhat.com
CC: tklauser@distanz.ch
CC: will.deacon@arm.com
CC: paul.gortmaker@windriver.com
CC: linux@roeck-us.net
CC: linux-kernel@vger.kernel.org
CC: linux-arch@vger.kernel.org
CC: albert@sifive.com
Subject: Re: [PATCH 5/9] RISC-V: Task implementation
In-Reply-To: <20170628233254.GR6973@jhogan-linux.le.imgtec.org>
Message-ID: <mhng-edeb5d3c-7b5f-4314-982c-f64e44e55154@palmer-si-x1c4>
Mime-Version: 1.0 (MHng)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1552
Lines: 50

On Wed, 28 Jun 2017 16:32:55 PDT (-0700), james.hogan@imgtec.com wrote:
> On Wed, Jun 28, 2017 at 11:55:34AM -0700, Palmer Dabbelt wrote:
>> diff --git a/arch/riscv/include/asm/kprobes.h b/arch/riscv/include/asm/kprobes.h
>> new file mode 100644
>> index 000000000000..1190de7a0f74
>> --- /dev/null
>> +++ b/arch/riscv/include/asm/kprobes.h
>> @@ -0,0 +1,22 @@
> ...
>> +#ifdef CONFIG_KPROBES
>> +#error "RISC-V doesn't skpport CONFIG_KPROBES"
>> +#endif
>
> I'm wondering where your fallback definition of e.g. NOKPROBE_SYMBOL
> comes from then.
>
> Could you just use the asm-generic one?

I believe so.

>> diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
>> new file mode 100644
>> index 000000000000..b13d3ea3bf79
>> --- /dev/null

> ...
>> +void show_regs(struct pt_regs *regs)
>> +{
>> +	show_regs_print_info(KERN_DEFAULT);
>> +
>> +	printk(KERN_CONT "sepc: " REG_FMT " ra : " REG_FMT " sp : " REG_FMT "\n",
>> +		regs->sepc, regs->ra, regs->sp);
>
> I've noticed inconsistent use of pr_* and printk(KERN_* in this
> patchset. Maybe now would be the best time to switch everything to pr_*.

I went through and fixed them all.

>> +	/* Reset FPU context
>> +	 *	frm: round to nearest, ties to even (IEEE default)
>> +	 *	fflags: accrued exceptions cleared
>> +	 */
>
> Similarly lots of multiline comments which don't follow the standard
> style in Documentation/process/coding-style.rst. Maybe now is the best
> time to convert if you're going to.

Someone else found one and suggested this, it's on my TODO list.

Thanks!