lkml.org 
[lkml]   [2017]   [Dec]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 2/2] x86-64/Xen: eliminate W+X mappings
From
Date
On 12/12/17 11:48, Jan Beulich wrote:
>>>> On 12.12.17 at 11:38, <mingo@kernel.org> wrote:
>> * Jan Beulich <JBeulich@suse.com> wrote:
>>> --- 4.15-rc3/arch/x86/xen/mmu_pv.c
>>> +++ 4.15-rc3-x86_64-Xen-avoid-W+X/arch/x86/xen/mmu_pv.c
>>> @@ -1902,6 +1902,16 @@ void __init xen_setup_kernel_pagetable(p
>>> /* Graft it onto L4[511][510] */
>>> copy_page(level2_kernel_pgt, l2);
>>>
>>> + /* Zap execute permission from the ident map. Due to the sharing of
>>> + * L1 entries we need to do this in the L2. */
>>
>> please use the customary (multi-line) comment style:
>>
>> /*
>> * Comment .....
>> * ...... goes here.
>> */
>>
>> specified in Documentation/CodingStyle.
>
> I would have but didn't because all other comments in this function
> use this (wrong) style. I've concluded that consistency is better
> here than matching the style doc. If the Xen maintainers tell me
> otherwise, I'll happily adjust the patch.

Yes, please use the correct style with new comments.

>
>>> + if (__supported_pte_mask & _PAGE_NX)
>>> + for (i = 0; i < PTRS_PER_PMD; ++i) {
>>> + if (pmd_none(level2_ident_pgt[i]))
>>> + continue;
>>> + level2_ident_pgt[i] =
>>> + pmd_set_flags(level2_ident_pgt[i], _PAGE_NX);
>>
>> So the line break here is quite distracting, especially considering how similar it
>> is to the alignment of the 'continue' statement. I.e. visually it looks like
>> control flow alignment.
>>
>> Would be much better to just leave it a single page and ignore checkpatch
>> here.
>
> Again I'll wait to see what the Xen maintainers think. I too dislike
> line splits like this one, but the line ended up quite a bit too long,
> not just a character or two. I also wasn't sure whether splitting
> between the function arguments would be okay, leaving the first
> line just slightly too long.

That would result in a 80 character line, which IMHO is the best choice
here.


Juergen

\
 
 \ /
  Last update: 2017-12-12 14:15    [W:0.094 / U:1.568 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site