Re: [PATCH -next v4] staging: fwserial: Switch to kfree_rcu() API

[shangxiaojing <shangxiaojing@huawei.com>]

On 2022/9/19 18:27, Dan Carpenter wrote:
> On Mon, Sep 19, 2022 at 05:43:33PM +0800, shangxiaojing wrote:
>> On 2022/9/19 17:11, Greg KH wrote:
>>> On Mon, Sep 19, 2022 at 05:10:56PM +0800, Shang XiaoJing wrote:
>>>> Instead of invoking a synchronize_rcu() to free a pointer after a grace
>>>> period, we can directly make use of a new API that does the same but in
>>>> a more efficient way.
>>>>
>>>> Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
>>>> ---
>>>> Changelog:
>>>> v3: the first version of the PATCH
>>>> v1: v3 resent as v1
>>>> v2: use kfree_rcu() instead of kvfree_rcu() for clarity
>>>> v4: resend v2 as v4 to avoid versioning confusion
>>>> ---
>>>>    drivers/staging/fwserial/fwserial.c | 3 +--
>>>>    1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
>>>> index 81b06d88ed0d..8d2b4ed1f39e 100644
>>>> --- a/drivers/staging/fwserial/fwserial.c
>>>> +++ b/drivers/staging/fwserial/fwserial.c
>>>> @@ -2117,8 +2117,7 @@ static void fwserial_remove_peer(struct fwtty_peer *peer)
>>>>    	if (port)
>>>>    		fwserial_release_port(port, true);
>>>> -	synchronize_rcu();
>>>> -	kfree(peer);
>>>> +	kfree_rcu(peer);
>> The kfree_rcu(peer) should be kfree_rcu(peer, rcu), due to the rcu_head
>> member named rcu in fwtty_peer.
> Oh, huh.  What happens if you don't pas the "rcu" parameter?  I see
> there is a similar instance in ext4_apply_quota_options().
>
> 		kfree_rcu(qname);

We assume for the condition that CONFIG_KASAN_GENERIC does not defined 
and system has no pressure (bacause kvfree_call_rcu maintains 3 path):

The input parameter "head" of kvfree_call_rcu will be NULL if we don't 
pass the rcu, which will run like:

                             might_sleep();
                             synchronize_rcu();
                             kvfree((void *) func);

instead of:

                             if (head) {
                                     call_rcu(head, func);
                                     return;
                             }

where the difference is synchronize_rcu vs call_rcu.

call_rcu() will not block, having a wider range of use compared with 
synchronize_rcu().

>
> regards,
> dan carpenter

Thanks,

Shang XiaoJing