On Thu, Nov 17, 2016 at 9:44 PM, Doug Ledford <[email protected]> wrote:
> On 11/17/16 1:49 PM, Leon Romanovsky wrote:
>> On Thu, Nov 17, 2016 at 07:13:54AM -0500, Doug Ledford wrote:
>>> Hi Linus,
>>>
>>> Due to various issues, I've been away and couldn't send a pull request
>>> for about three weeks. There were a number of -rc patches that built up
>>> in the meantime (some where there already from the early -rc stages).
>>> Obviously, there were way too many to send now, so I tried to pare the
>>> list down to the more important patches for the -rc cycle.
Hi Doug,
Except for the hfi1 patches, all the rest (core, mlx5, mlx4 and rxe)
are marked now as only 21 hours old in your 4.9-rc branch and they
seems be made from you picking partial subsets of multiple series,
with none of them acked by you on the list.
If you agree that I am describing things correctly - how are we
expected to follow on your patch picking? I find it sort of impossible
and error prone.
>> Are you adding the rest to your for-next branch? We would like to have
>> enough time to check that nothing is lost.
> Yes, it's already there in the mlx-next branch on github.
Re the patches there, this one
IB/mlx4: Set traffic class in AH
"Set traffic class within sl_tclass_flowlabel when create iboe AH.
Without this the TOS value will be empty when running VLAN tagged
traffic, because the TOS value is taken from the traffic class in the
address handle attributes.
Fixes: 9106c41 ('IB/mlx4: Fix SL to 802.1Q priority-bits mapping for IBoE')"
claims to fix my commit, I have approached Leon and Co for
clarifications/questions over the list on the patch and nothing was
answered.
Please do not send it to Linus and wait for them to respond. I
disagree that it fixes my commit b/c my commit was prior to when
route-able RoCE was introduced and on that time TOS had no relation.
and this one
"IB/mlx4: Put non zero value in max_ah device attribute
Use INT_MAX since this is the max value the attribute can hold, though
hardware capability is unlimited.
Fixes: 225c7b1 ('IB/mlx4: Add a driver Mellanox ConnectX InfiniBand adapters')"
does a tiny enhancement for a 10y old commit of Roland, why you think
we need it in 4.9-rc6 or 7??
Or.
On 11/17/2016 5:24 PM, Or Gerlitz wrote:
> On Thu, Nov 17, 2016 at 9:44 PM, Doug Ledford <[email protected]> wrote:
>> On 11/17/16 1:49 PM, Leon Romanovsky wrote:
>>> On Thu, Nov 17, 2016 at 07:13:54AM -0500, Doug Ledford wrote:
>>>> Hi Linus,
>>>>
>>>> Due to various issues, I've been away and couldn't send a pull request
>>>> for about three weeks. There were a number of -rc patches that built up
>>>> in the meantime (some where there already from the early -rc stages).
>>>> Obviously, there were way too many to send now, so I tried to pare the
>>>> list down to the more important patches for the -rc cycle.
>
> Hi Doug,
>
> Except for the hfi1 patches, all the rest (core, mlx5, mlx4 and rxe)
> are marked now as only 21 hours old in your 4.9-rc branch and they
> seems be made from you picking partial subsets of multiple series,
Correct.
> with none of them acked by you on the list.
I had an all day meeting today and had to get out the door early. The
patches will be responded to.
> If you agree that I am describing things correctly - how are we
> expected to follow on your patch picking? I find it sort of impossible
> and error prone.
When I started this I said the official, canonical source of information
on patches like this is patchworks. That still holds true. In this
case, I pulled the full series of patches into a single bundle, then
reviewed every patch individually. I checked for importance and
dependence on other patches. Those that I thought could be moved to
4.10 were moved into a new bundle and then removed from the existing
bundle. In this way, the patches were always in one or the other. When
I was done, I used git am on the two bundles and one into the 4.9-rc and
the other into a -next branch. In that way I made sure I didn't miss
any from the four series that I pulled. Finally, I used the bundles to
mark the patches as accepted in patchworks. By marking the entire
bundles as accepted, and not individual patches, it makes sure that what
I mark accepted is the same as what I ran git am on. So, if the patch
shows in patchworks as accepted, then I got it. If it doesn't, then I
missed it.
>>> Are you adding the rest to your for-next branch? We would like to have
>>> enough time to check that nothing is lost.
>
>> Yes, it's already there in the mlx-next branch on github.
>
> Re the patches there, this one
>
> IB/mlx4: Set traffic class in AH
>
> "Set traffic class within sl_tclass_flowlabel when create iboe AH.
> Without this the TOS value will be empty when running VLAN tagged
> traffic, because the TOS value is taken from the traffic class in the
> address handle attributes.
>
> Fixes: 9106c41 ('IB/mlx4: Fix SL to 802.1Q priority-bits mapping for IBoE')"
>
> claims to fix my commit, I have approached Leon and Co for
> clarifications/questions over the list on the patch and nothing was
> answered.
I agree with you. It doesn't fix your patch. The commit message can
still be fixed up.
> Please do not send it to Linus and wait for them to respond. I
> disagree that it fixes my commit b/c my commit was prior to when
> route-able RoCE was introduced and on that time TOS had no relation.
I agree. A better fix tag would be the commit that added RoCEv2 support.
> and this one
>
> "IB/mlx4: Put non zero value in max_ah device attribute
>
> Use INT_MAX since this is the max value the attribute can hold, though
> hardware capability is unlimited.
>
> Fixes: 225c7b1 ('IB/mlx4: Add a driver Mellanox ConnectX InfiniBand adapters')"
>
> does a tiny enhancement for a 10y old commit of Roland, why you think
> we need it in 4.9-rc6 or 7??
I don't, it's in the mlx-next branch which means I'll queue it up for
the 4.10 merge window. I have no plan on sending that branch for 4.9-rc.
--
Doug Ledford <[email protected]>
GPG Key ID: 0E572FDD
On Fri, Nov 18, 2016 at 4:01 AM, Doug Ledford <[email protected]> wrote:
> On 11/17/2016 5:24 PM, Or Gerlitz wrote:
[...]
> I agree with you. It doesn't fix your patch. The commit message can
> still be fixed up.
>> Please do not send it to Linus and wait for them to respond. I
>> disagree that it fixes my commit b/c my commit was prior to when
>> route-able RoCE was introduced and on that time TOS had no relation.
> I agree. A better fix tag would be the commit that added RoCEv2 support.
But this is the smaller part of the problem. The bigger part is that I
have asked for clarifications on the patch and they didn't provide
anything. So if you are picking patches where a reviewer comments are
ignored, what lesson are you teaching the submitter, that he can just
continue with this practice? why you letting this go that way?
>> does a tiny enhancement for a 10y old commit of Roland, why you think
>> we need it in 4.9-rc6 or 7??
> I don't, it's in the mlx-next branch which means I'll queue it up for
> the 4.10 merge window. I have no plan on sending that branch for 4.9-rc.
Are you going to comment on that to the submitter? if not, they are
going to continue with this practice.
How are we supposed to realize from patchworks + your github branches
that patches that were submitted for 4.9-rc are picked for 4.10? this
is very confusing and error prone too.
Please comment also on the bunch of patches I pointed you where the
copy you have picked into your tree (pulled it from somewhere?) isn't
what was submitted.
Or.
On 11/19/2016 2:46 PM, Or Gerlitz wrote:
> On Fri, Nov 18, 2016 at 4:01 AM, Doug Ledford <[email protected]> wrote:
>> On 11/17/2016 5:24 PM, Or Gerlitz wrote:
>
> [...]
>
>> I agree with you. It doesn't fix your patch. The commit message can
>> still be fixed up.
>
>>> Please do not send it to Linus and wait for them to respond. I
>>> disagree that it fixes my commit b/c my commit was prior to when
>>> route-able RoCE was introduced and on that time TOS had no relation.
>
>> I agree. A better fix tag would be the commit that added RoCEv2 support.
>
> But this is the smaller part of the problem. The bigger part is that I
> have asked for clarifications on the patch and they didn't provide
> anything.
You asked for clarification on the commit message, I didn't hear any
objections to the content of the patch itself.
> So if you are picking patches where a reviewer comments are
> ignored, what lesson are you teaching the submitter, that he can just
> continue with this practice? why you letting this go that way?
Because I can fix up the log message at any time prior to pulling it
into my official -next branch. Since that's all you objected to, I can
take the patch and wait for the final version of the comments. It's not
a big deal Or.
>>> does a tiny enhancement for a 10y old commit of Roland, why you think
>>> we need it in 4.9-rc6 or 7??
>
>> I don't, it's in the mlx-next branch which means I'll queue it up for
>> the 4.10 merge window. I have no plan on sending that branch for 4.9-rc.
>
> Are you going to comment on that to the submitter? if not, they are
> going to continue with this practice.
Comment on what to the submitter? That the patch might not have been
-rc material? I would have been OK with it around rc1 or rc2, just not
this late in the rc cycle. In the end, I don't, nor can I, rely on
submitters to determine what's RC material and what isn't, that's what
I'm supposed to be doing. I will always apply my own judgment on that
issue and submitters will learn over time when their patches get skipped
on any sort of a regular basis.
> How are we supposed to realize from patchworks + your github branches
> that patches that were submitted for 4.9-rc are picked for 4.10? this
> is very confusing and error prone too.
I emailed the submitters off list about it and provided them a list of
what patches went where and why.
> Please comment also on the bunch of patches I pointed you where the
> copy you have picked into your tree (pulled it from somewhere?) isn't
> what was submitted.
I'm sorry, but you'll have to refresh my memory on this issue.
--
Doug Ledford <[email protected]>
GPG Key ID: 0E572FDD
On Sat, Nov 19, 2016 at 06:11:22PM -0500, Doug Ledford wrote:
> On 11/19/2016 2:46 PM, Or Gerlitz wrote:
> > On Fri, Nov 18, 2016 at 4:01 AM, Doug Ledford <[email protected]> wrote:
> >> On 11/17/2016 5:24 PM, Or Gerlitz wrote:
> >
> > [...]
> > Are you going to comment on that to the submitter? if not, they are
> > going to continue with this practice.
>
> Comment on what to the submitter? That the patch might not have been
> -rc material? I would have been OK with it around rc1 or rc2, just not
> this late in the rc cycle. In the end, I don't, nor can I, rely on
> submitters to determine what's RC material and what isn't, that's what
> I'm supposed to be doing. I will always apply my own judgment on that
> issue and submitters will learn over time when their patches get skipped
> on any sort of a regular basis.
And I'm pretty fine with Doug's judgement regarding -rc vs. -next. Our
submission flow meets the expected by RDMA maintainer and we will
continue to work in the same mode as long it suits Doug's expectations
for acceptable/unacceptable submission.
>
> > How are we supposed to realize from patchworks + your github branches
> > that patches that were submitted for 4.9-rc are picked for 4.10? this
> > is very confusing and error prone too.
>
> I emailed the submitters off list about it and provided them a list of
> what patches went where and why.
Thank you, I compared the submitted list with found in your trees and
everything looks in place.