lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Mon, 19 Feb 2007 22:08:29 +0100
From:	Francois Romieu <romieu@...zoreil.com>
To:	Jarek Poplawski <jarkao2@...pl>
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH 3/4] 8139too: RTNL and flush_scheduled_work deadlock

Cc: list trimmed.

Jarek Poplawski <jarkao2@...pl> :
> On Fri, Feb 16, 2007 at 09:20:34PM +0100, Francois Romieu wrote:
[...]
> > Btw, the thread runs every 3*HZ at most.
> 
> You are right (mostly)! But I think rtnl_lock is special
> and should be spared (even this 3*HZ) and here it's used
> for some mainly internal purpose (close synchronization).
> And it looks like mainly for this internal reason holding
> of rtnl_lock is increased. And because rtnl_lock is quite
> popular you have to take into consideration that after
> this 3*HZ it could spend some time waiting for the lock.
> So, maybe it would be nicer to check this netif_running
> twice (after rtnl_lock where needed), but maybe it's a
> mater of taste only, and yours is better, as well.

The region protected by RTNL has been widened to include a tx_timeout
handler. It is supposed to handle an occasional error, something that
should not even happen at 3*HZ. Optimizing it is useless, especially
on an high-end performer like the 8139.

> (Btw. I didn't verify this, but I hope you checked that
> places not under rtnl_lock before the patch are safe from
> some locking problems now.)

I did. It is not a reason to trust the patch though.

-- 
Ueimor
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists