Re: [PATCH 2/2] SUNRPC: Optimize 'svc_print_xprts()'

[Chuck Lever]

> On Mar 26, 2020, at 5:44 PM, NeilBrown <neilb@xxxxxxx> wrote:
> 
> On Thu, Mar 26 2020, Christophe JAILLET wrote:
> 
>> Le 25/03/2020 Ã 23:53, NeilBrown a Ãcrit :
>>> Can I suggest something more like this:
>>> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
>>> index de3c077733a7..0292f45b70f6 100644
>>> --- a/net/sunrpc/svc_xprt.c
>>> +++ b/net/sunrpc/svc_xprt.c
>>> @@ -115,16 +115,9 @@ int svc_print_xprts(char *buf, int maxlen)
>>>  	buf[0] = '\0';
>>> 
>>>  	spin_lock(&svc_xprt_class_lock);
>>> -	list_for_each_entry(xcl, &svc_xprt_class_list, xcl_list) {
>>> -		int slen;
>>> -
>>> -		sprintf(tmpstr, "%s %d\n", xcl->xcl_name, xcl->xcl_max_payload);
>>> -		slen = strlen(tmpstr);
>>> -		if (len + slen > maxlen)
>>> -			break;
>>> -		len += slen;
>>> -		strcat(buf, tmpstr);
>>> -	}
>>> +	list_for_each_entry(xcl, &svc_xprt_class_list, xcl_list)
>>> +		len += scnprintf(buf + len, maxlen - len, "%s %d\n",
>>> +				 xcl->xcl_name, xcl->xcl_max_payload);
>>>  	spin_unlock(&svc_xprt_class_lock);
>>> 
>>>  	return len;
>>> 
>>> NeilBrown
>> 
>> Hi,
>> 
>> this was what I suggested in the patch:
>>     ---
>>     This patch should have no functional change.
>>     We could go further, use scnprintf and write directly in the 
>> destination
>>     buffer. However, this could lead to a truncated last line.
>>     ---
> 
> Sorry - I missed that.
> So add
> 
> end = strrchr(tmpstr, '\n');
> if (end)
>    end[1] = 0;
> else
>    tmpstr[0] = 0;
> 
> or maybe something like
> 	list_for_each_entry(xcl, &svc_xprt_class_list, xcl_list) {
> 		int l = snprintf(buf + len, maxlen - len, "%s %d\n",
> 				 xcl->xcl_name, xcl->xcl_max_payload);
>                if (l < maxlen - len)
>                	len += l;
>        }
>        buf[len] = 0;
> 
> There really is no need to have the secondary buffer, and I think doing
> so just complicates the code.

In the interest of getting this fix into the upcoming merge window, let's
stick with the temporary buffer approach. Thanks!


--
Chuck Lever