netdev - Re: [PATCH] Improvev netconsole support for RTL8139 NIC driver

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 linux-hardening linux-cve-announce 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]

Message-ID: <47E9BF7B.3030003@pobox.com>
Date:	Tue, 25 Mar 2008 23:14:03 -0400
From:	Jeff Garzik <jgarzik@...ox.com>
To:	David Miller <davem@...emloft.net>
CC:	yang.shi@...driver.com, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Improvev netconsole support for RTL8139 NIC driver

David Miller wrote:
> From: Jeff Garzik <jgarzik@...ox.com>
> Date: Tue, 25 Mar 2008 22:23:24 -0400
> 
>> This is bogus -- you should never need to slow down the hot path in such 
>> a way.
> 
> Slow down in what way?  Even on x86 saving the flags is just
> about as expensive as a plain sti/cli.

Replacing spin_lock() [current 8139too.c] with spin_lock_irqsave() 
results in a larger interrupt handler... more CPU instructions for the 
same result.

> I would in fact prefer to see drivers unconditionally use
> spin_lock_irqsave() et al. in the interrupt handler, for
> consistency.

The entire spin_lock() apparatus in the interrupt handler disappears 
nicely on uniprocessor machines.

Plus, you are not competing with any other interrupts other than your 
own, which is the only major class of problems where spin_lock_irqsave() 
in interrupt handler is really needed (PS/2 kbd + mouse is an example).

Or more simply, it's not needed, so nothing is gained by doing 
additional work in the hot path for the sake of consistency.

	Jeff

--
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