Re: [Xen-devel] [PATCH 1/2] xen/mm: fold PGC_broken into PGC_state bits

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.03.2020 14:54, David Woodhouse wrote:
> On Thu, 2020-03-19 at 12:59 +0100, Jan Beulich wrote:
>>> Read that message again from the point of view of a contributor.
>>> Pretend it isn't even me; pretend it's someone attempting to make
>>> their first, trivial, improvement to the Xen ecosystem.
>>>
>>> I hope you'll understand why my initial reaction was just a
>>> monosyllabic 'no'.
>>
>> To be honest- no, I don't. I didn't say "no way".
> 
> Then you have completely missed my point about how subtly understating
> your 'objections' makes no difference at all to the outcome.
> 
> But OK, I'll come back to that at the end. You have made your intent
> clear, more than once now, and we should take it on board.
> 
>>  Instead I asked back to see whether there's more background to this.
>> It is a useful piece of information to know that -MP post-dates -MD
>> by 10 or more years. It's still speculation of why a new option was
>> added rather than making this default behavior, but I feel less
>> afraid of the change this way than by an implied "this not going to
>> do any harm" without really being certain why there is a separate
>> option in the first place (and gcc doc also not saying anything to
>> this effect).
> 
> It is not my job to make you feel less afraid of change.
> 
>> I can certainly follow your sentiment, not the least because
>> especially in my early days I also frequently got back replies I
>> didn't like, in various projects. Yet in a case like this one I'm
>> afraid it is not the reviewer's job to point out the unsafety of
>> a change, but it's the submitter who has to (sufficiently) prove
>> that a change won't break anything. 
> 
> I'm sure you didn't mean it as such, Jan, but FYI that response could
> be construed as being fairly patronising. If you were to direct it
> towards someone who's even remotely inclined to feeling patronised,
> that is. :)

I certainly didn't mean to, I apologize. (My dictionary gives me
several very different meanings of "patronize", so I'm somewhat
guessing which meaning you infer here.)

>> Yes, in the typical case, when there's a recognizable bug, the
>> reviewer would point this out. But there are cases where there's no
>> obvious bug, but experience (and, as so often, insufficient
>> documentation) tells one to be wary of changes of, possibly, any
>> kind.
> 
> I find this response to be purely obstructive and unhelpful.

I'm sorry if it feels like this to you.

> Your
> response to my patch was basically asking me to prove a negative, and I
> find myself surprised and disappointed that you cannot acknowledge
> that. I didn't think our viewpoints were really that far apart; perhaps
> I was wrong.

I'm certainly willing to acknowledge that I've asked a question
that may be difficult if possible at all to answer in a way that
we'd be fully certain in the end. Yet even after all of the
discussion we've had here I still think the question was
appropriate to ask. It continues to be unobvious to me that non-
default behavior of a tool would imply using this behavior is
going to be free of side effects. The historical aspect you've
dug out afterwards is at least a partial explanation which,
seeing that you've got an unconditional and a conditional ack,
is good enough for me to let the change go in, despite still
not being finally convinced of it being free of side effects.

> If there was an actual bug — or even the suspicion of a bug — I could
> understand it. But this is just voodoo "we're too scared to change
> things because we don't understand".

Not just this, but also because things had been broken in subtle
ways in the past. Until we get a better one, we have to live with
the build system being fragile here and there.

> We are better than that. You can be better than that.
> 
> But I will take on board your comments about understatement and the
> fact that you hadn't actually said "no". In future I shall consider
> merely ignoring such interjections unless you explicitly state that you
> are blocking the acceptance of a patch. Or, I suppose, resorting to the
> style of monosyllabic answer that I had originally given in this case.
> 
> I trust that maintainers will take that on board too, and that open
> "questions" from you in a thread will not be considered sufficient
> reason not to merge a patch.
> 
> That seems to be what you're saying is your intent, yes? 

My intent was to get clarification before the patch would go in.
I didn't mean to block it, but I also didn't see it go in without
such clarification. I'm struggling to see what's bad in asking
whether you/we are certain enough that a change won't have bad
side effects; if there were, we might treat an easy to work around
situation by one hard to recognize and address. Seeing you reply
just "no" seemed a fair answer to me (while I sensed a certain
level of annoyance), albeit not one that would resolve the
question. In anticipation I did include anyone else who might
know right away. Had I known the answer myself, I of course
wouldn't have asked.

Bottom line - when I say "no", I mean "no". When I ask a question
I expect it to be resolved, at least to a reasonable degree. When
I say "I wonder" I indeed mean just that; to me "may I suggest to
consider as an alternative" is simply more words, which may again
be an effect of English not being my native language. And when I
say "ack", I mean "ack". (I also didn't think I made any comments
about understatement; it was you who brought up that [cultural]
aspect.)

I'm afraid as a result of this discussion I'm now more confused
as to finding common grounds than I was before.

Jan

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