Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1537456 > unrolled thread

[net-next][PATCH v2 18/18] RDS: IB: add missing connection cache usage info

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [net-next][PATCH v2 18/18] RDS: IB: add missing connection cache usage info Santosh Shilimkar <santosh.shilimkar@oracle.com> - 2016-12-07 05:10 +0100
    Re: [net-next][PATCH v2 18/18] RDS: IB: add missing connection  cache usage info David Miller <davem@davemloft.net> - 2016-12-07 17:00 +0100
      Re: [net-next][PATCH v2 18/18] RDS: IB: add missing connection cache  usage info Santosh Shilimkar <santosh.shilimkar@oracle.com> - 2016-12-07 17:50 +0100
        Re: [net-next][PATCH v2 18/18] RDS: IB: add missing connection  cache usage info David Miller <davem@davemloft.net> - 2016-12-07 18:10 +0100
          Re: [net-next][PATCH v2 18/18] RDS: IB: add missing connection cache  usage info Santosh Shilimkar <santosh.shilimkar@oracle.com> - 2016-12-07 18:30 +0100
            Re: [net-next][PATCH v2 18/18] RDS: IB: add missing connection  cache usage info David Miller <davem@davemloft.net> - 2016-12-07 18:40 +0100
              Re: [net-next][PATCH v2 18/18] RDS: IB: add missing connection cache  usage info Santosh Shilimkar <santosh.shilimkar@oracle.com> - 2016-12-07 21:50 +0100

#1537456 — [net-next][PATCH v2 18/18] RDS: IB: add missing connection cache usage info

rds-tools already support it.

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>
---
 include/uapi/linux/rds.h | 1 +
 net/rds/ib.c             | 1 +
 2 files changed, 2 insertions(+)

diff --git a/include/uapi/linux/rds.h b/include/uapi/linux/rds.h
index 3833113..410ae3c 100644
--- a/include/uapi/linux/rds.h
+++ b/include/uapi/linux/rds.h
@@ -183,6 +183,7 @@ struct rds_info_rdma_connection {
 	uint32_t	max_send_sge;
 	uint32_t	rdma_mr_max;
 	uint32_t	rdma_mr_size;
+	uint32_t	cache_allocs;
 };
 
 /* RDS message Receive Path Latency points */
diff --git a/net/rds/ib.c b/net/rds/ib.c
index 8d70884..b5e2699 100644
--- a/net/rds/ib.c
+++ b/net/rds/ib.c
@@ -313,6 +313,7 @@ static int rds_ib_conn_info_visitor(struct rds_connection *conn,
 		iinfo->max_send_wr = ic->i_send_ring.w_nr;
 		iinfo->max_recv_wr = ic->i_recv_ring.w_nr;
 		iinfo->max_send_sge = rds_ibdev->max_sge;
+		iinfo->cache_allocs = atomic_read(&ic->i_cache_allocs);
 		rds_ib_get_mr_info(rds_ibdev, iinfo);
 	}
 	return 1;
-- 
1.9.1

[toc] | [next] | [standalone]

#1537867 — Re: [net-next][PATCH v2 18/18] RDS: IB: add missing connection cache usage info

From: Santosh Shilimkar <santosh.shilimkar@oracle.com>
Date: Tue,  6 Dec 2016 20:01:56 -0800

> rds-tools already support it.
> 
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>
> ---
>  include/uapi/linux/rds.h | 1 +
>  net/rds/ib.c             | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/include/uapi/linux/rds.h b/include/uapi/linux/rds.h
> index 3833113..410ae3c 100644
> --- a/include/uapi/linux/rds.h
> +++ b/include/uapi/linux/rds.h
> @@ -183,6 +183,7 @@ struct rds_info_rdma_connection {
>  	uint32_t	max_send_sge;
>  	uint32_t	rdma_mr_max;
>  	uint32_t	rdma_mr_size;
> +	uint32_t	cache_allocs;
>  };
>  
>  /* RDS message Receive Path Latency points */

What level of compatability exists here?  If we run an old tool on a new
kernel, or a new tool on an old kernel, does it work properly?

[toc] | [prev] | [next] | [standalone]

#1537901 — Re: [net-next][PATCH v2 18/18] RDS: IB: add missing connection cache usage info

On 12/7/2016 7:55 AM, David Miller wrote:
> From: Santosh Shilimkar <santosh.shilimkar@oracle.com>
> Date: Tue,  6 Dec 2016 20:01:56 -0800
>
>> rds-tools already support it.
>>
>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>
>> ---
>>  include/uapi/linux/rds.h | 1 +
>>  net/rds/ib.c             | 1 +
>>  2 files changed, 2 insertions(+)
>>
>> diff --git a/include/uapi/linux/rds.h b/include/uapi/linux/rds.h
>> index 3833113..410ae3c 100644
>> --- a/include/uapi/linux/rds.h
>> +++ b/include/uapi/linux/rds.h
>> @@ -183,6 +183,7 @@ struct rds_info_rdma_connection {
>>  	uint32_t	max_send_sge;
>>  	uint32_t	rdma_mr_max;
>>  	uint32_t	rdma_mr_size;
>> +	uint32_t	cache_allocs;
>>  };
>>
>>  /* RDS message Receive Path Latency points */
>
> What level of compatability exists here?  If we run an old tool on a new
> kernel, or a new tool on an old kernel, does it work properly?
>
Tools repo carries a copy of the header and thats how the old tool and
new tools have been running with older/newer kernels. There are few more
bits left before I can start using directly kernel header for newer
tools.

Moreover this particular parameter is only used for verbose mode which
isn't used in default options.

Regards,
Santosh

[toc] | [prev] | [next] | [standalone]

#1537937 — Re: [net-next][PATCH v2 18/18] RDS: IB: add missing connection cache usage info

From: Santosh Shilimkar <santosh.shilimkar@oracle.com>
Date: Wed, 7 Dec 2016 08:44:04 -0800

> On 12/7/2016 7:55 AM, David Miller wrote:
>> From: Santosh Shilimkar <santosh.shilimkar@oracle.com>
>> Date: Tue,  6 Dec 2016 20:01:56 -0800
>>
>> What level of compatability exists here?  If we run an old tool on a
>> new
>> kernel, or a new tool on an old kernel, does it work properly?
>>
> Tools repo carries a copy of the header and thats how the old tool and
> new tools have been running with older/newer kernels. There are few
> more
> bits left before I can start using directly kernel header for newer
> tools.
> 
> Moreover this particular parameter is only used for verbose mode which
> isn't used in default options.

That doesn't really answer my question, I think.

Are you saying that one is required to run old tools on old kernels,
and new tools on new kernels, and that's how you have this setup in
your repo?

If so, that really isn't acceptable.  Both old and new tools must work
with all kernel versions.

[toc] | [prev] | [next] | [standalone]

#1537949 — Re: [net-next][PATCH v2 18/18] RDS: IB: add missing connection cache usage info

On 12/7/2016 9:05 AM, David Miller wrote:
> From: Santosh Shilimkar <santosh.shilimkar@oracle.com>
> Date: Wed, 7 Dec 2016 08:44:04 -0800
>
>> On 12/7/2016 7:55 AM, David Miller wrote:
>>> From: Santosh Shilimkar <santosh.shilimkar@oracle.com>
>>> Date: Tue,  6 Dec 2016 20:01:56 -0800
>>>
>>> What level of compatability exists here?  If we run an old tool on a
>>> new
>>> kernel, or a new tool on an old kernel, does it work properly?
>>>
>> Tools repo carries a copy of the header and thats how the old tool and
>> new tools have been running with older/newer kernels. There are few
>> more
>> bits left before I can start using directly kernel header for newer
>> tools.
>>
>> Moreover this particular parameter is only used for verbose mode which
>> isn't used in default options.
>
> That doesn't really answer my question, I think.
>
Sorry for not being clear.

> Are you saying that one is required to run old tools on old kernels,
> and new tools on new kernels, and that's how you have this setup in
> your repo?
>
No.

> If so, that really isn't acceptable.  Both old and new tools must work
> with all kernel versions.
>
Older version of tools works on either kernel versions. Older tools
don't parse this additional info since its copy of header not
carrying some of these extra verbose fields. Newer/Updated tools which
can parse this extra info in needs newer or an updated kernel which
supports and populates these fields.

As mentioned, this particular option used only in verbose mode so
am ok to drop this change if its still a concern.

Regards,
Santosh

[toc] | [prev] | [next] | [standalone]

#1537952 — Re: [net-next][PATCH v2 18/18] RDS: IB: add missing connection cache usage info

From: Santosh Shilimkar <santosh.shilimkar@oracle.com>
Date: Wed, 7 Dec 2016 09:20:17 -0800

> Newer/Updated tools which can parse this extra info in needs newer
> or an updated kernel which supports and populates these fields.
> 
> As mentioned, this particular option used only in verbose mode so
> am ok to drop this change if its still a concern.

What does the newer tool do on an older kernel if it doesn't see
the fields?  Does it check the size of the structure given back
to it, and conditionally handle the older vs. the newer layout?

It must do this.

[toc] | [prev] | [next] | [standalone]

#1538066 — Re: [net-next][PATCH v2 18/18] RDS: IB: add missing connection cache usage info

On 12/7/2016 9:36 AM, David Miller wrote:

[...]

> What does the newer tool do on an older kernel if it doesn't see
> the fields?  Does it check the size of the structure given back
> to it, and conditionally handle the older vs. the newer layout?
>
> It must do this.
>
Right but the rds-tool doesn't handle it well and needs
to be fixed before this or any additional change in rds-info
structures. To handle the kernel struct size issues with its
user copy, there is a provision to probe kernel struct len
and then decide on layout buts its not used/implemented
in tools. The parsing comment I made is also not completely
accurate.

For now, I will drop this patch and submit it once the tools
code is fixed along with other used fields to handle both
layouts.

I should have checked the full compatibility matrix as you
commented. Sorry for oversight. Thanks for your comments Dave.

Regard,
Santosh

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web