Re: [Xen-devel] [PATCH v3 1/2] xen/link: Introduce .bss.percpu.page_aligned

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 30/07/2019 10:49, Jan Beulich wrote:
> On 30.07.2019 11:35, Andrew Cooper wrote:
>> On 30/07/2019 09:42, Jan Beulich wrote:
>>> On 29.07.2019 19:38, Andrew Cooper wrote:
>>>> --- a/xen/arch/x86/xen.lds.S
>>>> +++ b/xen/arch/x86/xen.lds.S
>>>> @@ -293,14 +293,17 @@ SECTIONS
>>>>          __bss_start = .;
>>>>          *(.bss.stack_aligned)
>>>>          *(.bss.page_aligned*)
>>>> -       *(.bss)
>>>> -       . = ALIGN(SMP_CACHE_BYTES);
>>>> +       . = ALIGN(PAGE_SIZE);
>>>>          __per_cpu_start = .;
>>>> +       *(.bss.percpu.page_aligned)
>>>> +       . = ALIGN(PAGE_SIZE);
>>> But this goes too far: What we want is for the TSS to occupy a full
>>> page, not for whatever random other page-aligned object ends up
>>> last here (should any every appear).
>> Come again?  This is ridiculous.
>>
>> Objects in a section following foo.page_aligned should never end up in
>> the tail of the final page of foo.page_aligned, because then they are in
>> the wrong section.
> How do you derive "wrong section"? Sections have an alignment and a
> size. The latter doesn't need to be a multiple of the former. The
> section ends wherever its size says it ends. Using this property is
> actually desirable in other cases, to limit waste of space.

The principle of least surprise, for a section with page_aligned in its
name, is that the section starts and ends on a page boundary.  This is
what people expect when they see a name like that.

It really doesn't matter what is technically possible.  What does matter
is peoples reasonable expectations.

>> A short TSS is a pain to deal with, but even you said you didn't like
>> the xen_tss idea when you suggested it.
>>
>> The name of the section is very deliberately not TSS specific, because
>> there is plenty of other cleanup which will end up with objects in this
>> section.
> Well, if they're all strictly a multiple of PAGE_SIZE, then writing
> down a respective requirement might be acceptable. But even then Inot 
> submitted by me.
> wouldn't be overly happy going that route.

This reply, like most others in this thread, is actively unhelpful, and
I give up.

I can't read your mind.  Neither can anyone else, and noone has the time
to divine what you want.

If you don't come up with something more helpful than "I don't like it
this way", then I'm going to commit this series in its current form, and
you can adjusting it to your own taste, in your own time.

This goes for other series as well, including ones submitted by others. 
It is absolutely critical that review feedback identifies, in a clear
manner, how you expect the issue to be resolved.

For any non-trivial piece of feedback, if it can't be phrased in the
form "I would this patch ok if you alter $X to $Y", then it is probably
wants rethinking.  Whatever the feedback actually is, that gives a
concrete $X which is the perceived problem, and a concrete $Y which is
either a clear discussion point, or a clear direction to work towards.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel