Re: [PATCH V1 0/3] Add cross-channel support

[Doug Ledford]

On 12/24/2015 05:41 AM, Or Gerlitz wrote:
> On 12/24/2015 12:00 PM, Christoph Hellwig wrote:
>> On Thu, Dec 24, 2015 at 10:02:29AM +0200, Or Gerlitz wrote:
>>> We had consensus among the reviewers that the 1st patch ("IB/core: Align
>>> coding style of ib_device_cap_flags structure") is wrong cleanup which
>>> basically is (1) unneeded (2) creates more damage (git blame and such,
>>> non-applicable to uapi, more) than benefit, etc -- finally Leon was
>>> convinced too [1].
>> It's not really an issue vs uapi.  Using the the wierd BIT() macro
>> would have been, but without it I think this cleanup is ok, even if I
>> personally wouldn't have done it.  git-blame isn't really a major
>> issue either, as you can blame past revisions.
> 
> I would personally wouldn't done cleanup either and I managed to
> convinced Leon to drop it, so we had concensus among the developers, the
> maintainer didn't have other opinion and he took the wrong step -- so
> we're asking to fix, that's all.

That's not true.  I didn't bother to speak up in the thread, and I read
all of the comments.  I didn't move to BIT macros for the reason that
Christoph thinks they are crap and I didn't bother to prove him wrong
and took his word for it.  However, just aligning the macros in the area
that the patch touched is reasonable (versus aligning the entire file
just for the fun of it), and git blame will continue working fine.  My
taking of this patch was intentional (in fact, the patch didn't apply, I
had to redo it entirely by hand because the comment changes caused by
Christoph's MR cleanup patches kept this patch from applying at all).
In any case, it wasn't a mistake, and there is nothing to fix up.

>>
>>> Leon will re-spin in the coming 1-2 hours V2, could please pick it
>>> instead
>>> of V1, when people agree on direction X and you are not against it,
>>> lets do
>>> X and not Y.
>> It would be great if we could stop rebasing whats already in the tree
>> for the benefit of everyone building on top of this.  For example just
>> finished rebasing my series to move many constants includin this one
>> to the uapi headers, and I'd hate to rebase it once again now that
>> the dust has settled.
> 
> The root issue here is that nothing was picked before 4.4-rc6, so we're
> in a situation where rebases are needed in the own-maintainer tree
> (github) to make things right. No way to avoid that.
> 
> We should aim that for 4.6 and onward, code for -next will start getting
> in around rc1-2 and then things will be more robust, etc
> 
> Or.
> 
> Or.
> 


-- 
Doug Ledford 
  GPG KeyID: 0E572FDD




signature.asc
Description: OpenPGP digital signature

Re: [PATCH V1 0/3] Add cross-channel support

[Or Gerlitz]

On 12/24/2015 5:31 AM, Doug Ledford wrote:

On 12/20/2015 12:16 PM, Leon Romanovsky wrote:

Leon Romanovsky (3):
   IB/core: Align coding style of ib_device_cap_flags structure
   IB/core: Add cross-channel support
   IB/mlx5: Add driver cross-channel support

  drivers/infiniband/core/uverbs_cmd.c |  5 ++-
  drivers/infiniband/hw/mlx5/cq.c  |  7 +++-
  drivers/infiniband/hw/mlx5/main.c|  3 ++
  drivers/infiniband/hw/mlx5/mlx5_ib.h | 16 
  drivers/infiniband/hw/mlx5/qp.c  | 54 ++-
  include/linux/mlx5/qp.h  |  3 ++
  include/rdma/ib_verbs.h  | 71 +---
  7 files changed, 117 insertions(+), 42 deletions(-)



I took the series as is.  Please make sure to resubmit the libibverbs portion 
of these changes with the requested man page updates.


Doug,

Wait.

We had consensus among the reviewers that the 1st patch ("IB/core: Align 
coding style of ib_device_cap_flags structure") is wrong cleanup which 
basically is (1) unneeded (2) creates more damage (git blame and such,  
non-applicable to uapi, more) than benefit, etc -- finally Leon was 
convinced too [1].


Leon will re-spin in the coming 1-2 hours V2, could please pick it 
instead of V1, when people agree on direction X and you are not against 
it, lets do X and not Y.


thanks,

Or.

[1] http://marc.info/?t=14506106803=1=2


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V1 0/3] Add cross-channel support

[Christoph Hellwig]

On Thu, Dec 24, 2015 at 10:02:29AM +0200, Or Gerlitz wrote:
> We had consensus among the reviewers that the 1st patch ("IB/core: Align
> coding style of ib_device_cap_flags structure") is wrong cleanup which
> basically is (1) unneeded (2) creates more damage (git blame and such,
> non-applicable to uapi, more) than benefit, etc -- finally Leon was
> convinced too [1].

It's not really an issue vs uapi.  Using the the wierd BIT() macro
would have been, but without it I think this cleanup is ok, even if I
personally wouldn't have done it.  git-blame isn't really a major
issue either, as you can blame past revisions.

> Leon will re-spin in the coming 1-2 hours V2, could please pick it instead
> of V1, when people agree on direction X and you are not against it, lets do
> X and not Y.

It would be great if we could stop rebasing whats already in the tree
for the benefit of everyone building on top of this.  For example just
finished rebasing my series to move many constants includin this one
to the uapi headers, and I'd hate to rebase it once again now that
the dust has settled.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V1 0/3] Add cross-channel support

[Or Gerlitz]

On 12/24/2015 12:00 PM, Christoph Hellwig wrote:

On Thu, Dec 24, 2015 at 10:02:29AM +0200, Or Gerlitz wrote:

We had consensus among the reviewers that the 1st patch ("IB/core: Align
coding style of ib_device_cap_flags structure") is wrong cleanup which
basically is (1) unneeded (2) creates more damage (git blame and such,
non-applicable to uapi, more) than benefit, etc -- finally Leon was
convinced too [1].

It's not really an issue vs uapi.  Using the the wierd BIT() macro
would have been, but without it I think this cleanup is ok, even if I
personally wouldn't have done it.  git-blame isn't really a major
issue either, as you can blame past revisions.


I would personally wouldn't done cleanup either and I managed to 
convinced Leon to drop it, so we had concensus among the developers, the 
maintainer didn't have other opinion and he took the wrong step -- so 
we're asking to fix, that's all.



Leon will re-spin in the coming 1-2 hours V2, could please pick it instead
of V1, when people agree on direction X and you are not against it, lets do
X and not Y.

It would be great if we could stop rebasing whats already in the tree
for the benefit of everyone building on top of this.  For example just
finished rebasing my series to move many constants includin this one
to the uapi headers, and I'd hate to rebase it once again now that
the dust has settled.


The root issue here is that nothing was picked before 4.4-rc6, so we're 
in a situation where rebases are needed in the own-maintainer tree 
(github) to make things right. No way to avoid that.


We should aim that for 4.6 and onward, code for -next will start getting 
in around rc1-2 and then things will be more robust, etc


Or.

Or.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V1 0/3] Add cross-channel support

[Doug Ledford]

On 12/20/2015 05:16 AM, Leon Romanovsky wrote:
> From: Leon Romanovsky 
> 
> This patchset adds cross-channel support.
> 
> The cross-channel feature allows to execute WQEs that involve
> synchronization of I/O operations’ on different QPs.
> 
> This capability enables to program complex flows with a single
> function call, hereby significantly reducing overhead associated
> with I/O processing.
> 
> Changes from v0:
>   * Set UAR to be the same for QP and CQ.
> 
> Leon Romanovsky (3):
>   IB/core: Align coding style of ib_device_cap_flags structure
>   IB/core: Add cross-channel support
>   IB/mlx5: Add driver cross-channel support
> 
>  drivers/infiniband/core/uverbs_cmd.c |  5 ++-
>  drivers/infiniband/hw/mlx5/cq.c  |  7 +++-
>  drivers/infiniband/hw/mlx5/main.c|  3 ++
>  drivers/infiniband/hw/mlx5/mlx5_ib.h | 16 
>  drivers/infiniband/hw/mlx5/qp.c  | 54 ++-
>  include/linux/mlx5/qp.h  |  3 ++
>  include/rdma/ib_verbs.h  | 71 
> +---
>  7 files changed, 117 insertions(+), 42 deletions(-)
> 

I took the series as is.  Please make sure to resubmit the libibverbs
portion of these changes with the requested man page updates.

-- 
Doug Ledford 
  GPG KeyID: 0E572FDD




signature.asc
Description: OpenPGP digital signature