diff mbox

[v3,rdma-next,5/5] iw_cxgb4: dump detailed provider-specific QP information

Message ID 9543e504320df1a6e9d75106288794bb3879b24f.1522433107.git.swise@opengridcomputing.com (mailing list archive)
State Changes Requested
Delegated to: Jason Gunthorpe
Headers show

Commit Message

Steve Wise March 30, 2018, 6:03 p.m. UTC
Provide a cxgb4-specific function to fill in qp state details.
This allows dumping important c4iw_qp state useful for debugging.

Included in the dump are the t4_sq, t4_rq structs, plus a dump
of the t4_swsqe and t4swrqe descriptors for the first and last
pending entries.

Signed-off-by: Steve Wise <swise@opengridcomputing.com>
---
 drivers/infiniband/hw/cxgb4/Makefile   |   3 +-
 drivers/infiniband/hw/cxgb4/iw_cxgb4.h |   5 +
 drivers/infiniband/hw/cxgb4/provider.c |   8 +
 drivers/infiniband/hw/cxgb4/restrack.c | 294 +++++++++++++++++++++++++++++++++
 4 files changed, 309 insertions(+), 1 deletion(-)
 create mode 100644 drivers/infiniband/hw/cxgb4/restrack.c

Comments

Jason Gunthorpe April 20, 2018, 6:24 p.m. UTC | #1
On Fri, Mar 30, 2018 at 11:03:46AM -0700, Steve Wise wrote:
> @@ -645,6 +652,7 @@ void c4iw_register_device(struct work_struct *work)
>  	dev->ibdev.iwcm->add_ref = c4iw_qp_add_ref;
>  	dev->ibdev.iwcm->rem_ref = c4iw_qp_rem_ref;
>  	dev->ibdev.iwcm->get_qp = c4iw_get_qp;
> +	dev->ibdev.res.fill_res_entry = fill_res_entry;

Wait you added dev_info and port_info too but not using? Don't add
stuff until you add the driver side.. So just defer that earlier patch.

> +static int fill_sq(struct sk_buff *msg, struct t4_wq *wq)
> +{
> +	struct nlattr *entry_attr;
> +
> +	/* WQ+SQ */
> +	entry_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_PROVIDER_ENTRY);
> +	if (!entry_attr)
> +		goto err;

Er, you want to nest ? But how will anything figure out what this
RDMA_NLDEV_ATTR_PROVIDER_ENTRY is connected to? Seems like something
is missing here.

I think I'd rather see no nesting and have you prefix the strings with
something to group them??

> +
> +	if (rdma_nl_put_provider_u32(msg, "    idx", idx))

Why leading spaces? No.. Saw this a couple of times.

> +static int fill_res_qp_entry(struct sk_buff *msg,
> +			     struct rdma_restrack_entry *res)
> +{
> +	struct ib_qp *ibqp = container_of(res, struct ib_qp, res);
> +	struct t4_swsqe *fsp = NULL, *lsp = NULL;
> +	struct t4_swrqe *frp = NULL, *lrp = NULL;
> +	struct c4iw_qp *qhp = to_c4iw_qp(ibqp);
> +	struct t4_swsqe first_sqe, last_sqe;
> +	struct t4_swrqe first_rqe, last_rqe;
> +	u16 first_sq_idx, last_sq_idx;
> +	u16 first_rq_idx, last_rq_idx;
> +	struct nlattr *table_attr;
> +	struct t4_wq wq;
> +
> +	/* User qp state is not available, so don't dump user qps */
> +	if (qhp->ucontext)
> +		return 0;
> +
> +	table_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_PROVIDER);
> +	if (!table_attr)
> +		goto err;

I think the core code should setup this nest and handle the closing of
the nest.. The driver should not be able to stuff arbitary things
outside its assigned scope.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve Wise April 20, 2018, 6:32 p.m. UTC | #2
> 
> On Fri, Mar 30, 2018 at 11:03:46AM -0700, Steve Wise wrote:
> > @@ -645,6 +652,7 @@ void c4iw_register_device(struct work_struct
> *work)
> >  	dev->ibdev.iwcm->add_ref = c4iw_qp_add_ref;
> >  	dev->ibdev.iwcm->rem_ref = c4iw_qp_rem_ref;
> >  	dev->ibdev.iwcm->get_qp = c4iw_get_qp;
> > +	dev->ibdev.res.fill_res_entry = fill_res_entry;
> 
> Wait you added dev_info and port_info too but not using? Don't add
> stuff until you add the driver side.. So just defer that earlier patch.

Leon had asked for this.  But I'll pull it for now.

> 
> > +static int fill_sq(struct sk_buff *msg, struct t4_wq *wq)
> > +{
> > +	struct nlattr *entry_attr;
> > +
> > +	/* WQ+SQ */
> > +	entry_attr = nla_nest_start(msg,
> RDMA_NLDEV_ATTR_PROVIDER_ENTRY);
> > +	if (!entry_attr)
> > +		goto err;
> 
> Er, you want to nest ? But how will anything figure out what this
> RDMA_NLDEV_ATTR_PROVIDER_ENTRY is connected to? Seems like
> something
> is missing here.
> 
> I think I'd rather see no nesting and have you prefix the strings with
> something to group them??
> 

The nesting allowed for the rdma tool to take that as a time for a new line.
So you'd get the qp-global stuff for this qp on one line, then the
sq-specific on the next line, and then rq-specific.  


> > +
> > +	if (rdma_nl_put_provider_u32(msg, "    idx", idx))
> 
> Why leading spaces? No.. Saw this a couple of times.

Again, this made for nicer formatting on the display side.  It is kind of
tricky to get decent formatting/readability with these generic provider
attributes.


> 
> > +static int fill_res_qp_entry(struct sk_buff *msg,
> > +			     struct rdma_restrack_entry *res)
> > +{
> > +	struct ib_qp *ibqp = container_of(res, struct ib_qp, res);
> > +	struct t4_swsqe *fsp = NULL, *lsp = NULL;
> > +	struct t4_swrqe *frp = NULL, *lrp = NULL;
> > +	struct c4iw_qp *qhp = to_c4iw_qp(ibqp);
> > +	struct t4_swsqe first_sqe, last_sqe;
> > +	struct t4_swrqe first_rqe, last_rqe;
> > +	u16 first_sq_idx, last_sq_idx;
> > +	u16 first_rq_idx, last_rq_idx;
> > +	struct nlattr *table_attr;
> > +	struct t4_wq wq;
> > +
> > +	/* User qp state is not available, so don't dump user qps */
> > +	if (qhp->ucontext)
> > +		return 0;
> > +
> > +	table_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_PROVIDER);
> > +	if (!table_attr)
> > +		goto err;
> 
> I think the core code should setup this nest and handle the closing of
> the nest.. The driver should not be able to stuff arbitary things
> outside its assigned scope.

Leon had some requirement to support provider attributes for shared objects.
So each provider could add its nested attributes. 

Steve.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe April 20, 2018, 6:43 p.m. UTC | #3
On Fri, Apr 20, 2018 at 01:32:32PM -0500, Steve Wise wrote:
> > > +static int fill_sq(struct sk_buff *msg, struct t4_wq *wq)
> > > +{
> > > +	struct nlattr *entry_attr;
> > > +
> > > +	/* WQ+SQ */
> > > +	entry_attr = nla_nest_start(msg,
> > RDMA_NLDEV_ATTR_PROVIDER_ENTRY);
> > > +	if (!entry_attr)
> > > +		goto err;
> > 
> > Er, you want to nest ? But how will anything figure out what this
> > RDMA_NLDEV_ATTR_PROVIDER_ENTRY is connected to? Seems like
> > something
> > is missing here.
> > 
> > I think I'd rather see no nesting and have you prefix the strings with
> > something to group them??
> > 
> 
> The nesting allowed for the rdma tool to take that as a time for a new line.
> So you'd get the qp-global stuff for this qp on one line, then the
> sq-specific on the next line, and then rq-specific.  

Oh gross.. no..

> > > +	if (rdma_nl_put_provider_u32(msg, "    idx", idx))
> > 
> > Why leading spaces? No.. Saw this a couple of times.
> 
> Again, this made for nicer formatting on the display side.  It is kind of
> tricky to get decent formatting/readability with these generic provider
> attributes.

Also gross, no..

The kernel shouldn't be involved in formatting like this.

> > > +	table_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_PROVIDER);
> > > +	if (!table_attr)
> > > +		goto err;
> > 
> > I think the core code should setup this nest and handle the closing of
> > the nest.. The driver should not be able to stuff arbitary things
> > outside its assigned scope.
> 
> Leon had some requirement to support provider attributes for shared objects.
> So each provider could add its nested attributes. 

I don't know what this means

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve Wise April 20, 2018, 7:03 p.m. UTC | #4
> 
> On Fri, Apr 20, 2018 at 01:32:32PM -0500, Steve Wise wrote:
> > > > +static int fill_sq(struct sk_buff *msg, struct t4_wq *wq)
> > > > +{
> > > > +	struct nlattr *entry_attr;
> > > > +
> > > > +	/* WQ+SQ */
> > > > +	entry_attr = nla_nest_start(msg,
> > > RDMA_NLDEV_ATTR_PROVIDER_ENTRY);
> > > > +	if (!entry_attr)
> > > > +		goto err;
> > >
> > > Er, you want to nest ? But how will anything figure out what this
> > > RDMA_NLDEV_ATTR_PROVIDER_ENTRY is connected to? Seems like
> > > something
> > > is missing here.
> > >
> > > I think I'd rather see no nesting and have you prefix the strings with
> > > something to group them??
> > >
> >
> > The nesting allowed for the rdma tool to take that as a time for a new
line.
> > So you'd get the qp-global stuff for this qp on one line, then the
> > sq-specific on the next line, and then rq-specific.
> 
> Oh gross.. no..
> 
> > > > +	if (rdma_nl_put_provider_u32(msg, "    idx", idx))
> > >
> > > Why leading spaces? No.. Saw this a couple of times.
> >
> > Again, this made for nicer formatting on the display side.  It is kind
of
> > tricky to get decent formatting/readability with these generic provider
> > attributes.
> 
> Also gross, no..
> 

Any suggestion on how the rdma tool should display this opaque data so it is
readable?

> The kernel shouldn't be involved in formatting like this.
> 
> > > > +	table_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_PROVIDER);
> > > > +	if (!table_attr)
> > > > +		goto err;
> > >
> > > I think the core code should setup this nest and handle the closing of
> > > the nest.. The driver should not be able to stuff arbitary things
> > > outside its assigned scope.
> >
> > Leon had some requirement to support provider attributes for shared
> objects.
> > So each provider could add its nested attributes.
> 
> I don't know what this means
> 
> Jason


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe April 20, 2018, 7:17 p.m. UTC | #5
On Fri, Apr 20, 2018 at 02:03:22PM -0500, Steve Wise wrote:
> > 
> > On Fri, Apr 20, 2018 at 01:32:32PM -0500, Steve Wise wrote:
> > > > > +static int fill_sq(struct sk_buff *msg, struct t4_wq *wq)
> > > > > +{
> > > > > +	struct nlattr *entry_attr;
> > > > > +
> > > > > +	/* WQ+SQ */
> > > > > +	entry_attr = nla_nest_start(msg,
> > > > RDMA_NLDEV_ATTR_PROVIDER_ENTRY);
> > > > > +	if (!entry_attr)
> > > > > +		goto err;
> > > >
> > > > Er, you want to nest ? But how will anything figure out what this
> > > > RDMA_NLDEV_ATTR_PROVIDER_ENTRY is connected to? Seems like
> > > > something
> > > > is missing here.
> > > >
> > > > I think I'd rather see no nesting and have you prefix the strings with
> > > > something to group them??
> > > >
> > >
> > > The nesting allowed for the rdma tool to take that as a time for a new
> line.
> > > So you'd get the qp-global stuff for this qp on one line, then the
> > > sq-specific on the next line, and then rq-specific.
> > 
> > Oh gross.. no..
> > 
> > > > > +	if (rdma_nl_put_provider_u32(msg, "    idx", idx))
> > > >
> > > > Why leading spaces? No.. Saw this a couple of times.
> > >
> > > Again, this made for nicer formatting on the display side.  It is kind
> of
> > > tricky to get decent formatting/readability with these generic provider
> > > attributes.
> > 
> > Also gross, no..
> > 
> 
> Any suggestion on how the rdma tool should display this opaque data so it is
> readable?

Probably don't get to have that in an ideal sense..

But have ip route list them one per line is much better than an
unstructure jumble of many per line..

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve Wise April 20, 2018, 9:50 p.m. UTC | #6
> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> owner@vger.kernel.org> On Behalf Of Jason Gunthorpe
> Sent: Friday, April 20, 2018 2:17 PM
> To: Steve Wise <swise@opengridcomputing.com>
> Cc: dledford@redhat.com; leon@kernel.org; linux-rdma@vger.kernel.org
> Subject: Re: [PATCH v3 rdma-next 5/5] iw_cxgb4: dump detailed provider-
> specific QP information
> 
> On Fri, Apr 20, 2018 at 02:03:22PM -0500, Steve Wise wrote:
> > >
> > > On Fri, Apr 20, 2018 at 01:32:32PM -0500, Steve Wise wrote:
> > > > > > +static int fill_sq(struct sk_buff *msg, struct t4_wq *wq)
> > > > > > +{
> > > > > > +	struct nlattr *entry_attr;
> > > > > > +
> > > > > > +	/* WQ+SQ */
> > > > > > +	entry_attr = nla_nest_start(msg,
> > > > > RDMA_NLDEV_ATTR_PROVIDER_ENTRY);
> > > > > > +	if (!entry_attr)
> > > > > > +		goto err;
> > > > >
> > > > > Er, you want to nest ? But how will anything figure out what this
> > > > > RDMA_NLDEV_ATTR_PROVIDER_ENTRY is connected to? Seems like
> > > > > something
> > > > > is missing here.
> > > > >
> > > > > I think I'd rather see no nesting and have you prefix the strings
with
> > > > > something to group them??
> > > > >
> > > >
> > > > The nesting allowed for the rdma tool to take that as a time for a
new
> > line.
> > > > So you'd get the qp-global stuff for this qp on one line, then the
> > > > sq-specific on the next line, and then rq-specific.
> > >
> > > Oh gross.. no..
> > >
> > > > > > +	if (rdma_nl_put_provider_u32(msg, "    idx", idx))
> > > > >
> > > > > Why leading spaces? No.. Saw this a couple of times.
> > > >
> > > > Again, this made for nicer formatting on the display side.  It is
kind
> > of
> > > > tricky to get decent formatting/readability with these generic
provider
> > > > attributes.
> > >
> > > Also gross, no..
> > >
> >
> > Any suggestion on how the rdma tool should display this opaque data so
it
> is
> > readable?
> 
> Probably don't get to have that in an ideal sense..
> 
> But have ip route list them one per line is much better than an
> unstructure jumble of many per line..

That gets terribly long.  But is it better than all on one line.  Hence my
grouping attributes as sets of nested that all get displayed on one line for
each nested set.


> 
> Jason

What about a PRINT_FMT type attr?  With an enum value like NEWLINE, INDENT,
etc?

I'm beginning to think my effort should have been spent adding what I need
to debugfs for iw_cxgb4.  I'm not sure what I want applies across all rdma
providers...

Steve.


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe April 20, 2018, 10:13 p.m. UTC | #7
On Fri, Apr 20, 2018 at 04:50:48PM -0500, Steve Wise wrote:

> What about a PRINT_FMT type attr?  With an enum value like NEWLINE, INDENT,
> etc?

The kernel just shouldn't supply that level of string formatting to a
user space tool. So weird.

The groups might be OK, but they need to make some kind of sense
beyond formatting and have some kind of self-description of what the
group is.

Lets see what Leon thinks

> I'm beginning to think my effort should have been spent adding what I need
> to debugfs for iw_cxgb4.  I'm not sure what I want applies across all rdma
> providers...

If you really care abouting formatting details then have your own tool
print it..

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve Wise April 20, 2018, 11:10 p.m. UTC | #8
> 
> On Fri, Apr 20, 2018 at 04:50:48PM -0500, Steve Wise wrote:
> 
> > What about a PRINT_FMT type attr?  With an enum value like NEWLINE,
> INDENT,
> > etc?
> 
> The kernel just shouldn't supply that level of string formatting to a
> user space tool. So weird.
> 
> The groups might be OK, but they need to make some kind of sense
> beyond formatting and have some kind of self-description of what the
> group is.
> 
> Lets see what Leon thinks
> 
> > I'm beginning to think my effort should have been spent adding what I
> need
> > to debugfs for iw_cxgb4.  I'm not sure what I want applies across all
rdma
> > providers...
> 
> If you really care abouting formatting details then have your own tool
> print it..
> 

Good idea...


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky April 23, 2018, 1:13 p.m. UTC | #9
On Fri, Apr 20, 2018 at 04:13:03PM -0600, Jason Gunthorpe wrote:
> On Fri, Apr 20, 2018 at 04:50:48PM -0500, Steve Wise wrote:
>
> > What about a PRINT_FMT type attr?  With an enum value like NEWLINE, INDENT,
> > etc?
>
> The kernel just shouldn't supply that level of string formatting to a
> user space tool. So weird.
>
> The groups might be OK, but they need to make some kind of sense
> beyond formatting and have some kind of self-description of what the
> group is.
>
> Lets see what Leon thinks

Sorry for being late, I didn't mean that driver will provide formatting,
but simply name of the proposed attribute, so rdmatool will print it as
is, all grouped together.

# rdma -dd res show qp lqpn 1
....
driver_field_name_1 driver_field_value_1 driver_field_name_2 driver_field_value_2 driver_field_name_3 driver_field_value_3

It is important to keep formatting in this simple format so all "awk"
scripts will have access to values fields very easily.


>
> > I'm beginning to think my effort should have been spent adding what I need
> > to debugfs for iw_cxgb4.  I'm not sure what I want applies across all rdma
> > providers...
>
> If you really care abouting formatting details then have your own tool
> print it..
>
> Jason
Steve Wise April 23, 2018, 3:41 p.m. UTC | #10
On 4/23/2018 8:13 AM, Leon Romanovsky wrote:
> On Fri, Apr 20, 2018 at 04:13:03PM -0600, Jason Gunthorpe wrote:
>> On Fri, Apr 20, 2018 at 04:50:48PM -0500, Steve Wise wrote:
>>
>>> What about a PRINT_FMT type attr?  With an enum value like NEWLINE, INDENT,
>>> etc?
>> The kernel just shouldn't supply that level of string formatting to a
>> user space tool. So weird.
>>
>> The groups might be OK, but they need to make some kind of sense
>> beyond formatting and have some kind of self-description of what the
>> group is.
>>
>> Lets see what Leon thinks
> Sorry for being late, I didn't mean that driver will provide formatting,
> but simply name of the proposed attribute, so rdmatool will print it as
> is, all grouped together.
>
> # rdma -dd res show qp lqpn 1
> ....
> driver_field_name_1 driver_field_value_1 driver_field_name_2 driver_field_value_2 driver_field_name_3 driver_field_value_3
>
> It is important to keep formatting in this simple format so all "awk"
> scripts will have access to values fields very easily.
>

I use awk often.  So I'm with you on this.  But I was trying to make the
output look like other output in the iproute2 commands.  If you want one
(very) long line, I"m ok with that.  But please don't object, then, when
I post the rdma tool output with these long lines.

Steve.


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky April 24, 2018, 7:18 a.m. UTC | #11
On Mon, Apr 23, 2018 at 10:41:43AM -0500, Steve Wise wrote:
>
>
> On 4/23/2018 8:13 AM, Leon Romanovsky wrote:
> > On Fri, Apr 20, 2018 at 04:13:03PM -0600, Jason Gunthorpe wrote:
> >> On Fri, Apr 20, 2018 at 04:50:48PM -0500, Steve Wise wrote:
> >>
> >>> What about a PRINT_FMT type attr?  With an enum value like NEWLINE, INDENT,
> >>> etc?
> >> The kernel just shouldn't supply that level of string formatting to a
> >> user space tool. So weird.
> >>
> >> The groups might be OK, but they need to make some kind of sense
> >> beyond formatting and have some kind of self-description of what the
> >> group is.
> >>
> >> Lets see what Leon thinks
> > Sorry for being late, I didn't mean that driver will provide formatting,
> > but simply name of the proposed attribute, so rdmatool will print it as
> > is, all grouped together.
> >
> > # rdma -dd res show qp lqpn 1
> > ....
> > driver_field_name_1 driver_field_value_1 driver_field_name_2 driver_field_value_2 driver_field_name_3 driver_field_value_3
> >
> > It is important to keep formatting in this simple format so all "awk"
> > scripts will have access to values fields very easily.
> >
>
> I use awk often.  So I'm with you on this.  But I was trying to make the
> output look like other output in the iproute2 commands.  If you want one
> (very) long line, I"m ok with that.  But please don't object, then, when
> I post the rdma tool output with these long lines.

I'll not object, but will ask you to use "-p" option (pretty) to put
newline and extra space indentation of such newlines every X number of
pairs (right now, the number can be hard coded).

Thanks

>
> Steve.
>
>
Steve Wise April 24, 2018, 1:30 p.m. UTC | #12
> -----Original Message-----
> From: Leon Romanovsky <leon@kernel.org>
> Sent: Tuesday, April 24, 2018 2:18 AM
> To: Steve Wise <swise@opengridcomputing.com>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>; dledford@redhat.com; linux-
> rdma@vger.kernel.org
> Subject: Re: [PATCH v3 rdma-next 5/5] iw_cxgb4: dump detailed provider-
> specific QP information
> 
> On Mon, Apr 23, 2018 at 10:41:43AM -0500, Steve Wise wrote:
> >
> >
> > On 4/23/2018 8:13 AM, Leon Romanovsky wrote:
> > > On Fri, Apr 20, 2018 at 04:13:03PM -0600, Jason Gunthorpe wrote:
> > >> On Fri, Apr 20, 2018 at 04:50:48PM -0500, Steve Wise wrote:
> > >>
> > >>> What about a PRINT_FMT type attr?  With an enum value like
> NEWLINE, INDENT,
> > >>> etc?
> > >> The kernel just shouldn't supply that level of string formatting to a
> > >> user space tool. So weird.
> > >>
> > >> The groups might be OK, but they need to make some kind of sense
> > >> beyond formatting and have some kind of self-description of what the
> > >> group is.
> > >>
> > >> Lets see what Leon thinks
> > > Sorry for being late, I didn't mean that driver will provide
formatting,
> > > but simply name of the proposed attribute, so rdmatool will print it
as
> > > is, all grouped together.
> > >
> > > # rdma -dd res show qp lqpn 1
> > > ....
> > > driver_field_name_1 driver_field_value_1 driver_field_name_2
> driver_field_value_2 driver_field_name_3 driver_field_value_3
> > >
> > > It is important to keep formatting in this simple format so all "awk"
> > > scripts will have access to values fields very easily.
> > >
> >
> > I use awk often.  So I'm with you on this.  But I was trying to make the
> > output look like other output in the iproute2 commands.  If you want one
> > (very) long line, I"m ok with that.  But please don't object, then, when
> > I post the rdma tool output with these long lines.
> 
> I'll not object, but will ask you to use "-p" option (pretty) to put
> newline and extra space indentation of such newlines every X number of
> pairs (right now, the number can be hard coded).
> 

This seems reasonable.

Thanks,

Steve.


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

Patch

diff --git a/drivers/infiniband/hw/cxgb4/Makefile b/drivers/infiniband/hw/cxgb4/Makefile
index fa40b68..9edd920 100644
--- a/drivers/infiniband/hw/cxgb4/Makefile
+++ b/drivers/infiniband/hw/cxgb4/Makefile
@@ -3,4 +3,5 @@  ccflags-y += -Idrivers/net/ethernet/chelsio/libcxgb
 
 obj-$(CONFIG_INFINIBAND_CXGB4) += iw_cxgb4.o
 
-iw_cxgb4-y :=  device.o cm.o provider.o mem.o cq.o qp.o resource.o ev.o id_table.o
+iw_cxgb4-y :=  device.o cm.o provider.o mem.o cq.o qp.o resource.o ev.o id_table.o \
+	       restrack.o
diff --git a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
index cc92900..5eec877 100644
--- a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
+++ b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
@@ -55,6 +55,7 @@ 
 #include <rdma/iw_cm.h>
 #include <rdma/rdma_netlink.h>
 #include <rdma/iw_portmap.h>
+#include <rdma/restrack.h>
 
 #include "cxgb4.h"
 #include "cxgb4_uld.h"
@@ -1078,4 +1079,8 @@  void __iomem *c4iw_bar2_addrs(struct c4iw_rdev *rdev, unsigned int qid,
 void c4iw_invalidate_mr(struct c4iw_dev *rhp, u32 rkey);
 struct c4iw_wr_wait *c4iw_alloc_wr_wait(gfp_t gfp);
 
+typedef int c4iw_restrack_func(struct sk_buff *msg,
+			       struct rdma_restrack_entry *res);
+extern c4iw_restrack_func *c4iw_restrack_funcs[RDMA_RESTRACK_MAX];
+
 #endif
diff --git a/drivers/infiniband/hw/cxgb4/provider.c b/drivers/infiniband/hw/cxgb4/provider.c
index 0b9cc73..1feade8 100644
--- a/drivers/infiniband/hw/cxgb4/provider.c
+++ b/drivers/infiniband/hw/cxgb4/provider.c
@@ -551,6 +551,13 @@  static struct net_device *get_netdev(struct ib_device *dev, u8 port)
 	return ndev;
 }
 
+static int fill_res_entry(struct sk_buff *msg, struct rdma_restrack_entry *res)
+{
+	return (res->type < ARRAY_SIZE(c4iw_restrack_funcs) &&
+		c4iw_restrack_funcs[res->type]) ?
+		c4iw_restrack_funcs[res->type](msg, res) : 0;
+}
+
 void c4iw_register_device(struct work_struct *work)
 {
 	int ret;
@@ -645,6 +652,7 @@  void c4iw_register_device(struct work_struct *work)
 	dev->ibdev.iwcm->add_ref = c4iw_qp_add_ref;
 	dev->ibdev.iwcm->rem_ref = c4iw_qp_rem_ref;
 	dev->ibdev.iwcm->get_qp = c4iw_get_qp;
+	dev->ibdev.res.fill_res_entry = fill_res_entry;
 	memcpy(dev->ibdev.iwcm->ifname, dev->rdev.lldi.ports[0]->name,
 	       sizeof(dev->ibdev.iwcm->ifname));
 
diff --git a/drivers/infiniband/hw/cxgb4/restrack.c b/drivers/infiniband/hw/cxgb4/restrack.c
new file mode 100644
index 0000000..e2b12e1
--- /dev/null
+++ b/drivers/infiniband/hw/cxgb4/restrack.c
@@ -0,0 +1,294 @@ 
+/*
+ * Copyright (c) 2018 Chelsio, Inc. All rights reserved.
+ *
+ * This software is available to you under a choice of one of two
+ * licenses.  You may choose to be licensed under the terms of the GNU
+ * General Public License (GPL) Version 2, available from the file
+ * COPYING in the main directory of this source tree, or the
+ * OpenIB.org BSD license below:
+ *
+ *     Redistribution and use in source and binary forms, with or
+ *     without modification, are permitted provided that the following
+ *     conditions are met:
+ *
+ *      - Redistributions of source code must retain the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer.
+ *
+ *      - Redistributions in binary form must reproduce the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer in the documentation and/or other materials
+ *        provided with the distribution.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include "iw_cxgb4.h"
+#include <rdma/restrack.h>
+#include <uapi/rdma/rdma_netlink.h>
+
+static int fill_sq(struct sk_buff *msg, struct t4_wq *wq)
+{
+	struct nlattr *entry_attr;
+
+	/* WQ+SQ */
+	entry_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_PROVIDER_ENTRY);
+	if (!entry_attr)
+		goto err;
+
+	if (rdma_nl_put_provider_u32(msg, "sqid", wq->sq.qid))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u32(msg, "flushed", wq->flushed))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u32(msg, "memsize", wq->sq.memsize))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u32(msg, "cidx", wq->sq.cidx))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u32(msg, "pidx", wq->sq.pidx))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u32(msg, "wq_pidx", wq->sq.wq_pidx))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u32(msg, "flush_cidx", wq->sq.flush_cidx))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u32(msg, "in_use", wq->sq.in_use))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u32(msg, "size", wq->sq.size))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u32_hex(msg, "flags", wq->sq.flags))
+		goto err_cancel_entry;
+
+	nla_nest_end(msg, entry_attr);
+	return 0;
+
+err_cancel_entry:
+	nla_nest_cancel(msg, entry_attr);
+err:
+	return -EMSGSIZE;
+}
+
+static int fill_rq(struct sk_buff *msg, struct t4_wq *wq)
+{
+	struct nlattr *entry_attr;
+
+	/* RQ */
+	entry_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_PROVIDER_ENTRY);
+	if (!entry_attr)
+		goto err;
+
+	if (rdma_nl_put_provider_u32(msg, "rqid", wq->rq.qid))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u32(msg, "memsize", wq->rq.memsize))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u32(msg, "cidx", wq->rq.cidx))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u32(msg, "pidx", wq->rq.pidx))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u32(msg, "wq_pidx", wq->rq.wq_pidx))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u32(msg, "msn", wq->rq.msn))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u32_hex(msg, "rqt_hwaddr", wq->rq.rqt_hwaddr))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u32(msg, "rqt_size", wq->rq.rqt_size))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u32(msg, "in_use", wq->rq.in_use))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u32(msg, "size", wq->rq.size))
+		goto err_cancel_entry;
+
+	nla_nest_end(msg, entry_attr);
+	return 0;
+
+err_cancel_entry:
+	nla_nest_cancel(msg, entry_attr);
+err:
+	return -EMSGSIZE;
+}
+
+static int fill_swsqe(struct sk_buff *msg, struct t4_sq *sq, u16 idx,
+		      struct t4_swsqe *sqe)
+{
+	struct nlattr *entry_attr;
+
+	entry_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_PROVIDER_ENTRY);
+	if (!entry_attr)
+		goto err;
+
+	if (rdma_nl_put_provider_u32(msg, "    idx", idx))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u32(msg, "opcode", sqe->opcode))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u64_hex(msg, "wr_id", sqe->wr_id))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u32(msg, "complete", sqe->complete))
+		goto err_cancel_entry;
+	if (sqe->complete) {
+		if (rdma_nl_put_provider_u32(msg, "cqe_status",
+					     CQE_STATUS(&sqe->cqe)))
+			goto err_cancel_entry;
+	}
+	if (rdma_nl_put_provider_u32(msg, "signaled", sqe->signaled))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u32(msg, "flushed", sqe->flushed))
+		goto err_cancel_entry;
+
+	nla_nest_end(msg, entry_attr);
+	return 0;
+
+err_cancel_entry:
+	nla_nest_cancel(msg, entry_attr);
+err:
+	return -EMSGSIZE;
+}
+
+/*
+ * Dump the first and last pending sqes.
+ */
+static int fill_swsqes(struct sk_buff *msg, struct t4_sq *sq,
+		       u16 first_idx, struct t4_swsqe *first_sqe,
+		       u16 last_idx, struct t4_swsqe *last_sqe)
+{
+	if (!first_sqe)
+		goto out;
+	if (fill_swsqe(msg, sq, first_idx, first_sqe))
+		goto err;
+	if (!last_sqe)
+		goto out;
+	if (fill_swsqe(msg, sq, last_idx, last_sqe))
+		goto err;
+out:
+	return 0;
+err:
+	return -EMSGSIZE;
+}
+
+static int fill_swrqe(struct sk_buff *msg, struct t4_rq *rq, u16 idx,
+		      struct t4_swrqe *rqe)
+{
+	struct nlattr *entry_attr;
+
+	entry_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_PROVIDER_ENTRY);
+	if (!entry_attr)
+		goto err;
+
+	if (rdma_nl_put_provider_u32(msg, "    idx", idx))
+		goto err_cancel_entry;
+	if (rdma_nl_put_provider_u64_hex(msg, "wr_id", rqe->wr_id))
+		goto err_cancel_entry;
+
+	nla_nest_end(msg, entry_attr);
+	return 0;
+
+err_cancel_entry:
+	nla_nest_cancel(msg, entry_attr);
+err:
+	return -EMSGSIZE;
+}
+
+/*
+ * Dump the first and last pending rqes.
+ */
+static int fill_swrqes(struct sk_buff *msg, struct t4_rq *rq,
+		       u16 first_idx, struct t4_swrqe *first_rqe,
+		       u16 last_idx, struct t4_swrqe *last_rqe)
+{
+	if (!first_rqe)
+		goto out;
+	if (fill_swrqe(msg, rq, first_idx, first_rqe))
+		goto err;
+	if (!last_rqe)
+		goto out;
+	if (fill_swrqe(msg, rq, last_idx, last_rqe))
+		goto err;
+out:
+	return 0;
+err:
+	return -EMSGSIZE;
+}
+
+static int fill_res_qp_entry(struct sk_buff *msg,
+			     struct rdma_restrack_entry *res)
+{
+	struct ib_qp *ibqp = container_of(res, struct ib_qp, res);
+	struct t4_swsqe *fsp = NULL, *lsp = NULL;
+	struct t4_swrqe *frp = NULL, *lrp = NULL;
+	struct c4iw_qp *qhp = to_c4iw_qp(ibqp);
+	struct t4_swsqe first_sqe, last_sqe;
+	struct t4_swrqe first_rqe, last_rqe;
+	u16 first_sq_idx, last_sq_idx;
+	u16 first_rq_idx, last_rq_idx;
+	struct nlattr *table_attr;
+	struct t4_wq wq;
+
+	/* User qp state is not available, so don't dump user qps */
+	if (qhp->ucontext)
+		return 0;
+
+	table_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_PROVIDER);
+	if (!table_attr)
+		goto err;
+
+	/* Get a consistent snapshot */
+	spin_lock_irq(&qhp->lock);
+	wq = qhp->wq;
+
+	/* If there are any pending sqes, copy the first and last */
+	if (wq.sq.cidx != wq.sq.pidx) {
+		first_sq_idx = wq.sq.cidx;
+		first_sqe = qhp->wq.sq.sw_sq[first_sq_idx];
+		fsp = &first_sqe;
+		last_sq_idx = wq.sq.pidx;
+		if (last_sq_idx-- == 0)
+			last_sq_idx = wq.sq.size - 1;
+		if (last_sq_idx != first_sq_idx) {
+			last_sqe = qhp->wq.sq.sw_sq[last_sq_idx];
+			lsp = &last_sqe;
+		}
+	}
+
+	/* If there are any pending rqes, copy the first and last */
+	if (wq.rq.cidx != wq.rq.pidx) {
+		first_rq_idx = wq.rq.cidx;
+		first_rqe = qhp->wq.rq.sw_rq[first_rq_idx];
+		frp = &first_rqe;
+		last_rq_idx = wq.rq.pidx;
+		if (last_rq_idx-- == 0)
+			last_rq_idx = wq.rq.size - 1;
+		if (last_rq_idx != first_rq_idx) {
+			last_rqe = qhp->wq.rq.sw_rq[last_rq_idx];
+			lrp = &last_rqe;
+		}
+	}
+	spin_unlock_irq(&qhp->lock);
+
+	if (fill_sq(msg, &wq))
+		goto err_cancel_table;
+
+	if (fill_swsqes(msg, &wq.sq, first_sq_idx, fsp, last_sq_idx, lsp))
+		goto err_cancel_table;
+
+	if (fill_rq(msg, &wq))
+		goto err_cancel_table;
+
+	if (fill_swrqes(msg, &wq.rq, first_rq_idx, frp, last_rq_idx, lrp))
+		goto err_cancel_table;
+
+	nla_nest_end(msg, table_attr);
+	return 0;
+
+err_cancel_table:
+	nla_nest_cancel(msg, table_attr);
+err:
+	return -EMSGSIZE;
+}
+
+c4iw_restrack_func *c4iw_restrack_funcs[RDMA_RESTRACK_MAX] = {
+	[RDMA_RESTRACK_QP]	= fill_res_qp_entry,
+};