Re: [PATCH] riscv: uaccess: Allow the last potential unrolled copy

From: Ben Dooks <ben.dooks@codethink.co.uk>
To: Alexandre Ghiti <alex@ghiti.fr>,
	Xiao Wang <xiao.w.wang@intel.com>,
	paul.walmsley@sifive.com, palmer@dabbelt.com,
	aou@eecs.berkeley.edu
Cc: jerry.shih@sifive.com, nick.knight@sifive.com,
	ajones@ventanamicro.com, bjorn@rivosinc.com,
	andy.chiu@sifive.com, viro@zeniv.linux.org.uk,
	cleger@rivosinc.com, alexghiti@rivosinc.com,
	haicheng.li@intel.com, akira.tsukamoto@gmail.com,
	linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] riscv: uaccess: Allow the last potential unrolled copy
Date: Fri, 3 May 2024 13:19:47 +0100	[thread overview]
Message-ID: <4bc238eb-410b-46b1-98e1-4aaa35e34404@codethink.co.uk> (raw)
In-Reply-To: <7ebc5b28-7115-494f-a607-e46c71214cce@ghiti.fr>

On 03/05/2024 13:16, Alexandre Ghiti wrote:
> Hi Xiao,
> 
> On 13/03/2024 11:33, Xiao Wang wrote:
>> When the dst buffer pointer points to the last accessible aligned 
>> addr, we
>> could still run another iteration of unrolled copy.
>>
>> Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
>> ---
>>   arch/riscv/lib/uaccess.S | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S
>> index 2e665f8f8fcc..1399d797d81b 100644
>> --- a/arch/riscv/lib/uaccess.S
>> +++ b/arch/riscv/lib/uaccess.S
>> @@ -103,7 +103,7 @@ SYM_FUNC_START(fallback_scalar_usercopy)
>>       fixup REG_S   t4,  7*SZREG(a0), 10f
>>       addi    a0, a0, 8*SZREG
>>       addi    a1, a1, 8*SZREG
>> -    bltu    a0, t0, 2b
>> +    bleu    a0, t0, 2b
>>       addi    t0, t0, 8*SZREG /* revert to original value */
>>       j    .Lbyte_copy_tail
> 
> 
> I agree it is still safe to continue for another word_copy here.
> 
> Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Out of interest, has anyone checked if causing a schedule event during
this code breaks like the last time we had issues with the upstream
testing?

I did propose saving the state of the user-access flag in the task
struct but we mostly solved it by making sleeping functions stay
away from the address calculation. This of course may have been done
already or need to be done if three's long areas where the user-access
flags can be disabled (generally only a few drivers did this, so we
may not have come across the problem)

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv