On 17-01-13 08:34 AM, Stephen Hemminger wrote:
> On Thu, 12 Jan 2017 18:51:00 -0800
> John Fastabend <[email protected]> wrote:
>
>>
>> -static void free_receive_bufs(struct virtnet_info *vi)
>> +static void free_receive_bufs(struct virtnet_info *vi, bool need_lock)
>> {
>> struct bpf_prog *old_prog;
>> int i;
>>
>> - rtnl_lock();
>> + if (need_lock)
>> + rtnl_lock();
>> for (i = 0; i < vi->max_queue_pairs; i++) {
>> while (vi->rq[i].pages)
>> __free_pages(get_a_page(&vi->rq[i], GFP_KERNEL), 0);
>> @@ -1879,7 +1880,8 @@ static void free_receive_bufs(struct virtnet_info *vi)
>> if (old_prog)
>> bpf_prog_put(old_prog);
>> }
>> - rtnl_unlock();
>> + if (need_lock)
>> + rtnl_unlock();
>> }
>
> Conditional locking is bad idea; sparse complains about it and is later source
> of bugs. The more typical way of doing this in kernel is:
OK I'll use the normal form.
>
> void _foo(some args)
> {
> ASSERT_RTNL();
>
> ...
> }
>
> void foo(some args)
> {
> rtnl_lock();
> _foo(some args)
> rtnl_unlock();
> }
>
>
