Re: [PATCH] Revert Intel CET changes to __jmp_buf_tag (Bug 22743)

[Carlos O'Donell <carlos at redhat dot com>]

On 01/25/2018 08:40 AM, H.J. Lu wrote:
> On Thu, Jan 25, 2018 at 8:36 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>> On 01/25/2018 08:28 AM, H.J. Lu wrote:
>>> On Thu, Jan 25, 2018 at 8:22 AM, Zack Weinberg <zackw@panix.com> wrote:
>>>> On Thu, Jan 25, 2018 at 10:33 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> On Thu, Jan 25, 2018 at 6:55 AM, Zack Weinberg <zackw@panix.com> wrote:
>>>>>> In my opinion, the fact that you two are having this argument
>>>>>> reinforces Carlos' position: the original patch should be reverted and
>>>>>> we should figure out what to do in 2.28 when we're not under time
>>>>>> pressure.  HJ, do you have some concrete external reason why you must
>>>>>> have this new feature in 2.27?  If so, please tell us what it is.  To
>>>>>> me it doesn't seem urgent.
>>>>>
>>>>> My question is if we are going to fix it at all.  If yes, why not 2.27.
>>>>> Both approaches are opaque to users.  They can't tell the difference.
>>>>
>>>> My concerns are entirely based on timing: specifically, you seem to be
>>>> in a rush to squeak under the 2.27 deadline.  Rushing leads to
>>>> mistakes.
>>>
>>> The main issue for this one is testcase.  Once a testcase is found, we
>>> know how to avoid the issue.
>>>
>>>> This seems like the sort of thing that could reasonably be backported
>>>> to the release branch(es) ... *after* we have calmly, without rushing,
>>>> figured out the correct fix in mainline.
>>>>
>>>
>>> I am fine with reverting my patch only on 2.27 branch, not on master.
>>
>> This does not make sense. The revert on master would last for as long as
>> you have to come up with a patch that works and everyone accepts and has
>> consensus.
> 
> We have 2 proposals, one with a patch and one without.  How long
> should it take to make a decision?

However long it takes.

Until then we revert the patches.

>> You checked these patches in without consensus, and instead of waiting
>> or pinging for review, you checked them in.
>>
>> For x86_64 there is no machine maintainer, it requires community consensus,
> 
> Do we need/want machine maintainers for x86-64 (i386)?

No. We need people to follow consensus rules, ping patches, and ask for review.

Without review we are going to increase the risk of defects going in to the port.

The point of review is to lower defect rates and attain better solutions.

>> the port is too important not to get serious community review.
>>
>> They changes had negative ABI consequences, and now you have several
>> people interested in making sure future patches don't break ABI.
> 
> There are no arguments here.
> 
>> You have drawn attention to this work and now you have to reach consensus
>> on a solution for a primary architecture which is very important to all of
>> us in the downstream distributions. More time is required to make these
>> patches work.
>>
>> I see no clear argument for why this needs to be in 2.27.
> 
> That is fine with me.
> 
>> I will be reverting the patches in the next 8 hours.
> 
> We need this on master so that we can work on CET support.

No. You do not *need* anything on master, you can work on your own branches
with all of your collaborators.

Master is for work that is complete, has consensus, and is ready to be
delivered into a public release for which we will promise ABI guarantees.

-- 
Cheers,
Carlos.