Re: [PATCH net-next 1/2] net/smc: introduce statistics for allocated ringbufs of link group

Wen Gu
2024-08-06T12:23:21Z

On 2024/8/6 18:49, Simon Horman wrote:
> On Mon, Aug 05, 2024 at 05:05:50PM +0800, Wen Gu wrote:
>> Currently we have the statistics on sndbuf/RMB sizes of all connections
>> that have ever been on the link group, namely smc_stats_memsize. However
>> these statistics are incremental and since the ringbufs of link group
>> are allowed to be reused, we cannot know the actual allocated buffers
>> through these. So here introduces the statistic on actual allocated
>> ringbufs of the link group, it will be incremented when a new ringbuf is
>> added into buf_list and decremented when it is deleted from buf_list.
>>
>> Signed-off-by: Wen Gu <guwen@linux.alibaba.com>
>> ---
>> include/uapi/linux/smc.h | 4 ++++
>> net/smc/smc_core.c | 52 ++++++++++++++++++++++++++++++++++++----
>> net/smc/smc_core.h | 2 ++
>> 3 files changed, 54 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/uapi/linux/smc.h b/include/uapi/linux/smc.h
>> index b531e3ef011a..d27b8dc50f90 100644
>> --- a/include/uapi/linux/smc.h
>> +++ b/include/uapi/linux/smc.h
>> @@ -127,6 +127,8 @@ enum {
>> SMC_NLA_LGR_R_NET_COOKIE, /* u64 */
>> SMC_NLA_LGR_R_PAD, /* flag */
>> SMC_NLA_LGR_R_BUF_TYPE, /* u8 */
>> + SMC_NLA_LGR_R_SNDBUF_ALLOC, /* u64 */
>> + SMC_NLA_LGR_R_RMB_ALLOC, /* u64 */
>> __SMC_NLA_LGR_R_MAX,
>> SMC_NLA_LGR_R_MAX = __SMC_NLA_LGR_R_MAX - 1
>> };
>> @@ -162,6 +164,8 @@ enum {
>> SMC_NLA_LGR_D_V2_COMMON, /* nest */
>> SMC_NLA_LGR_D_EXT_GID, /* u64 */
>> SMC_NLA_LGR_D_PEER_EXT_GID, /* u64 */
>> + SMC_NLA_LGR_D_SNDBUF_ALLOC, /* u64 */
>> + SMC_NLA_LGR_D_DMB_ALLOC, /* u64 */
>> __SMC_NLA_LGR_D_MAX,
>> SMC_NLA_LGR_D_MAX = __SMC_NLA_LGR_D_MAX - 1
>> };
>> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
>> index 71fb334d8234..73c7999fc74f 100644
>> --- a/net/smc/smc_core.c
>> +++ b/net/smc/smc_core.c
>> @@ -221,6 +221,37 @@ static void smc_lgr_unregister_conn(struct smc_connection *conn)
>> write_unlock_bh(&lgr->conns_lock);
>> }
>>
>> +/* must be called under lgr->{sndbufs|rmbs} lock */
>> +static inline void smc_lgr_buf_list_add(struct smc_link_group *lgr,
>> + bool is_rmb,
>> + struct list_head *buf_list,
>> + struct smc_buf_desc *buf_desc)
>
> Please do not use the inline keyword in .c files unless there is a
> demonstrable reason to do so, e.g. performance. Rather, please allow
> the compiler to inline functions as it sees fit.
>
> The inline keyword in .h files is, of course, fine.
>

Yes.. I forgot to remove 'inline' when I moved these two helpers
from .h file to .c file. I will fix this in next version.

Thank you!

>> +{
>> + list_add(&buf_desc->list, buf_list);
>> + if (is_rmb) {
>> + lgr->alloc_rmbs += buf_desc->len;
>> + lgr->alloc_rmbs +=
>> + lgr->is_smcd ? sizeof(struct smcd_cdc_msg) : 0;
>> + } else {
>> + lgr->alloc_sndbufs += buf_desc->len;
>> + }
>> +}
>> +
>> +/* must be called under lgr->{sndbufs|rmbs} lock */
>> +static inline void smc_lgr_buf_list_del(struct smc_link_group *lgr,
>> + bool is_rmb,
>> + struct smc_buf_desc *buf_desc)
>
> Ditto.
>
>> +{
>> + list_del(&buf_desc->list);
>> + if (is_rmb) {
>> + lgr->alloc_rmbs -= buf_desc->len;
>> + lgr->alloc_rmbs -=
>> + lgr->is_smcd ? sizeof(struct smcd_cdc_msg) : 0;
>> + } else {
>> + lgr->alloc_sndbufs -= buf_desc->len;
>> + }
>> +}
>> +
>
> ...
>