Re: [PATCH] svcrdma: refactor marshalling logic

["J. Bruce Fields" <bfields@fieldses.org>]

On Fri, Apr 25, 2014 at 09:24:47AM -0400, Chuck Lever wrote:
> Hi Bruce-
> 
> On Apr 25, 2014, at 6:58 AM, J. Bruce Fields <bfields@fieldses.org> wrote:
> 
> > On Thu, Apr 10, 2014 at 01:37:23PM -0500, Steve Wise wrote:
> >> From: Tom Tucker <tom@ogc.us>
> >> 
> >> This patch refactors the marshalling logic to remove the intermediary
> >> map structures.  It also fixes an existing bug where the NFSRDMA server
> >> was not minding the device fast register page list length limitations.
> >> 
> >> Signed-off-by: Tom Tucker <tom@ogc.us>
> >> ---
> >> 
> >> include/linux/sunrpc/svc_rdma.h          |    3 
> >> net/sunrpc/xprtrdma/svc_rdma_recvfrom.c  |  551 +++++++++---------------------
> >> net/sunrpc/xprtrdma/svc_rdma_sendto.c    |  230 +------------
> >> net/sunrpc/xprtrdma/svc_rdma_transport.c |   57 ++-
> >> 4 files changed, 222 insertions(+), 619 deletions(-)
> > 
> > Is it possible to make this change in more than one step?
> > 
> > RDMA is pretty esoteric to most of us, so honestly this will probably
> > get merged based just on your having tested it, but if it was possible
> > to break this up into smaller patches you might give us at least a
> > fighting chance of giving it some review….
> 
> I agree it could be broken up.
> 
> However, my testing revealed that the patch inadvertently breaks support
> for HCAs that do not support FRMR.  I’ve reported this to Steve and Tom.
> IMO you should wait for a newer version of the refactoring patch.

Great, thanks, I'll wait.

> There are plenty of people who can review and test these patches.  We are
> working on setting up regular testing in a broad array of environments.
> 
> In general it would be best for maintainers not to merge NFS/RDMA patches
> at least until you see Tested-by and/or Reviewed-by on the mailing
> list.

(which note this had).

> I
> also thought it was appropriate to mail patches To: you when they are ready
> to be merged, but To: the mailing lists when asking for review.  Will that
> be a problem?

It'd be clearer to add an [RFC ...] or say clearly in the email that
it's not meant to be applied yet.

--b.