[PATCH] IB: merge struct ib_device_attr into struct ib_device

2015-10-12 Thread Christoph Hellwig 
Avoid the need to query for device attributes and store them in a
separate structure by merging struct ib_device_attr into struct
ib_device.  This matches how the device structures are used in most
Linux subsystems.

Signed-off-by: Christoph Hellwig 
---
 drivers/infiniband/core/cm.c   |  12 +-
 drivers/infiniband/core/cma.c  |   8 -
 drivers/infiniband/core/device.c   |  20 ---
 drivers/infiniband/core/fmr_pool.c |  20 +--
 drivers/infiniband/core/sysfs.c|  14 +-
 drivers/infiniband/core/uverbs_cmd.c   | 128 +++-
 drivers/infiniband/core/verbs.c|   8 +-
 drivers/infiniband/hw/cxgb3/iwch_provider.c|  60 +++-
 drivers/infiniband/hw/cxgb4/provider.c |  64 +++-
 drivers/infiniband/hw/mlx4/main.c  | 169 -
 drivers/infiniband/hw/mlx5/main.c  | 116 ++
 drivers/infiniband/hw/mthca/mthca_provider.c   |  77 +-
 drivers/infiniband/hw/nes/nes_verbs.c  |  94 +---
 drivers/infiniband/hw/ocrdma/ocrdma_main.c |  40 -
 drivers/infiniband/hw/ocrdma/ocrdma_verbs.c|  49 --
 drivers/infiniband/hw/ocrdma/ocrdma_verbs.h|   2 -
 drivers/infiniband/hw/qib/qib_verbs.c  |  86 +--
 drivers/infiniband/hw/usnic/usnic_ib_main.c|   3 +-
 drivers/infiniband/hw/usnic/usnic_ib_verbs.c   |  50 ++
 drivers/infiniband/hw/usnic/usnic_ib_verbs.h   |   4 +-
 drivers/infiniband/ulp/ipoib/ipoib_cm.c|  19 +--
 drivers/infiniband/ulp/ipoib/ipoib_ethtool.c   |  14 +-
 drivers/infiniband/ulp/ipoib/ipoib_main.c  |  21 +--
 drivers/infiniband/ulp/iser/iscsi_iser.c   |   4 +-
 drivers/infiniband/ulp/iser/iscsi_iser.h   |   2 -
 drivers/infiniband/ulp/iser/iser_memory.c  |   9 +-
 drivers/infiniband/ulp/iser/iser_verbs.c   |  38 ++---
 drivers/infiniband/ulp/isert/ib_isert.c|  43 ++
 drivers/infiniband/ulp/isert/ib_isert.h|   1 -
 drivers/infiniband/ulp/srp/ib_srp.c|  32 ++--
 drivers/infiniband/ulp/srpt/ib_srpt.c  |  15 +-
 drivers/infiniband/ulp/srpt/ib_srpt.h  |   3 -
 .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c|  21 +--
 drivers/staging/rdma/amso1100/c2.h |   3 -
 drivers/staging/rdma/amso1100/c2_pd.c  |   6 +-
 drivers/staging/rdma/amso1100/c2_provider.c|  23 +--
 drivers/staging/rdma/amso1100/c2_rnic.c|  63 +++-
 drivers/staging/rdma/ehca/ehca_hca.c   |  78 +-
 drivers/staging/rdma/ehca/ehca_iverbs.h|   3 +-
 drivers/staging/rdma/ehca/ehca_main.c  |   3 +-
 drivers/staging/rdma/hfi1/verbs.c  |  89 +--
 drivers/staging/rdma/ipath/ipath_verbs.c   |  90 +--
 include/rdma/ib_verbs.h|  98 ++--
 net/rds/ib.c   |  28 +---
 net/rds/iw.c   |  23 +--
 net/sunrpc/xprtrdma/frwr_ops.c |   7 +-
 net/sunrpc/xprtrdma/svc_rdma_transport.c   |  48 +++---
 net/sunrpc/xprtrdma/verbs.c|  24 +--
 net/sunrpc/xprtrdma/xprt_rdma.h|   1 -
 49 files changed, 725 insertions(+), 1108 deletions(-)

diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index ea4db9c..56c7a70 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -3749,16 +3749,6 @@ int ib_cm_init_qp_attr(struct ib_cm_id *cm_id,
 }
 EXPORT_SYMBOL(ib_cm_init_qp_attr);
 
-static void cm_get_ack_delay(struct cm_device *cm_dev)
-{
-   struct ib_device_attr attr;
-
-   if (ib_query_device(cm_dev->ib_device, ))
-   cm_dev->ack_delay = 0; /* acks will rely on packet life time */
-   else
-   cm_dev->ack_delay = attr.local_ca_ack_delay;
-}
-
 static ssize_t cm_show_counter(struct kobject *obj, struct attribute *attr,
   char *buf)
 {
@@ -3870,7 +3860,7 @@ static void cm_add_one(struct ib_device *ib_device)
return;
 
cm_dev->ib_device = ib_device;
-   cm_get_ack_delay(cm_dev);
+   cm_dev->ack_delay = ib_device->local_ca_ack_delay;
cm_dev->going_down = 0;
cm_dev->device = device_create(_class, _device->dev,
   MKDEV(0, 0), NULL,
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index b1ab13f..077c4e2 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -1847,7 +1847,6 @@ static int iw_conn_req_handler(struct iw_cm_id *cm_id,
struct rdma_id_private *listen_id, *conn_id;
struct rdma_cm_event event;
int ret;
-   struct ib_device_attr attr;
struct sockaddr *laddr = (struct

Re: [PATCH] IB: merge struct ib_device_attr into struct ib_device

2015-10-11 Thread Christoph Hellwig 
On Sun, Oct 11, 2015 at 07:16:27PM +0300, Sagi Grimberg wrote:
> Christoph, would you mind rebasing it on top of 4.3-rc4 or so? I
> want to develop over it so I can test it on the fly.

I can do a rebase to whatever you want.  Initially this was
over your reg_api branch.  Is a rebase to the latest reg_api.6
fine?
--
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] IB: merge struct ib_device_attr into struct ib_device

2015-10-11 Thread Sagi Grimberg 

On 9/24/2015 4:35 PM, Christoph Hellwig wrote:

On Thu, Sep 24, 2015 at 08:41:17AM +0300, Or Gerlitz wrote:


We had a smaller volume move to cache the device attributes on the IB
device structure, and I just
realized it was dropped on the floor. Ira, that was a reviewer comment you
got when worked on OPA
and I missed the fact it didn't reach to acceptance
http://marc.info/?t=14230931066=1=2
I vote for 1st and most doing this and taking things from there.


I'm strongly against this.  As the reviews show the move is highly
confusing.  The attributes don't change and there is no need to 'cache' or
'query' them.  Just merge them into the device, follow years of experience
with how every other Linux subsystem does it and be done with it.


Umm, are we on board with this one?

Christoph, would you mind rebasing it on top of 4.3-rc4 or so? I
want to develop over it so I can test it on the fly.
--
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] IB: merge struct ib_device_attr into struct ib_device

2015-10-11 Thread Sagi Grimberg 

I can do a rebase to whatever you want.  Initially this was
over your reg_api branch.  Is a rebase to the latest reg_api.6
fine?



That would be great.

Sagi.
--
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] IB: merge struct ib_device_attr into struct ib_device

2015-09-24 Thread Sagi Grimberg 

Christoph,

We had a smaller volume move to cache the device attributes on the IB
device structure, and I just
realized it was dropped on the floor. Ira, that was a reviewer comment
you got when worked on OPA
and I missed the fact it didn't reach to acceptance
http://marc.info/?t=14230931066=1=2
I vote for 1st and most doing this and taking things from there.


That's also workable.

But Ira's patch is missing ib_query_device conversion to
simply take the cached device attributes.

I'm fine with either way to go.

Sagi.
--
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

[PATCH] IB: merge struct ib_device_attr into struct ib_device

2015-09-23 Thread Christoph Hellwig 
Avoid the need to query for device attributes and store them in a
separate structure by merging struct ib_device_attr into struct
ib_device.  This matches how the device structures are used in most
Linux subsystems.

Signed-off-by: Christoph Hellwig 
---
 drivers/infiniband/core/cm.c   |  12 +-
 drivers/infiniband/core/cma.c  |   8 -
 drivers/infiniband/core/device.c   |  20 ---
 drivers/infiniband/core/fmr_pool.c |  20 +--
 drivers/infiniband/core/sysfs.c|  14 +-
 drivers/infiniband/core/uverbs_cmd.c   | 128 +++-
 drivers/infiniband/core/verbs.c|   8 +-
 drivers/infiniband/hw/cxgb3/iwch_provider.c|  60 +++-
 drivers/infiniband/hw/cxgb4/provider.c |  64 +++-
 drivers/infiniband/hw/mlx4/main.c  | 167 -
 drivers/infiniband/hw/mlx5/main.c  | 116 ++
 drivers/infiniband/hw/mthca/mthca_provider.c   |  77 +-
 drivers/infiniband/hw/nes/nes_verbs.c  |  94 +---
 drivers/infiniband/hw/ocrdma/ocrdma_main.c |  40 -
 drivers/infiniband/hw/ocrdma/ocrdma_verbs.c|  49 --
 drivers/infiniband/hw/ocrdma/ocrdma_verbs.h|   2 -
 drivers/infiniband/hw/qib/qib_verbs.c  |  86 +--
 drivers/infiniband/hw/usnic/usnic_ib_main.c|   3 +-
 drivers/infiniband/hw/usnic/usnic_ib_verbs.c   |  50 ++
 drivers/infiniband/hw/usnic/usnic_ib_verbs.h   |   4 +-
 drivers/infiniband/ulp/ipoib/ipoib_cm.c|  19 +--
 drivers/infiniband/ulp/ipoib/ipoib_ethtool.c   |  14 +-
 drivers/infiniband/ulp/ipoib/ipoib_main.c  |  21 +--
 drivers/infiniband/ulp/iser/iscsi_iser.c   |   4 +-
 drivers/infiniband/ulp/iser/iscsi_iser.h   |   2 -
 drivers/infiniband/ulp/iser/iser_memory.c  |   9 +-
 drivers/infiniband/ulp/iser/iser_verbs.c   |  38 ++---
 drivers/infiniband/ulp/isert/ib_isert.c|  43 ++
 drivers/infiniband/ulp/isert/ib_isert.h|   1 -
 drivers/infiniband/ulp/srp/ib_srp.c|  32 ++--
 drivers/infiniband/ulp/srpt/ib_srpt.c  |  15 +-
 drivers/infiniband/ulp/srpt/ib_srpt.h  |   3 -
 .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c|  21 +--
 drivers/staging/rdma/amso1100/c2.h |   3 -
 drivers/staging/rdma/amso1100/c2_pd.c  |   6 +-
 drivers/staging/rdma/amso1100/c2_provider.c|  23 +--
 drivers/staging/rdma/amso1100/c2_rnic.c|  63 +++-
 drivers/staging/rdma/ehca/ehca_hca.c   |  78 +-
 drivers/staging/rdma/ehca/ehca_iverbs.h|   3 +-
 drivers/staging/rdma/ehca/ehca_main.c  |   3 +-
 drivers/staging/rdma/hfi1/verbs.c  |  89 +--
 drivers/staging/rdma/ipath/ipath_verbs.c   |  90 +--
 include/rdma/ib_verbs.h|  98 ++--
 net/rds/ib.c   |  28 +---
 net/rds/iw.c   |  23 +--
 net/sunrpc/xprtrdma/frwr_ops.c |   7 +-
 net/sunrpc/xprtrdma/svc_rdma_transport.c   |  48 +++---
 net/sunrpc/xprtrdma/verbs.c|  23 +--
 net/sunrpc/xprtrdma/xprt_rdma.h|   1 -
 49 files changed, 723 insertions(+), 1107 deletions(-)

diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index ea4db9c..56c7a70 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -3749,16 +3749,6 @@ int ib_cm_init_qp_attr(struct ib_cm_id *cm_id,
 }
 EXPORT_SYMBOL(ib_cm_init_qp_attr);
 
-static void cm_get_ack_delay(struct cm_device *cm_dev)
-{
-   struct ib_device_attr attr;
-
-   if (ib_query_device(cm_dev->ib_device, ))
-   cm_dev->ack_delay = 0; /* acks will rely on packet life time */
-   else
-   cm_dev->ack_delay = attr.local_ca_ack_delay;
-}
-
 static ssize_t cm_show_counter(struct kobject *obj, struct attribute *attr,
   char *buf)
 {
@@ -3870,7 +3860,7 @@ static void cm_add_one(struct ib_device *ib_device)
return;
 
cm_dev->ib_device = ib_device;
-   cm_get_ack_delay(cm_dev);
+   cm_dev->ack_delay = ib_device->local_ca_ack_delay;
cm_dev->going_down = 0;
cm_dev->device = device_create(_class, _device->dev,
   MKDEV(0, 0), NULL,
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index b1ab13f..077c4e2 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -1847,7 +1847,6 @@ static int iw_conn_req_handler(struct iw_cm_id *cm_id,
struct rdma_id_private *listen_id, *conn_id;
struct rdma_cm_event event;
int ret;
-   struct ib_device_attr attr;
struct sockaddr *laddr = (struct

Re: [PATCH] IB: merge struct ib_device_attr into struct ib_device

2015-09-23 Thread Chuck Lever 
Hey Christoph-


> On Sep 23, 2015, at 8:52 AM, Christoph Hellwig  wrote:
> 
> Avoid the need to query for device attributes and store them in a
> separate structure by merging struct ib_device_attr into struct
> ib_device.  This matches how the device structures are used in most
> Linux subsystems.

Getting rid of ib_query_device() makes sense. Moving the device
attributes into ib_device is nice. Getting rid of ib_device_attr
is of questionable value. Why do we need to go there?

IB core API changes generate merge conflicts with feature work
in the ULPs. It’s a pain for maintainers. In this particular
case, keeping ib_device_attr would greatly reduce ULP churn.


> Signed-off-by: Christoph Hellwig 
> ---
> drivers/infiniband/core/cm.c   |  12 +-
> drivers/infiniband/core/cma.c  |   8 -
> drivers/infiniband/core/device.c   |  20 ---
> drivers/infiniband/core/fmr_pool.c |  20 +--
> drivers/infiniband/core/sysfs.c|  14 +-
> drivers/infiniband/core/uverbs_cmd.c   | 128 +++-
> drivers/infiniband/core/verbs.c|   8 +-
> drivers/infiniband/hw/cxgb3/iwch_provider.c|  60 +++-
> drivers/infiniband/hw/cxgb4/provider.c |  64 +++-
> drivers/infiniband/hw/mlx4/main.c  | 167 -
> drivers/infiniband/hw/mlx5/main.c  | 116 ++
> drivers/infiniband/hw/mthca/mthca_provider.c   |  77 +-
> drivers/infiniband/hw/nes/nes_verbs.c  |  94 +---
> drivers/infiniband/hw/ocrdma/ocrdma_main.c |  40 -
> drivers/infiniband/hw/ocrdma/ocrdma_verbs.c|  49 --
> drivers/infiniband/hw/ocrdma/ocrdma_verbs.h|   2 -
> drivers/infiniband/hw/qib/qib_verbs.c  |  86 +--
> drivers/infiniband/hw/usnic/usnic_ib_main.c|   3 +-
> drivers/infiniband/hw/usnic/usnic_ib_verbs.c   |  50 ++
> drivers/infiniband/hw/usnic/usnic_ib_verbs.h   |   4 +-
> drivers/infiniband/ulp/ipoib/ipoib_cm.c|  19 +--
> drivers/infiniband/ulp/ipoib/ipoib_ethtool.c   |  14 +-
> drivers/infiniband/ulp/ipoib/ipoib_main.c  |  21 +--
> drivers/infiniband/ulp/iser/iscsi_iser.c   |   4 +-
> drivers/infiniband/ulp/iser/iscsi_iser.h   |   2 -
> drivers/infiniband/ulp/iser/iser_memory.c  |   9 +-
> drivers/infiniband/ulp/iser/iser_verbs.c   |  38 ++---
> drivers/infiniband/ulp/isert/ib_isert.c|  43 ++
> drivers/infiniband/ulp/isert/ib_isert.h|   1 -
> drivers/infiniband/ulp/srp/ib_srp.c|  32 ++--
> drivers/infiniband/ulp/srpt/ib_srpt.c  |  15 +-
> drivers/infiniband/ulp/srpt/ib_srpt.h  |   3 -
> .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c|  21 +--
> drivers/staging/rdma/amso1100/c2.h |   3 -
> drivers/staging/rdma/amso1100/c2_pd.c  |   6 +-
> drivers/staging/rdma/amso1100/c2_provider.c|  23 +--
> drivers/staging/rdma/amso1100/c2_rnic.c|  63 +++-
> drivers/staging/rdma/ehca/ehca_hca.c   |  78 +-
> drivers/staging/rdma/ehca/ehca_iverbs.h|   3 +-
> drivers/staging/rdma/ehca/ehca_main.c  |   3 +-
> drivers/staging/rdma/hfi1/verbs.c  |  89 +--
> drivers/staging/rdma/ipath/ipath_verbs.c   |  90 +--
> include/rdma/ib_verbs.h|  98 ++--
> net/rds/ib.c   |  28 +---
> net/rds/iw.c   |  23 +--
> net/sunrpc/xprtrdma/frwr_ops.c |   7 +-
> net/sunrpc/xprtrdma/svc_rdma_transport.c   |  48 +++---
> net/sunrpc/xprtrdma/verbs.c|  23 +--
> net/sunrpc/xprtrdma/xprt_rdma.h|   1 -
> 49 files changed, 723 insertions(+), 1107 deletions(-)
> 
> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
> index ea4db9c..56c7a70 100644
> --- a/drivers/infiniband/core/cm.c
> +++ b/drivers/infiniband/core/cm.c
> @@ -3749,16 +3749,6 @@ int ib_cm_init_qp_attr(struct ib_cm_id *cm_id,
> }
> EXPORT_SYMBOL(ib_cm_init_qp_attr);
> 
> -static void cm_get_ack_delay(struct cm_device *cm_dev)
> -{
> - struct ib_device_attr attr;
> -
> - if (ib_query_device(cm_dev->ib_device, ))
> - cm_dev->ack_delay = 0; /* acks will rely on packet life time */
> - else
> - cm_dev->ack_delay = attr.local_ca_ack_delay;
> -}
> -
> static ssize_t cm_show_counter(struct kobject *obj, struct attribute *attr,
>  char *buf)
> {
> @@ -3870,7 +3860,7 @@ static void cm_add_one(struct ib_device *ib_device)
>   return;
> 
>   cm_dev->ib_device = ib_device;
> - cm_get_ack_delay(cm_dev);
> + cm_dev->ack_delay = ib_device->local_ca_ack_delay;
>   cm_dev->going_down = 0;
>

Re: [PATCH] IB: merge struct ib_device_attr into struct ib_device

2015-09-23 Thread Christoph Hellwig 
On Wed, Sep 23, 2015 at 10:23:15AM -0700, Chuck Lever wrote:
> Getting rid of ib_query_device() makes sense. Moving the device
> attributes into ib_device is nice. Getting rid of ib_device_attr
> is of questionable value. Why do we need to go there?
> 
> IB core API changes generate merge conflicts with feature work
> in the ULPs. It’s a pain for maintainers. In this particular
> case, keeping ib_device_attr would greatly reduce ULP churn.

But it will keep a totally nonsenical structure around in the future.

Note that the whole in-kernel Verbs API is a bit of a nightmare, so
there will be all kinds of larger changes.  Better get it right as long
as we have as few ULPs as we have now.
--
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] IB: merge struct ib_device_attr into struct ib_device

2015-09-23 Thread Or Gerlitz 

On 9/23/2015 9:41 PM, Christoph Hellwig wrote:

On Wed, Sep 23, 2015 at 10:23:15AM -0700, Chuck Lever wrote:

Getting rid of ib_query_device() makes sense. Moving the device
attributes into ib_device is nice. Getting rid of ib_device_attr
is of questionable value. Why do we need to go there?

IB core API changes generate merge conflicts with feature work
in the ULPs. It’s a pain for maintainers. In this particular
case, keeping ib_device_attr would greatly reduce ULP churn.

But it will keep a totally nonsenical structure around in the future.

Note that the whole in-kernel Verbs API is a bit of a nightmare, so
there will be all kinds of larger changes.  Better get it right as long
as we have as few ULPs as we have now.



Christoph,

We had a smaller volume move to cache the device attributes on the IB 
device structure, and I just
realized it was dropped on the floor. Ira, that was a reviewer comment 
you got when worked on OPA
and I missed the fact it didn't reach to acceptance 
http://marc.info/?t=14230931066=1=2

I vote for 1st and most doing this and taking things from there.

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