Re: [e1000e] BUG triggered in blink path

[Jeff Kirsher <jeffrey.t.kirsher@intel.com>]

On Thu, Nov 11, 2010 at 02:17, Holger Eitzenberger
<holger@eitzenberger.org> wrote:
> Hi Jesse,
>
> I've attached the patch against net-next-2.6.  Please check if it's ok
> for you.  I checked e1000, igb and ixgbe as well, they don't have that
> problem.
>
>  /holger
>

Patch looks good, I have added the patch to my tree.

Cheers,
Jeff

>> > After taking a look I think this may be caused by initializing
>> > adapter->led_blink_task several times in e1000_phys_id(), while possibly
>> > led_blink_task is running:
>> >
>> >     if ((hw->phy.type == e1000_phy_ife) ||
>> >         (hw->mac.type == e1000_pchlan) ||
>> >         (hw->mac.type == e1000_82574)) {
>> >             INIT_WORK(&adapter->led_blink_task, e1000e_led_blink_task);
>> >             if (!adapter->blink_timer.function) {
>> >
>> > I can't reproduce it after moving it inside the following if block,
>> > but I'm not quite sure if this catches all races in there.  Especially
>> > the msleep_interruptible() may be too optimistic because it may
>> > actually not wait long enough.  Someone with more knowledge of the
>> > driver should take a look.
>>
>> thanks for your investigation and troubleshooting.  I don't think it is
>> correct at all to be calling INIT_WORK more than once.  In fact the
>> INIT_WORK should just be moved into probe, and then e1000_phys_id should
>> just do schedule_work.
>>
>>
>