Re: [PATCH] netconsole: avoid a crash with multiple sysfs writers

[Neil Horman]

On Thu, Aug 08, 2013 at 09:14:31AM +0300, Dan Aloni wrote:
> On Thu, Aug 8, 2013 at 8:50 AM, Neil Horman <nhorman@xxxxxxxxxxxxx> wrote:
> >
> > On Wed, Aug 07, 2013 at 12:02:44PM +0300, Dan Aloni wrote:
> [..]
> >
> > > When my 'ifup eth' script was fired multiple times and ran concurrent o> @@ -682,7 +689,11 @@ restart:
> > >                                * we might sleep in __netpoll_cleanup()
> > >                                */
> > >                               spin_unlock_irqrestore(&target_list_lock, flags);
> > > +
> > > +                             mutex_lock(&nt->mutex);
> > >                               __netpoll_cleanup(&nt->np);
> > > +                             mutex_unlock(&nt->mutex);
> > > +
> > NAK, you can't hold a mutex while calling __netpoll_cleanup.  __netpoll_cleanup
> > may sleep and its illegal to hold a mutex while doing so.
> > Neil
> >
> 
> To my understanding, it mostly depends on locking order, and having
> sleeplocks in the outer order and spinlocks in the inner order is
> valid as long the locking order is not reversed.
> 
> Also, drivers/net/team/team.c - another netpoll user, already does the
> same thing I intended in this patch - it locks the outer team->lock
> mutex in team_uninit() while calling team_port_del() and then
> team_port_disable_netpoll() calls __netpoll_cleanup().
> 
Sorry, you're right, I was under the impression that you were already in atomic
context, but as long as you don't have preempt disabled when you try to take the
mutex in any path, you should be ok.

Signed-off-by: Neil Horman <nhorman@xxxxxxxxxxxxx>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/