Messages in this thread | | | Date | Sun, 26 Apr 1998 23:34:26 -0500 | From | Doug Ledford <> | Subject | Re: [patch] io-apic-2.1.98-B |
| |
David S. Miller wrote: > > Date: Sun, 26 Apr 1998 19:45:57 -0700 (PDT) > From: Linus Torvalds <torvalds@transmeta.com> > > I'd like to have the self-IPI part, but I would _really_ really > prefer to not have the counter, until somebody convices me that it > has to be there (and that implies giving a reason other than "some > ne2000 card has problems", because the problems you see sound like > they might be problems for other people regardless of any io-apic > issues). > > Ok, but one question, to a clueless driver developer, what should we > expect him to think that: > > disable_irq(x); > ... some stuff ... > disable_irq(x); > ... > enable_irq(x); > ... some stuff ... > enable_irq(x); > > does? On the Sparc we treat it the way you think it should work > (enable_irq() turns it on and no depth checking is done) however our > case is different since we've written and maintain all our drivers and > like to not think of ourselves as clueless driver developers ;-) > > Comments?
Or, in hostorical perspective, consider the following code:
disable_irq(host->irq); hostt->queuecommand(cmnd); enable_irq(host->irq);
Anyone recognize that code? It used to be in the mid level SCSI code until the mid 2.1.90 kernels. Now, consider that in the low level SCSI driver's queue command routine, that it also does disable/enable_irq() calls. Now consider the reasons for this code snippet. In the mid level code, it was designed to gaurantee the queuecommand routine would not be re-entered by the card's interrupt handler completion processing. In a low level driver, it very well may need to re-enable the irq to wait for an event (I think there are some SCSI drivers laying around that still do this sort of thing). The two goals are mutually exclusive and which one gets met depends on the implementation of enable_irq(); depth counting.
Since you asked my opinion :) I personally think the common usage of the dis/enable_irq(); calls is broken in design anyway. It doesn't include scenarios such as a single scsi driver handling multiple cards on different interrupts, etc. Therefore, I think the *only* place that dis/enable_irq() is appropriate is in a low level driver that explicitly knows the disable_irq() will be sufficient for its use. All other higher layers of code should be barred from using it unless they also have specific knowledge that other irqs won't circumvent the intended purpose anyway. Secondly, I think unbalanced dis/enable_irq() calls in a driver is sloppy practice. On the other hand, nested calls are not so sloppy, but are more utilitarian, as they can avoid otherwise useless conditionals for routines that may be called either with or without the irq disabled. As such, I vote for allowing nesting, disallowing unbalanced calls, and making every possible attempt to remove the calls entirely from code that isn't in a position to explicitly know all of the consequences of the use of these functions, such as has already been done in the mid level SCSI code.
--
Doug Ledford <dledford@dialnet.net> Opinions expressed are my own, but they should be everybody's.
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu
| |