On 17-01-13 08:34 AM, Stephen Hemminger wrote: > On Thu, 12 Jan 2017 18:51:00 -0800 > John Fastabend <john.fastab...@gmail.com> 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(); > } > >