[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-sparse
Subject:    Re: [PATCH 0/2] Fix illegal simplification of volatile stores
From:       Christopher Li <sparse () chrisli ! org>
Date:       2017-11-13 1:27:26
Message-ID: CANeU7Qn0o+_wVaq3Q9usBduq0Jx=K=Sc7Kq70sdeW=+QEsyQrg () mail ! gmail ! com
[Download RAW message or body]

On Mon, Nov 13, 2017 at 7:06 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>
> But did you sent this review out? When?
>

I start to send review for this series out at Oct 30 and Nov 5.

For this peculiarity OP_PUSH, I thought I have send out already
but I just find a few days ago I did not. That was my bad.

But the whole process of evaluating your OP_PUSH start much
earlier. Not only do I join the discussion of the thread that
cause the OP_PUSH in the first place. I also purpose the
alternative patch at Aug 13.
https://patchwork.kernel.org/patch/9897573/

That is before you send out the git pull for llvm-fix at Sep.

You drop out of the email discussion.
There is many level of participation of the review process.
Email discussion is one. Sending out patches are the higher
level of participation.

Those email are not directly response to your llvm-fix.
But you know very well those are alternative approach to
OP_PUSH. You make claim that I did not response to
your git pull request for 8 month and you know that is not true.
It just more mud on the wall.

> I have already explained that giving some review in a reasonable
> amount of time after a series is first submitted is one thing.
> Asking for cosmetic changes (or what I tend to call "surface"
> changes) months later is another thing.
> The difference being that new code is developed on top
> of the old one. And yes, it's always possible to rebase
> everything, when it's valuable functional changes.

But the OP_PUSH vs OP_SETVAL.size is not surface change.
Your OP_PUSH break the byte code compatibility and
increase the instruction count.

I would let the surface change slide and just merge it
consider you have other change depend on it.

>
>> You are making your series not touchable. Amount all
>> all patch email send over the mailing list. How much can
>> I actually pull from according our agreement? Zero.
>
> I consider our agreement as finished.

Per our agreement. If I am not complete happy about any
thing in the series. The solutions is I don't pull and you are
going to purpose a new git pull request address my concerns.

Given that you have patch depend on it and I have this
in my backlog for a long time. I am willing to help out and
create fix up patches.

I do care about make things easy for you. But I have to make
a stand on some critical issue like OP_PUSH.

>
> The reason why I insisted about the llvm series is that
> because it's the oldest and a lot of further development
> depends on it.

I understand the pain. But is it possible for you to pick a stable
point as your base like master? In the old days the master
update not very frequently. Now days master are update
reasonably. Wouldn't master serve better as base for the
topic branch?

>
> You seem to have missed the word "code" here above.
> I'll rephrase:
>    I don't comment on code if I disagree with the code
>    or if the code is not to my taste but still I find
>    the code valuable enough that I don't want to block
>    it's inclusion.

That is just a cryptic way of saying I don't like your code.

>
>> And what is the reason for not reviewing this:
>> "Simplify expr->flags assignement":
>>
>> https://patchwork.kernel.org/patch/10042019/
>
> That's exactly the kind of code I find completly without any
> value:
> - if you make some measurement, please add the mean value
>   *AND* the standard deviation
> - it's a change for some "optimization" but you state yourself
>   in the mail that you don't know if it make any kind of difference.

OK. That establish you did saw  the email and chose not
to response to it.

Let me ask this question. If this exact patch is not send by me,
It is send by let say  Ramsay Jones. Would it be much more likely
you did reply to the email? I think so. After all, that did match you
behavior pattern promptly reply to patch by others just not me.

That is the different between you and me. I did not response
due to lack of time. I treat your patch the same way as mine.
If I am not happy about it, I want to make it better. You ignore patch
intentionally and has strong correlation base on me send the patch or not.

And you are still saying this is not personal?

> I don't insist on anything. You have said yourself that I gave no
> arguments and it's kinda true.
> What I have is a working, integrated and tested set of patches.

Thanks. Will do then.

> Funny thing when you have yourself said in late July:
>         Warning ahead, the OP_PUSH need more discussion.
>         It is not going to apply very quickly.
>
> To which I replied:
>         The fact that you said in advance, without any
>         kind of discussion, and without having participated
>         in any sort of way in the initial discussion, that
>         it will be a slow thing (thus a painful thing) make me
>         just want to drop the whole thing and move on with
>         something else.
>
>
> I've now spend enough time with this and this is my last email
> on the whole subject.

Guilty.

That is why I did dig out the discussion and have exchange
some email with you.  I point out your mistake of using earler
Linus' email to counter his own later suggestion.
Your don't have an validate argument and you are the last one
did not response to my reasoning.

Further more, I send out alternative patch to match my suggestion.
You did not follow up.

Luc, I do have a lot of respect for you. Thank you for your contribution
to sparse.

I do want to merge your patches. I do want to do it in a way
that is less painful for you.

I might be a moron of insist certain things at times. Actually,
remove the might, I am.

I do mean to do good with sparse with the best intentions.
I don't want to screw you and I don't want to fork sparse.

Let me see what I can do with the OP_SETVAL.size.

Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]