Re: [PATCH v4 5/7] mm/x86: arch_check_zapped_pud()

David Hildenbrand
2024-08-08T20:45:18Z

>>> +void arch_check_zapped_pud(struct vm_area_struct *vma, pud_t pud)
>>> +{
>>> + /* See note in arch_check_zapped_pte() */
>>> + VM_WARN_ON_ONCE(!(vma->vm_flags & VM_SHADOW_STACK) &&
>>> + pud_shstk(pud));
>>
>> Please get rid of the line break. You have 100 characters.
>
> Coding-style.rst still tells me 80 here:
>
> The preferred limit on the length of a single line is 80 columns.
>
> Statements longer than 80 columns should be broken into sensible chunks,
> unless exceeding 80 columns significantly increases readability and does
> not hide information.
>
> Maybe this just changed very recently so even not in mm-unstable?
>
> I'll fix the two line-wrap in this patch anyway, as I figured these two
> cases even didn't hit 80.. probably because I used fill-column=75 locally..
>
> But still I'll probably need to figure it out for other spots. Please help
> me to justify.

My interpretation is (the doc is not completely clear to me as well, but
checkpatch.pl hardcodes the max_line_length=100) that we can happily use
up to 100 chars.

I also tend to stay within 80 chars, unless really reasonable. Years of
Linux kernel hacking really taught my inner self to not do it.

Here I would agree that having the VM_WARN_ON_ONCE in a single would aid
readability.

An example where 100 chars are likely a bad idea would be when nesting
that deeply such that most lines start exceeding 80 chars. We should
rather fix the code then -- like the coding style spells out :)

--
Cheers,

David / dhildenb