Re: [PATCH] Export current_is_keventd() for libphy

[Andy Fleming]

On Dec 7, 2006, at 11:05, Jeff Garzik wrote:

> Yes, I merged the code, but looking deeper at phy its clear I  
> missed some things.
>
> Looking into libphy's workqueue stuff, it has the following sequence:
>
> 	disable interrupts
> 	schedule_work()
>
> 	... time passes ...
> 	... workqueue routine is called ...
>
> 	enable interrupts
> 	handle interrupt
>
> I really have to question if a workqueue was the best choice of  
> direction for such a sequence.  You don't want to put off handling  
> an interrupt, with interrupts disabled, for a potentially unbounded  
> amount of time.
>
> Maybe the best course of action is to mark it with CONFIG_BROKEN  
> until it gets fixed.


Yes, this is one of the design choices I made to be able to lock  
properly around MDIO transactions.

1) MDIO transactions take a long time
2) Some interfaces provide an interrupt to indicate the transaction  
has completed.

So I didn't want an irq-disabling spin lock.  It would prevent 2 from  
ever being used, and would mean all interrupts were disabled for  
thousands of cycles for MDIO transactions.

So I decreed that no MDIO transactions can happen in interrupt  
context.  But the registers to disable the specific PHY's interrupt  
are only accessible through MDIO, so in order to be able to follow  
that edict AND ever handle the interrupt, I need to disable the  
interrupt.  But I only disable the PHY's interrupt (which is likely  
shared among some devices).  I agree it's ugly, but I haven't yet  
figured out another way to do it.

I'd love to eliminate the work queue altogether.  I keep thinking  
that I could do something with semaphores, or spin_trylocks, but I'm  
not sure about the best way to let the interrupt handlers do their  
thing.

Andy

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