2006-10-16 01:47:56

by David Gibson

Subject: Re: Failed to boot kernel 2.6.19-rc2 due to IBM veth problem.

On Mon, Oct 16, 2006 at 12:26:37AM +0800, Yao Fei Zhu wrote:
> Hi, all,
>
> Boot kernel 2.6.19-rc2 on IBM System P5 partitions will fall into xmon.
> Here is the boot log,

This is probably the same bug I recently posted about. The patch
below should fix it.

ibmveth: Fix index increment calculation

The recent commit 751ae21c6cd1493e3d0a4935b08fb298b9d89773 introduced
a bug in the producer/consumer index calculation in the ibmveth driver
- incautious use of the post-increment ++ operator resulted in an
increment being immediately reverted. This patch corrects the logic.

Without this patch, the driver oopses almost immediately after
activation on at least some machines.

Signed-off-by: David Gibson <[email protected]>

Index: working-2.6/drivers/net/ibmveth.c
===================================================================
--- working-2.6.orig/drivers/net/ibmveth.c 2006-10-13 14:19:54.000000000 +1000
+++ working-2.6/drivers/net/ibmveth.c 2006-10-13 14:19:59.000000000 +1000
@@ -212,8 +212,8 @@ static void ibmveth_replenish_buffer_poo
break;
}

- free_index = pool->consumer_index++ % pool->size;
- pool->consumer_index = free_index;
+ free_index = pool->consumer_index;
+ pool->consumer_index = (pool->consumer_index + 1) % pool->size;
index = pool->free_map[free_index];

ibmveth_assert(index != IBM_VETH_INVALID_MAP);
@@ -329,8 +329,10 @@ static void ibmveth_remove_buffer_from_p
adapter->rx_buff_pool[pool].buff_size,
DMA_FROM_DEVICE);

- free_index = adapter->rx_buff_pool[pool].producer_index++ % adapter->rx_buff_pool[pool].size;
- adapter->rx_buff_pool[pool].producer_index = free_index;
+ free_index = adapter->rx_buff_pool[pool].producer_index;
+ adapter->rx_buff_pool[pool].producer_index
+ = (adapter->rx_buff_pool[pool].producer_index + 1)
+ % adapter->rx_buff_pool[pool].size;
adapter->rx_buff_pool[pool].free_map[free_index] = index;

mb();


--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson

2006-10-16 17:44:31

by Yao Fei Zhu

Subject: Re: Failed to boot kernel 2.6.19-rc2 due to IBM veth problem.

David Gibson 写道:
> On Mon, Oct 16, 2006 at 12:26:37AM +0800, Yao Fei Zhu wrote:
>
>>Hi, all,
>>
>>Boot kernel 2.6.19-rc2 on IBM System P5 partitions will fall into xmon.
>>Here is the boot log,
>
>
> This is probably the same bug I recently posted about. The patch
> below should fix it.
>
> ibmveth: Fix index increment calculation
>
> The recent commit 751ae21c6cd1493e3d0a4935b08fb298b9d89773 introduced
> a bug in the producer/consumer index calculation in the ibmveth driver
> - incautious use of the post-increment ++ operator resulted in an
> increment being immediately reverted. This patch corrects the logic.
>
> Without this patch, the driver oopses almost immediately after
> activation on at least some machines.
>
> Signed-off-by: David Gibson <[email protected]>
>
> Index: working-2.6/drivers/net/ibmveth.c
> ===================================================================
> --- working-2.6.orig/drivers/net/ibmveth.c 2006-10-13 14:19:54.000000000 +1000
> +++ working-2.6/drivers/net/ibmveth.c 2006-10-13 14:19:59.000000000 +1000
> @@ -212,8 +212,8 @@ static void ibmveth_replenish_buffer_poo
> break;
> }
>
> - free_index = pool->consumer_index++ % pool->size;
> - pool->consumer_index = free_index;
> + free_index = pool->consumer_index;
> + pool->consumer_index = (pool->consumer_index + 1) % pool->size;
> index = pool->free_map[free_index];
>
> ibmveth_assert(index != IBM_VETH_INVALID_MAP);
> @@ -329,8 +329,10 @@ static void ibmveth_remove_buffer_from_p
> adapter->rx_buff_pool[pool].buff_size,
> DMA_FROM_DEVICE);
>
> - free_index = adapter->rx_buff_pool[pool].producer_index++ % adapter->rx_buff_pool[pool].size;
> - adapter->rx_buff_pool[pool].producer_index = free_index;
> + free_index = adapter->rx_buff_pool[pool].producer_index;
> + adapter->rx_buff_pool[pool].producer_index
> + = (adapter->rx_buff_pool[pool].producer_index + 1)
> + % adapter->rx_buff_pool[pool].size;
> adapter->rx_buff_pool[pool].free_map[free_index] = index;
>
> mb();
>
>
David, I have verified this fix, it works fine for me, Thanks. What's the status of it? Submitted?

2006-10-17 00:16:07

by David Gibson

Subject: Re: Failed to boot kernel 2.6.19-rc2 due to IBM veth problem.

On Tue, Oct 17, 2006 at 01:44:09AM +0800, Yao Fei Zhu wrote:
[snip]
> >
> David, I have verified this fix, it works fine for me, Thanks. What's the status of it? Submitted?

Yes, I've sent it to Santiago Leon, the ibmveth maintainer and also to
Jeff Garzik and Andrew Morton. It is in Andrew's -mm tree already,
haven't heard from Santiago or Jeff.

--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson