Re: init_net in restart_grace()

[Jeff Layton <jlayton@redhat.com>]

On Mon, 2017-10-30 at 18:20 +0300, Vasily Averin wrote:
> restart_grace() still use hardcoded init_net.
> It can cause to "list_add double add" in following scenario:
> 
> 1) nfsd and lockd was started in several net namespaces
> 2) nfsd in init_net was stopped (lockd was not stopped because
>  it have users from another net namespaces)
> 3) lockd got signal, called restart_grace() -> set_grace_period() 
>  and enabled lock_manager in hardcoded init_net.
> 4) nfsd in init_net is started again,
> its lockd_up() calls set_grace_period() and tries to add lock_manager
> into init_net 2nd time.
> 
> I do not understand how to fix this problem correctly.
> 
> We can:
> 1) remove per-netns calls in restart_grace().
> However it was worked for init-net-only case for ages,
> and I afraid users of this functionality will object.
> 
> 2) we can somehow provide reference to net namespace of process submitted handled signal
> and handle pointed net namespace only. 
> However this time it isn't clear for me how to do it.
>
> 3) we can call per-netns operations for all existing net namespaces
> However I'm not sure is it expected behaviour.
> Also I afraid it can be racy with creation/destroy of net namespaces.
> 
> 4) we can make lockd kernel thraad per net_ns, like nfsd.
> However this solution looks too heavy for this problem.
> 
> Could you please advise some other solution?
> 
> Thank you,
> 	Vasily Averin

The problem seems to be that list_add in locks_start_grace. It seems
like we only want to do that list_add if list_empty(&lm->list) is true.
We already do a list_del_init when removing from grace_list, and it
looks like both users of this API initialize their lists properly.

Is that enough of a fix or am I missing the bigger picture?
-- 
Jeff Layton <jlayton@redhat.com>