lkml.org 
[lkml]   [2022]   [Oct]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] LoongArch: Add unaligned access support
From
Hi,

Just my 2c...

On 2022/10/17 15:38, Arnd Bergmann wrote:
> On Mon, Oct 17, 2022, at 9:31 AM, Huacai Chen wrote:
>> Hi, Arnd,
>>
>> On Mon, Oct 17, 2022 at 3:12 PM Arnd Bergmann <arnd@arndb.de> wrote:
>>>
>>> On Sun, Oct 16, 2022, at 3:34 PM, Huacai Chen wrote:
>>>> Loongson-2 series (Loongson-2K500, Loongson-2K1000) don't support
>>>> unaligned access in hardware, while Loongson-3 series (Loongson-3A5000,
>>>> Loongson-3C5000) are configurable whether support unaligned access in
>>>> hardware. This patch add unaligned access emulation for those LoongArch
>>>> processors without hardware support.
>>>>
>>>> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
>>>
>>> What does the Loongarch ELF ABI say about this? On most architectures,
>>> C compilers are not allowed to produce unaligned accesses for standard
>>> compliant source code, the only way you'd get this is when casting
>>> a an unaligned (e.g. char*) pointer to another type with higher alignment
>>> requirement.
>> Some unaligned accesses are observed from the kernel network stack, it
>> seems related to whether the packet aligns to IP header or MAC header.
>
> This is usually a bug in the device driver. It's a fairly common bug
> since the network driver has to ensure the alignment is correct, but
> it's usually fixable, and fixing it results in better performance on
> machines that support unaligned access as well.
>
> Which driver did you observe this with?

I agree with Arnd that it's probably better to fix the drivers. Having
the debug feature would help, but in the end it's still the drivers that
should get the fix. For example I have previously fixed one such
unaligned access in iwlwifi when I was tinkering with a Loongson 3A4000,
it was pretty easy to spot with the right perf tools.

>
>> And, gcc has a -mstrict-align parameter, if without this, there are
>> unaligned instructions.
>
> Does this default to strict or non-strict mode? Usually gcc does not
> allow to turn this off on architectures that have no hardware support
> for unaligned access.

The LoongArch gcc behavior is tunable via the "-m[no-]strict-align"
command-line flag, and I believe gcc defaults to producing the
"non-strict" code, most likely because the most popular LoongArch model
(the 3A5000) supports efficient unaligned accesses. Also there's always
the possibility that code compiled for and tested on e.g. 3A5000 will
get run on the less capable models, so it's arguably desirable to not
let those just fail.

Yes it's vendors' responsibility to actually test their code/solution
and observe the failure early, but things happen and I'm actually not
sure if not doing the emulation will benefit the users at this point...

>
>>>> +/* sysctl hooks */
>>>> +int unaligned_enabled __read_mostly = 1; /* Enabled by default */
>>>> +int no_unaligned_warning __read_mostly = 1; /* Only 1 warning by default */
>>>
>>> The comment says 'sysctl', the implementation has a debugfs interface.
>> Originally "enabled", "warning" and "counters" are all debugfs
>> interfaces, then you told me to use sysctl. Now in this version
>> "enabled" and "warning" are converted to sysctl, but there are no
>> existing "counters" sysctl.
>
> I don't see the sysctl interface in the patch, what am I missing?

FYI they are chosen by the Kconfig options and live in kernel/sysctl.c.
And I believe the debugfs interface (the counters) is inspired by the
original mips code. Pretty niche use case but can be handy at times...

--
WANG "xen0n" Xuerui

Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/

\
 
 \ /
  Last update: 2022-10-17 10:06    [W:0.995 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site