[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20070319095545.GF8386@mellanox.co.il>
Date: Mon, 19 Mar 2007 11:55:45 +0200
From: "Michael S. Tsirkin" <mst@....mellanox.co.il>
To: "Michael S. Tsirkin" <mst@....mellanox.co.il>
Cc: Alexey Kuznetsov <kuznet@....inr.ac.ru>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
netdev@...r.kernel.org, general@...ts.openfabrics.org,
Roland Dreier <rolandd@...co.com>,
David Miller <davem@...emloft.net>
Subject: Re: dst_ifdown breaks infiniband?
> Quoting Michael S. Tsirkin <mst@....mellanox.co.il>:
> Subject: Re: dst_ifdown breaks infiniband?
>
> > > Any simpler ideas?
> >
> > Well, if inifiniband destructor really needs to take that lock... no.
> > Right now I do not see.
>
> OK, this is actually not hard to fix - for infiniband, we can just look at
> neighbour->dev->type or compare neighbour->dev and
> neighbour->parms->dev - if they are different, device is being unregistered,
> so we do not need to do anything in the destructor.
>
> I'll send a patch to openfabrics, shortly.
>
> However, after implementing this fix, I hit what could be use after
> free at module unloading. Dave, Alexey, Roland, could you take a look at
> the following please?
>
> Works fine for me (survived a couple of hours of crazy device
> loading/unloading/up/down/hotplug + link data and state activity)
> and seems to fix the issue.
>
> ---------
>
> If a device driver sets neigh_destructor in neigh_params, this could
> get called after the device has been unregistered and the driver module
> removed.
>
> This is an old bug, but apparently, started to get triggered more infiniband
> after recent multicast and connected mode changes.
>
> Fix this by delaying dev_put until the neigh_params object is removed.
>
> Signed-off-by: Michael S. Tsirkin <mst@....mellanox.co.il>
The problem seems real enough but the fix seems no good -
device unregister gets blocked with
unregister_netdevice: waiting for ib0 to become free. Usage count = 1
It seems the parms object can survive indefinitely after device is removed.
How about creating a new parms object in dst_ifdown, and pointing neighbour
to this?
Would that work?
The advantage of this approach is that neigh->parms is already protected
by RCU.
--
MST
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists