Re: Prepared RDMA Tree for 4.7

On 05/13/2016 12:22 AM, Leon Romanovsky wrote:

On Thu, May 12, 2016 at 10:01:17PM -0400, Doug Ledford wrote:

quoted

On 05/12/2016 01:44 PM, Leon Romanovsky wrote:

quoted

Hi Doug,

I prepared the base tree [1] with patches sent to RDMA mailing and passed review.

You can't know that.  The reason I say that is because they have to pass
my review as well, and that's usually an implicit thing.  If I review
them and they have already passed list review and they pass my review,
then I include them.  If they don't pass my review (which isn't often if
the other people on the list have already spoke up, but does happen),
then I ask for revisions.  If you take them without an explicit review
from me, then you don't know if I'll ask for a revision or not.  It's
premature.

Yes, I don't know for sure, but you can see that patches which I chose
have no dispute over them and have no reasons do not to be accepted.

That's not true.  You chose some of your own patches that I'm not sure
I'm going to accept.  The ib_addr module build patch among them.

quoted

Most, there was a linux-next build failure that I'm sure is related to a
change I hadn't had a chance to look into yet but looked fishy to me
(the move to put ib_addr into ib_core looks like it's broken, but I
hadn't delved into the code to verify it...the linux-next build failure
from tonight makes me thing that it is).

It is caused by **not merged and delayed** topic branches.

The reported failure from Stephen was with *your* tree.  If there are
any **not merged and delayed** topic branches then they are in your own
tree.  When you put together your own tree, if you can't do so in a
fashion that passes testing, don't act like that's someone else's fault,
that's *your* tree and *your* responsibility.

The tree Stephen tested was *your* rdma-next, and it was the result of a
merge of *your* topic/rdma-rw-api and *your* topic/fix_core.  This has
*nothing* to do with my tree and is all about your own merge failure.
When I merge topics and they don't build properly, I fix up my own tree,
I don't blame someone else.

quoted

quoted

I believe that it will save you a lot of work and time if you use it
as a base for next merge window submission (4.7).

Not really.  I still have to review the patches, and I still have to
update patchworks, and when I'm sorting through patchworks is when I get
the patches to include.  The incremental time to grab the patches out of
patchworks and run git am -s on the bundle is negligible.  The real time
is in reading the patches, and this doesn't save me any of that time.

I'm not working with patchworks, so I can't save here, but IMHO it still
valuable and save time:
1. Separated by topics

Which I do myself anyway.

2. ROB tags

Which patchwork collects anyway.

3. cleanpatch + checkpatch before merging

Which I don't always do, but I expect patch submitters to do.

4. We (Intel and Mellanox) already run their test regressions on the upstream code
and we will run it on **almost** (pending your acceptance) latest ->next branch.
So the bugs will be caught before sending pull request to Linus.

You're making a big leap of faith that what you have pulled together
will represent even close to the next ->next pull request.  That isn't
justified.  See below...

quoted

quoted

The topics which were added:
 * topic/fix-core

Which includes one patch I haven't determined I will take yet.

quoted

quoted

 * topic/hfi1

Which has less than half of the hfi1 commits my current for-next tag
has, I'm sure Intel will be appreciative of all those lost commits.

quoted

quoted

 * topic/i40iw

Your branch has 25 commits and mine has 27.  Again, I'm sure Intel will
appreciate the lost commits.

quoted

quoted

 * topic/ipoib

This topic branch doesn't exist, but ipoib-device-address does, however,
I haven't decided to take that patch just yet, so it's probably a good
thing that you didn't include it in rdma-next even though you say you
did.  But if I can't trust the list of topic branches to be correct,
that's another issue.

quoted

quoted

 * topic/iw_cxgb4

Both of our branches have 26 commits, but I know mine has one that yours
doesn't, so now I need to search through yours to see which it has that
mine doesn't.

quoted

quoted

 * topic/iwcm

This shouldn't have been a topic, it's only one commit, but I have it.
It was in one of my misc topics.

quoted

quoted

 * topic/nes

This is only one commit, and I haven't pulled it yet.  But mine has
three other commits you are missing.

quoted

quoted

 * topic/rdma-rw-api

This is similar to mine.

quoted

quoted

 * topic/srp

And this is similar.

quoted

The help is appreciated though ;-)

Thanks,
Will you support my effort to continue publish ->next tree?

I'll give you the same advice Linus gave me in 1994 when I started to
publish my own 1.2.13+ linux kernel tree: I don't care what you publish.
 You are free to publish anything you want.  That's the beauty of open
source.

As for what I'll use, I'll simply point out these facts:

When I helped you get your kernel.org account, I told the kernel.org
people that it was so that both Dave Miller and I could pull the exact
same *commits* from your tree for patches that might have merge
conflicts between our trees if submitted separately.  This if for things
like changes to the mlx5 offsets file.  Even if Dave and I both take the
exact same patches, that still throws a merge issue when Linus merges
the two trees.  The only way for Linus to avoid seeing that at all is if
Dave and I pull the exact same commits from your tree.  This merge cycle
was pretty easy, there were only two commits that we would likely need
to share.  But, instead of putting the tree up with those two commits in
a topic branch that each of us could merge, you sent the patches to the
list, and we both had to commit our own commits for these patches.  I'm
going to have to mention that when I send my pull request to Linus
(although, the mlx5 IB changes this cycle are simply non-existent, so I
could just drop them instead).  So this branch that I expected from you,
I didn't get.

I didn't help you set up your account because I had any intention of
pulling other branches from you.  You are free to publish whatever you
want, but I still must do my own work.  I'm accountable for things like
missed patches and crappy merges.  I prefer to be accountable for things
I actually did, not things other people did.

I would suggest you keep doing what you are doing.  Pulling the branches
that you guys care about together and running them through your internal
test/regression stuff is valuable, no doubt.  It might even save me time
when I get around to taking a patch series as you might have caught some
build/test issue and a new version of the series can be sent to the
mailing list before I include the final series in my tree.  That all has
value.

But ultimately, I will pick up the patches myself and I will do as I
said when I first started doing this job and track them in patchworks.
And what you think rdma-next will be, is not necessarily what it will in
actuality be.  And I will expect that you will actually publish the
topic branches in the future that were the reason we set up your k.o git
repo in the first place.

-- 
Doug Ledford [off-list ref]
              GPG KeyID: 0E572FDD