Date:	Fri, 31 Jul 2015 13:41:39 -0400
From:	Doug Ledford <dledford@...hat.com>
To:	Jason Gunthorpe <jgunthorpe@...idianresearch.com>
Cc:	Or Gerlitz <gerlitz.or@...il.com>,
	Matan Barak <matanb@...lanox.com>,
	"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
	Sean Hefty <sean.hefty@...el.com>,
	Somnath Kotur <Somnath.Kotur@...gotech.com>,
	Moni Shoua <monis@...lanox.com>,
	"talal@...lanox.com" <talal@...lanox.com>,
	Haggai Eran <haggaie@...lanox.com>,
	Linux Netdev List <netdev@...r.kernel.org>
Subject: Re: [PATCH for-next V7 00/10] Move RoCE GID management to IB/Core

On 07/31/2015 12:32 PM, Jason Gunthorpe wrote:
> On Fri, Jul 31, 2015 at 08:50:24AM -0400, Doug Ledford wrote:
> 
>>> So... are we ready to go with V7 upstream?
>>
>> Yes.  I've already pulled it in.
> 
> You are taking the netdev stuff without an ack from netdev??

I've pulled it in, yes.  Dave Miller/netdev needs to ack it still.  I
really doubt they will object to the 1st or 3rd patch, but they might
have comments on the second.  Since they weren't copied on the original
submission, I'll be pinging them separately.  Dave may prefer if they go
through his tree, and that's fine, but I still need them in my tree as
of now for testing purposes.

> I've been too busy too look at v7, but a quick check of the 'move the
> cache into core code stuff' looks wrong to
> me. ib_unregister_event_handler and flush_workqueue should not/can not be
> called from a kref free'r.

Please be more specific here.  What are you objecting to?  Are you
objecting to a flush_workqueue from a release() context?  Because I
don't see anything in the kref documentation or the kref implementation
that prevents or prohibits this.  Or are you referring to the fact that
they aren't unregistering their event handler until well after the
parent device is unregistered?  That's obviously wrong, but it's also
easy to fix (the obvious fix is that they should be calling
ib_cache_cleanup_one from the top of ib_unregister_device versus waiting
for a kref put).

Regardless though, the reason I'm taking this (and a number of other
things) into my tree is that waiting for things to be absolutely perfect
on a submission is an interminable waiting game.  We have about 3 weeks
or maybe a little more until the next merge window opens.  In that time,
doing v8 of this patchset, and v9 of that patchset, and v5 of another
patchset all gets to be a huge bog on everyone's time.  These various
patchsets have reached the point where they are close and incremental
patches would be better than resubmitting the entire patchset over and
over again.  Not to mention that some of these fixes are quick and easy
to do and I'd rather quit waiting 24+ hours for a respin turnaround when
I could just hop in and make the fix myself in 15 minutes and test it
immediately.

> Looking at your for-rebase branch.. please make sure you get all the
> attributions this cycle :|

Yet *another* reason why these v6, v7, v8 patchsets are a huge time
drain :-/.  For a 10 patch v7 patchset, I need to look through 70 patch
listings in patchworks and try to see which reviewers didn't get their
attribution carried forward, and on top of that I have to make a
judgment call about which reviewed-bys should or shouldn't get carried
forward based upon how much changed in the next patch and whether or not
a previous review is still relevant or has the patch changed enough that
it needs a new review?  Really, this is my only complaint about
patchworks.  Aside from attribution loss on resubmit, it works great.

Anyway, this thread brings up an important issue, scheduling for the 4.3
merge window.  I'll bring that up in a separate email later today.

-- 
Doug Ledford <dledford@...hat.com>
              GPG KeyID: 0E572FDD



Download attachment "signature.asc" of type "application/pgp-signature" (885 bytes)

Powered by blists - more mailing lists