xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: David Woodhouse <dwmw2@infradead.org>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Julien Grall <julien@xen.org>, Wei Liu <wl@xen.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <george.dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Jeff Kubascik <jeff.kubascik@dornerworks.com>,
	Stewart Hildebrand <stewart.hildebrand@dornerworks.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [Xen-devel] [PATCH 1/2] xen/mm: fold PGC_broken into PGC_state bits
Date: Thu, 19 Mar 2020 15:46:03 +0100	[thread overview]
Message-ID: <618f0848-5159-4de5-5586-76a7067b8b96@suse.com> (raw)
In-Reply-To: <59ee24e1b9190c59b976b559a83ede5cc96c3d03.camel@infradead.org>

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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

  reply	other threads:[~2020-03-19 14:46 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-04 15:14 [Xen-devel] [XEN PATCH v2 1/2] Check zone before merging adjacent blocks in heap Stewart Hildebrand
2020-02-04 15:14 ` [Xen-devel] [DO NOT APPLY XEN PATCH v2 2/2] Test case for buddy allocator merging issue Stewart Hildebrand
2020-02-04 15:22 ` [Xen-devel] [XEN PATCH v2 1/2] Check zone before merging adjacent blocks in heap Jan Beulich
2020-02-04 15:37   ` George Dunlap
2020-02-05  9:50     ` David Woodhouse
2020-02-05 10:02       ` Jan Beulich
2020-02-05 10:24         ` David Woodhouse
2020-02-05 10:49           ` Jan Beulich
2020-02-05 11:23             ` David Woodhouse
2020-02-05 13:37               ` Jan Beulich
2020-02-05 14:12                 ` David Woodhouse
2020-02-07 15:49                   ` David Woodhouse
2020-02-07 15:57                     ` [Xen-devel] [PATCH 1/2] xen/mm: fold PGC_broken into PGC_state bits David Woodhouse
2020-02-07 20:27                       ` Julien Grall
2020-02-09 13:22                         ` David Woodhouse
2020-02-09 17:59                           ` Julien Grall
2020-03-17 21:39                         ` David Woodhouse
2020-02-20 11:10                       ` Jan Beulich
2020-03-17 21:52                         ` David Woodhouse
2020-03-18  9:56                           ` Jan Beulich
2020-03-18 12:31                             ` Julien Grall
2020-03-18 13:23                               ` Jan Beulich
2020-03-18 17:13                               ` David Woodhouse
2020-03-19  8:49                                 ` Jan Beulich
2020-03-19 10:26                                   ` David Woodhouse
2020-03-19 11:59                                     ` Jan Beulich
2020-03-19 13:54                                       ` David Woodhouse
2020-03-19 14:46                                         ` Jan Beulich [this message]
2020-02-07 15:57                     ` [Xen-devel] [PATCH 2/2] xen/mm: Introduce PG_state_uninitialised David Woodhouse
2020-02-07 16:30                       ` Xia, Hongyan
2020-02-07 16:32                         ` David Woodhouse
2020-02-07 16:40                           ` Xia, Hongyan
2020-02-07 17:06                             ` David Woodhouse
2020-02-07 18:04                               ` David Woodhouse
2020-02-20 11:59                                 ` Jan Beulich
2020-02-20 13:27                                   ` Julien Grall
2020-03-17 22:15                                   ` David Woodhouse
2020-03-18  8:53                                     ` Paul Durrant
2020-03-18 10:10                                       ` Jan Beulich
2020-03-18 10:41                                         ` Paul Durrant
2020-03-18 11:12                                           ` Jan Beulich
2020-03-18 10:03                                     ` Jan Beulich
2020-03-18 12:11                                       ` David Woodhouse
2020-03-18 13:27                                         ` Jan Beulich
2020-02-05 10:22       ` [Xen-devel] [XEN PATCH v2 1/2] Check zone before merging adjacent blocks in heap Julien Grall
2020-02-05 10:32         ` David Woodhouse
2020-02-05 11:36         ` David Woodhouse
2020-02-04 15:37   ` Stewart Hildebrand
2020-03-19 21:17 [Xen-devel] [PATCH 0/2] Handle David Woodhouse
2020-03-19 21:21 ` [Xen-devel] [PATCH 1/2] xen/mm: fold PGC_broken into PGC_state bits David Woodhouse
2020-03-20 13:17   ` Paul Durrant

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=618f0848-5159-4de5-5586-76a7067b8b96@suse.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=dwmw2@infradead.org \
    --cc=george.dunlap@citrix.com \
    --cc=george.dunlap@eu.citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=jeff.kubascik@dornerworks.com \
    --cc=julien@xen.org \
    --cc=konrad.wilk@oracle.com \
    --cc=sstabellini@kernel.org \
    --cc=stewart.hildebrand@dornerworks.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).