[RFC] tulip: Support for byte queue limits

[RFC] tulip: Support for byte queue limits

[George Spelvin]

The New Hotness of fq_codel wants bql support, but my WAN link is on my
Old And Busted tulip card, which lacks it.

It's just a few lines, but the important thing is knowing where to
put them, and I've sort of guessed.  In particular, it seems like the
netdev_sent_queue call could be almost anywhere in tulip_start_xmit and
I'm not sure if there are reasons to prefer any particular position.

(You may have my S-o-b on copyright grounds, but I'd like to test it
some more before declaring this patch ready to be merged.)


diff --git a/drivers/net/ethernet/dec/tulip/interrupt.c b/drivers/net/ethernet/dec/tulip/interrupt.c
index 92306b3..d74426e 100644
--- a/drivers/net/ethernet/dec/tulip/interrupt.c
+++ b/drivers/net/ethernet/dec/tulip/interrupt.c
@@ -532,6 +532,7 @@ irqreturn_t tulip_interrupt(int irq, void *dev_instance)
 #endif
 	unsigned int work_count = tulip_max_interrupt_work;
 	unsigned int handled = 0;
+	unsigned int bytes_compl = 0;
 
 	/* Let's see whether the interrupt really is for us */
 	csr5 = ioread32(ioaddr + CSR5);
@@ -634,6 +635,7 @@ irqreturn_t tulip_interrupt(int irq, void *dev_instance)
 						 PCI_DMA_TODEVICE);
 
 				/* Free the original skb. */
+				bytes_compl += tp->tx_buffers[entry].skb->len;
 				dev_kfree_skb_irq(tp->tx_buffers[entry].skb);
 				tp->tx_buffers[entry].skb = NULL;
 				tp->tx_buffers[entry].mapping = 0;
@@ -802,6 +804,7 @@ irqreturn_t tulip_interrupt(int irq, void *dev_instance)
 	}
 #endif /* CONFIG_TULIP_NAPI */
 
+	netdev_completed_queue(dev, tx, bytes_compl);
 	if ((missed = ioread32(ioaddr + CSR8) & 0x1ffff)) {
 		dev->stats.rx_dropped += missed & 0x10000 ? 0x10000 : missed;
 	}
diff --git a/drivers/net/ethernet/dec/tulip/tulip_core.c b/drivers/net/ethernet/dec/tulip/tulip_core.c
index 1e9443d..b4249f3 100644
--- a/drivers/net/ethernet/dec/tulip/tulip_core.c
+++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
@@ -703,6 +703,7 @@ tulip_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	wmb();
 
 	tp->cur_tx++;
+	netdev_sent_queue(dev, skb->len);
 
 	/* Trigger an immediate transmit demand. */
 	iowrite32(0, tp->base_addr + CSR1);
@@ -746,6 +747,7 @@ static void tulip_clean_tx_ring(struct tulip_private *tp)
 		tp->tx_buffers[entry].skb = NULL;
 		tp->tx_buffers[entry].mapping = 0;
 	}
+	netdev_reset_queue(tp->dev);
 }
 
 static void tulip_down (struct net_device *dev)

Re: [RFC] tulip: Support for byte queue limits

[Eric Dumazet]

On Fri, 2013-07-12 at 09:20 -0400, George Spelvin wrote:
> The New Hotness of fq_codel wants bql support, but my WAN link is on my
> Old And Busted tulip card, which lacks it.
> 
> It's just a few lines, but the important thing is knowing where to
> put them, and I've sort of guessed.  In particular, it seems like the
> netdev_sent_queue call could be almost anywhere in tulip_start_xmit and
> I'm not sure if there are reasons to prefer any particular position.
> 
> (You may have my S-o-b on copyright grounds, but I'd like to test it
> some more before declaring this patch ready to be merged.)

Seems fine to me.

You might wait before David Miller re-opens net-next for new
submissions, a bit after linux-3.11-rc1 is released, before
sending an official patch.

Thanks

Re: [RFC] tulip: Support for byte queue limits

[Grant Grundler]

On Fri, Jul 12, 2013 at 6:20 AM, George Spelvin <linux@horizon.com> wrote:
> The New Hotness of fq_codel wants bql support, but my WAN link is on my
> Old And Busted tulip card, which lacks it.
>
> It's just a few lines, but the important thing is knowing where to
> put them, and I've sort of guessed.  In particular, it seems like the
> netdev_sent_queue call could be almost anywhere in tulip_start_xmit and
> I'm not sure if there are reasons to prefer any particular position.

Hi George,
While you are right that functionally it doesn't matter, my preference
would be to have nothing between the wmb() and iowrite() that kicks
off the TX. This marginally helps kick off the TX process consistently
slightly sooner. On modern HW, probably irrelevant, but not on the HW
these chips are used on.

I don't have a clue about fq_codel and trust Eric thinks the change is good.

Lastly, given I haven't powered up a system in two years which has
tulip, any one want to take over maintainer for tulip driver?
It's basically obsolete with a few rare patches like this one coming in.

cheers,
grant

>
> (You may have my S-o-b on copyright grounds, but I'd like to test it
> some more before declaring this patch ready to be merged.)
>
>
> diff --git a/drivers/net/ethernet/dec/tulip/interrupt.c b/drivers/net/ethernet/dec/tulip/interrupt.c
> index 92306b3..d74426e 100644
> --- a/drivers/net/ethernet/dec/tulip/interrupt.c
> +++ b/drivers/net/ethernet/dec/tulip/interrupt.c
> @@ -532,6 +532,7 @@ irqreturn_t tulip_interrupt(int irq, void *dev_instance)
>  #endif
>         unsigned int work_count = tulip_max_interrupt_work;
>         unsigned int handled = 0;
> +       unsigned int bytes_compl = 0;
>
>         /* Let's see whether the interrupt really is for us */
>         csr5 = ioread32(ioaddr + CSR5);
> @@ -634,6 +635,7 @@ irqreturn_t tulip_interrupt(int irq, void *dev_instance)
>                                                  PCI_DMA_TODEVICE);
>
>                                 /* Free the original skb. */
> +                               bytes_compl += tp->tx_buffers[entry].skb->len;
>                                 dev_kfree_skb_irq(tp->tx_buffers[entry].skb);
>                                 tp->tx_buffers[entry].skb = NULL;
>                                 tp->tx_buffers[entry].mapping = 0;
> @@ -802,6 +804,7 @@ irqreturn_t tulip_interrupt(int irq, void *dev_instance)
>         }
>  #endif /* CONFIG_TULIP_NAPI */
>
> +       netdev_completed_queue(dev, tx, bytes_compl);
>         if ((missed = ioread32(ioaddr + CSR8) & 0x1ffff)) {
>                 dev->stats.rx_dropped += missed & 0x10000 ? 0x10000 : missed;
>         }
> diff --git a/drivers/net/ethernet/dec/tulip/tulip_core.c b/drivers/net/ethernet/dec/tulip/tulip_core.c
> index 1e9443d..b4249f3 100644
> --- a/drivers/net/ethernet/dec/tulip/tulip_core.c
> +++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
> @@ -703,6 +703,7 @@ tulip_start_xmit(struct sk_buff *skb, struct net_device *dev)
>         wmb();
>
>         tp->cur_tx++;
> +       netdev_sent_queue(dev, skb->len);
>
>         /* Trigger an immediate transmit demand. */
>         iowrite32(0, tp->base_addr + CSR1);
> @@ -746,6 +747,7 @@ static void tulip_clean_tx_ring(struct tulip_private *tp)
>                 tp->tx_buffers[entry].skb = NULL;
>                 tp->tx_buffers[entry].mapping = 0;
>         }
> +       netdev_reset_queue(tp->dev);
>  }
>
>  static void tulip_down (struct net_device *dev)

Re: [RFC] tulip: Support for byte queue limits

[George Spelvin]

> Hi George,
> While you are right that functionally it doesn't matter, my preference
> would be to have nothing between the wmb() and iowrite() that kicks
> off the TX. This marginally helps kick off the TX process consistently
> slightly sooner. On modern HW, probably irrelevant, but not on the HW
> these chips are used on.

I'll revise it.  It just made sense to me to put it next to the other
bookkeeping line of tp->cur_tx++.  Should I move them both below the
iowrite()?  As in:

--- a/drivers/net/ethernet/dec/tulip/tulip_core.c
+++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
@@ -702,11 +702,11 @@ tulip_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	tp->tx_ring[entry].status = cpu_to_le32(DescOwned);
 	wmb();
 
-	tp->cur_tx++;
-
 	/* Trigger an immediate transmit demand. */
 	iowrite32(0, tp->base_addr + CSR1);
 
+	tp->cur_tx++;
+	netdev_sent_queue(dev, skb->len);
 	spin_unlock_irqrestore(&tp->lock, flags);
 
 	return NETDEV_TX_OK;

> Lastly, given I haven't powered up a system in two years which has
> tulip, any one want to take over maintainer for tulip driver?
> It's basically obsolete with a few rare patches like this one coming in.

I'm not up to it myself, sorry.

Re: [RFC] tulip: Support for byte queue limits

[Grant Grundler]

On Fri, Jul 12, 2013 at 11:01 AM, George Spelvin <linux@horizon.com> wrote:
>> Hi George,
>> While you are right that functionally it doesn't matter, my preference
>> would be to have nothing between the wmb() and iowrite() that kicks
>> off the TX. This marginally helps kick off the TX process consistently
>> slightly sooner. On modern HW, probably irrelevant, but not on the HW
>> these chips are used on.
>
> I'll revise it.  It just made sense to me to put it next to the other
> bookkeeping line of tp->cur_tx++.  Should I move them both below the
> iowrite()?

I would prefer that. I agree it's better to keep those two lines of
code together.

Just keep in mind this is a nit and it's not critical to accepting your change.
So whatever you submit, I'll probably ACK.

>  As in:
>
> --- a/drivers/net/ethernet/dec/tulip/tulip_core.c
> +++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
> @@ -702,11 +702,11 @@ tulip_start_xmit(struct sk_buff *skb, struct net_device *dev)
>         tp->tx_ring[entry].status = cpu_to_le32(DescOwned);
>         wmb();
>
> -       tp->cur_tx++;
> -
>         /* Trigger an immediate transmit demand. */
>         iowrite32(0, tp->base_addr + CSR1);
>
> +       tp->cur_tx++;
> +       netdev_sent_queue(dev, skb->len);
>         spin_unlock_irqrestore(&tp->lock, flags);
>
>         return NETDEV_TX_OK;

Yup - looks good.

>> Lastly, given I haven't powered up a system in two years which has
>> tulip, any one want to take over maintainer for tulip driver?
>> It's basically obsolete with a few rare patches like this one coming in.
>
> I'm not up to it myself, sorry.

No worries. Just wanted to advertise to anyone who bothers to read
about tulip patches. :)

cheers,
grant

Re: [RFC] tulip: Support for byte queue limits

[Ben Hutchings]

On Fri, 2013-07-12 at 14:01 -0400, George Spelvin wrote:
> > Hi George,
> > While you are right that functionally it doesn't matter, my preference
> > would be to have nothing between the wmb() and iowrite() that kicks
> > off the TX. This marginally helps kick off the TX process consistently
> > slightly sooner. On modern HW, probably irrelevant, but not on the HW
> > these chips are used on.
> 
> I'll revise it.  It just made sense to me to put it next to the other
> bookkeeping line of tp->cur_tx++.  Should I move them both below the
> iowrite()?  As in:
> 
> --- a/drivers/net/ethernet/dec/tulip/tulip_core.c
> +++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
> @@ -702,11 +702,11 @@ tulip_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  	tp->tx_ring[entry].status = cpu_to_le32(DescOwned);
>  	wmb();
>  
> -	tp->cur_tx++;
> -
>  	/* Trigger an immediate transmit demand. */
>  	iowrite32(0, tp->base_addr + CSR1);
>  
> +	tp->cur_tx++;
> +	netdev_sent_queue(dev, skb->len);

This is not good practice, because once you start DMA you have
effectively passed ownership of the skb to the TX completion handler.

>  	spin_unlock_irqrestore(&tp->lock, flags);

Presumably the TX completion handler will hold this spinlock and
therefore cannot free the skb before you use skb->len above.  So this
will be safe now.  But one day someone may want to get rid of this lock,
so this is a trap waiting to spring.

Ben.

>  	return NETDEV_TX_OK;
> 
> > Lastly, given I haven't powered up a system in two years which has
> > tulip, any one want to take over maintainer for tulip driver?
> > It's basically obsolete with a few rare patches like this one coming in.
> 
> I'm not up to it myself, sorry.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [RFC] tulip: Support for byte queue limits

[Grant Grundler]

On Mon, Jul 15, 2013 at 4:58 PM, Ben Hutchings
<bhutchings@solarflare.com> wrote:
> On Fri, 2013-07-12 at 14:01 -0400, George Spelvin wrote:
>> > Hi George,
>> > While you are right that functionally it doesn't matter, my preference
>> > would be to have nothing between the wmb() and iowrite() that kicks
>> > off the TX. This marginally helps kick off the TX process consistently
>> > slightly sooner. On modern HW, probably irrelevant, but not on the HW
>> > these chips are used on.
>>
>> I'll revise it.  It just made sense to me to put it next to the other
>> bookkeeping line of tp->cur_tx++.  Should I move them both below the
>> iowrite()?  As in:
>>
>> --- a/drivers/net/ethernet/dec/tulip/tulip_core.c
>> +++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
>> @@ -702,11 +702,11 @@ tulip_start_xmit(struct sk_buff *skb, struct net_device *dev)
>>       tp->tx_ring[entry].status = cpu_to_le32(DescOwned);
>>       wmb();
>>
>> -     tp->cur_tx++;
>> -
>>       /* Trigger an immediate transmit demand. */
>>       iowrite32(0, tp->base_addr + CSR1);
>>
>> +     tp->cur_tx++;
>> +     netdev_sent_queue(dev, skb->len);
>
> This is not good practice, because once you start DMA you have
> effectively passed ownership of the skb to the TX completion handler.

Is the problem the reference to skb->len?
By passing ownership,  are you suggesting the device can change this value?

AFAIK, tulip device only knows about the contents of tx_ring[] and not skb's.
Would you like to see a comment added to that effect?

>>       spin_unlock_irqrestore(&tp->lock, flags);
>
> Presumably the TX completion handler will hold this spinlock and
> therefore cannot free the skb before you use skb->len above.  So this
> will be safe now.

Correct.

> But one day someone may want to get rid of this lock,
> so this is a trap waiting to spring.

Even for tulip driver?  Sorry, I just can't imagine anyone taking
enough interest in tulip driver to implement that.  I'm not even sure
it would be possible.

thanks!
grant

Re: [RFC] tulip: Support for byte queue limits

[Eric Dumazet]

On Mon, 2013-07-15 at 18:17 -0700, Grant Grundler wrote:

> Even for tulip driver?  Sorry, I just can't imagine anyone taking
> enough interest in tulip driver to implement that.  I'm not even sure
> it would be possible.
> 

I would not care. When I reviewed this patch, I was aware of this
problem (as this is the most common error done in BQL patches), and
decided patch was fine, because lock was held.

Re: [RFC] tulip: Support for byte queue limits

[Ben Hutchings]

On Mon, 2013-07-15 at 18:17 -0700, Grant Grundler wrote:
> On Mon, Jul 15, 2013 at 4:58 PM, Ben Hutchings
> <bhutchings@solarflare.com> wrote:
> > On Fri, 2013-07-12 at 14:01 -0400, George Spelvin wrote:
> >> > Hi George,
> >> > While you are right that functionally it doesn't matter, my preference
> >> > would be to have nothing between the wmb() and iowrite() that kicks
> >> > off the TX. This marginally helps kick off the TX process consistently
> >> > slightly sooner. On modern HW, probably irrelevant, but not on the HW
> >> > these chips are used on.
> >>
> >> I'll revise it.  It just made sense to me to put it next to the other
> >> bookkeeping line of tp->cur_tx++.  Should I move them both below the
> >> iowrite()?  As in:
> >>
> >> --- a/drivers/net/ethernet/dec/tulip/tulip_core.c
> >> +++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
> >> @@ -702,11 +702,11 @@ tulip_start_xmit(struct sk_buff *skb, struct net_device *dev)
> >>       tp->tx_ring[entry].status = cpu_to_le32(DescOwned);
> >>       wmb();
> >>
> >> -     tp->cur_tx++;
> >> -
> >>       /* Trigger an immediate transmit demand. */
> >>       iowrite32(0, tp->base_addr + CSR1);
> >>
> >> +     tp->cur_tx++;
> >> +     netdev_sent_queue(dev, skb->len);
> >
> > This is not good practice, because once you start DMA you have
> > effectively passed ownership of the skb to the TX completion handler.
> 
> Is the problem the reference to skb->len?
> By passing ownership,  are you suggesting the device can change this value?

No, the device can complete the descriptor and then the TX completion
handler will free the skb.

[...]
> > But one day someone may want to get rid of this lock,
> > so this is a trap waiting to spring.
> 
> Even for tulip driver?  Sorry, I just can't imagine anyone taking
> enough interest in tulip driver to implement that.  I'm not even sure
> it would be possible.

You're taking interest in it, aren't you?  But I accept this is a minor
issue.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [RFC] tulip: Support for byte queue limits

[Grant Grundler]

On Tue, Jul 16, 2013 at 7:37 AM, Ben Hutchings
<bhutchings@solarflare.com> wrote:
> On Mon, 2013-07-15 at 18:17 -0700, Grant Grundler wrote:
...
>> Is the problem the reference to skb->len?
>> By passing ownership,  are you suggesting the device can change this value?
>
> No, the device can complete the descriptor and then the TX completion
> handler will free the skb.

Ah ok. That's what the spinlock_irqsave() is protecting against.

> [...]
>> > But one day someone may want to get rid of this lock,
>> > so this is a trap waiting to spring.
>>
>> Even for tulip driver?  Sorry, I just can't imagine anyone taking
>> enough interest in tulip driver to implement that.  I'm not even sure
>> it would be possible.
>
> You're taking interest in it, aren't you?

Let me clarify my interest: I will reject patches to change the
locking for this driver. Patch reviews for this driver are a
"community service project" since I know reasonably well how this chip
behaves. But I'm not doing anything else (e.g. bug fixes or testing).

If someone else wants to change the tulip driver locking, they need to
submit a patch to become the new maintainer first (I'll ACK that :)

>  But I accept this is a minor issue.

Ok.

cheers,
grant

Re: [RFC] tulip: Support for byte queue limits

[George Spelvin]

>>  	wmb();
>>  
>> -	tp->cur_tx++;
>> -
>>  	/* Trigger an immediate transmit demand. */
>>  	iowrite32(0, tp->base_addr + CSR1);
>>  
>> +	tp->cur_tx++;
>> +	netdev_sent_queue(dev, skb->len);
>>  	spin_unlock_irqrestore(&tp->lock, flags);

> This is not good practice, because once you start DMA you have
> effectively passed ownership of the skb to the TX completion handler.

Thank you for this advice.  Just to be clear, is the only issue reading
skb->len from a potentially deallocated skb?  Or is it also going go
give the byte queue system fits if the transmit complete handler calls
netdev_completed_queue before the transmitter calls netdev_sent_queue?

I'd hope it can underflow safely, and only look at the net value after
the transmit handler returns.

> Presumably the TX completion handler will hold this spinlock and
> therefore cannot free the skb before you use skb->len above.  So this
> will be safe now.  But one day someone may want to get rid of this lock,
> so this is a trap waiting to spring.

Sounds like a fun project.  But I have to dig into the BQL code; if it's
getting and dropping locks inside netdev_*_queue, the win is limited.

(A lock-free version would have separate "sent" and "completed" counters,
and compute the difference when a snapshot is required.)

Re: [RFC] tulip: Support for byte queue limits

[Ben Hutchings]

On Wed, 2013-07-17 at 00:09 -0400, George Spelvin wrote:
> >>  	wmb();
> >>  
> >> -	tp->cur_tx++;
> >> -
> >>  	/* Trigger an immediate transmit demand. */
> >>  	iowrite32(0, tp->base_addr + CSR1);
> >>  
> >> +	tp->cur_tx++;
> >> +	netdev_sent_queue(dev, skb->len);
> >>  	spin_unlock_irqrestore(&tp->lock, flags);
> 
> > This is not good practice, because once you start DMA you have
> > effectively passed ownership of the skb to the TX completion handler.
> 
> Thank you for this advice.  Just to be clear, is the only issue reading
> skb->len from a potentially deallocated skb?

That's what I was thinking of.

> Or is it also going go
> give the byte queue system fits if the transmit complete handler calls
> netdev_completed_queue before the transmitter calls netdev_sent_queue?

I don't know.

> I'd hope it can underflow safely, and only look at the net value after
> the transmit handler returns.
> 
> > Presumably the TX completion handler will hold this spinlock and
> > therefore cannot free the skb before you use skb->len above.  So this
> > will be safe now.  But one day someone may want to get rid of this lock,
> > so this is a trap waiting to spring.
> 
> Sounds like a fun project.  But I have to dig into the BQL code; if it's
> getting and dropping locks inside netdev_*_queue, the win is limited.
> 
> (A lock-free version would have separate "sent" and "completed" counters,
> and compute the difference when a snapshot is required.)

Yes, it is lock-free with separate counters.  The implementation is in
lib/dynamic_queue_limits.c.  I think that the use of POSDIFF() protects
against races that could leave completed > sent.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [RFC] tulip: Support for byte queue limits

[George Spelvin]

Grumble.  After a week of uptime with my tulip patches
(the ones I posted here plus some cleanup patches) my
kernel gave me this:

------------[ cut here ]------------
WARNING: at net/sched/sch_generic.c:255 dev_watchdog+0xd9/0x137()
NETDEV WATCHDOG: cable (tulip): transmit queue 0 timed out
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.10.2-00033-gca137af #47
Hardware name: MICRO-STAR INTERNATIONAL CO.,LTD MS-7376/MS-7376, BIOS V1.7 01/13/2009
 0000000000000000 ffffffff81028aba ffff88021fc03e68 ffff880216b38000
 ffff88021fc03eb8 ffffffff8133643a ffffffff8133643a ffffffff81028b33
 ffffffff8159fe93 ffff880200000030 ffff88021fc03ec8 ffff88021fc03e88
Call Trace:
 <IRQ>  [<ffffffff81028aba>] ? warn_slowpath_common+0x56/0x6b
 [<ffffffff8133643a>] ? qdisc_rcu_free+0x19/0x19
 [<ffffffff8133643a>] ? qdisc_rcu_free+0x19/0x19
 [<ffffffff81028b33>] ? warn_slowpath_fmt+0x47/0x49
 [<ffffffff8133634e>] ? netif_tx_lock+0x47/0x72
 [<ffffffff81336513>] ? dev_watchdog+0xd9/0x137
 [<ffffffff81031f40>] ? call_timer_fn.isra.29+0x1c/0x6f
 [<ffffffff810321a2>] ? run_timer_softirq+0x19a/0x1c2
 [<ffffffff8102de09>] ? __do_softirq+0xb9/0x171
 [<ffffffff8102df7f>] ? irq_exit+0x3a/0x7a
 [<ffffffff8101967a>] ? smp_apic_timer_interrupt+0x72/0x7e
 [<ffffffff8142a60a>] ? apic_timer_interrupt+0x6a/0x70
 <EOI>  [<ffffffff81007e34>] ? amd_e400_idle+0xbf/0xc1
 [<ffffffff81007e2c>] ? amd_e400_idle+0xb7/0xc1
 [<ffffffff8104f74d>] ? cpu_startup_entry+0x9c/0xec
 [<ffffffff8164eb91>] ? start_kernel+0x2bd/0x2c8
 [<ffffffff8164e6f7>] ? repair_env_string+0x54/0x54
---[ end trace fa3269ab5c1a15ad ]---

Since it's on the tulip device, it's probably my fault, and
since it took a week to manifest itself, it's going to be
a complete PITA to reproduce.

Anyway, I haven't forgotten.