[1/3] batman-adv: wait for rtnl in batadv_store_mesh_iface instead of failing if it is taken

From: Matthias Schiffer <mschiffer@universe-factory.net>

The rtnl_lock in batadv_store_mesh_iface has been converted to a rtnl_trylock
some time ago to avoid a possible deadlock between rtnl and s_active on removal
of the sysfs nodes.

The behaviour introduced by that was quite confusing as it could lead to the
sysfs store to fail, making batman-adv setup scripts unreliable. As recently the
sysfs removal was postponed to a worker not running with the rtnl taken, the
deadlock can't occur any more and it is safe to change the trylock back to a
lock to make the sysfs store reliable again.

Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
Reviewed-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 net/batman-adv/sysfs.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

On Tue, 2013-06-11 at 08:34 +0200, Antonio Quartulli wrote:
> From: Matthias Schiffer <mschiffer@universe-factory.net>
> 
> The rtnl_lock in batadv_store_mesh_iface has been converted to a rtnl_trylock
> some time ago to avoid a possible deadlock between rtnl and s_active on removal
> of the sysfs nodes.
> 
> The behaviour introduced by that was quite confusing as it could lead to the
> sysfs store to fail, making batman-adv setup scripts unreliable.

I think what you actually wanted was ERESTARTNOINTR.  But the real
problem is that neither of these error codes is valid unless the system
call is aborted due to a signal.

Ben.

>  As recently the
> sysfs removal was postponed to a worker not running with the rtnl taken, the
> deadlock can't occur any more and it is safe to change the trylock back to a
> lock to make the sysfs store reliable again.
> 
> Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
> Reviewed-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
> Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
> Signed-off-by: Antonio Quartulli <ordex@autistici.org>
> ---
>  net/batman-adv/sysfs.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/net/batman-adv/sysfs.c b/net/batman-adv/sysfs.c
> index 15a22ef..929e304 100644
> --- a/net/batman-adv/sysfs.c
> +++ b/net/batman-adv/sysfs.c
> @@ -582,10 +582,7 @@ static ssize_t batadv_store_mesh_iface(struct kobject *kobj,
>  	    (strncmp(hard_iface->soft_iface->name, buff, IFNAMSIZ) == 0))
>  		goto out;
>  
> -	if (!rtnl_trylock()) {
> -		ret = -ERESTARTSYS;
> -		goto out;
> -	}
> +	rtnl_lock();
>  
>  	if (status_tmp == BATADV_IF_NOT_IN_USE) {
>  		batadv_hardif_disable_interface(hard_iface,

On Tue, Jun 11, 2013 at 03:09:30PM +0100, Ben Hutchings wrote:
> On Tue, 2013-06-11 at 08:34 +0200, Antonio Quartulli wrote:
> > From: Matthias Schiffer <mschiffer@universe-factory.net>
> > 
> > The rtnl_lock in batadv_store_mesh_iface has been converted to a rtnl_trylock
> > some time ago to avoid a possible deadlock between rtnl and s_active on removal
> > of the sysfs nodes.
> > 
> > The behaviour introduced by that was quite confusing as it could lead to the
> > sysfs store to fail, making batman-adv setup scripts unreliable.
> 
> I think what you actually wanted was ERESTARTNOINTR.  But the real
> problem is that neither of these error codes is valid unless the system
> call is aborted due to a signal.

Well, it should have been handled differently from the beginning..the other
problem was that ERESTARTSYS was propagated to userspace while this is not
supposed to happen.

However, with this patch the initialisation does not fail anymore. So we don't
need to care about the error code anymore :-)

Cheers,