Re: [Xen-devel] [PATCH] linux-2.6.18/netback: use correct index for invalidation in netbk_tx_check_mop()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.11.11 at 17:25, Laszlo Ersek <lersek@xxxxxxxxxx> wrote:
> On 11/18/11 14:15, Jan Beulich wrote:
> 
>> --- a/drivers/xen/netback/netback.c
>> +++ b/drivers/xen/netback/netback.c
>> @@ -1155,7 +1155,7 @@ static int netbk_tx_check_mop(struct sk_
>>              pending_idx = *((u16 *)skb->data);
>>              netif_idx_release(pending_idx);
>>              for (j = start; j<  i; j++) {
>> -                    pending_idx = (unsigned long)shinfo->frags[i].page;
>> +                    pending_idx = (unsigned long)shinfo->frags[j].page;
>>                      netif_idx_release(pending_idx);
>>              }
> 
> Please excuse the uneducated question: what could be the consequences of 
> using the wrong index here? I notice that with the fix, frags[i].page is 
> never touched, while without the fix, it is the only one that's released.

The index correlating to i gets released right before the loop. The
consequences of using the wrong index in the loop are - afaict -
that guests may cause netback (and perhaps Dom0 as a whole) to
run out of certain resources (grant table entries come to mind).

> I'm asking because we have an elusive bug: netloop sometimes crashes in 
> skb_remove_foreign_references(). As of 1125:985b8f62df25, the crash happens 
> on line 118:
> 
>    116                  vaddr = kmap_skb_frag(&skb_shinfo(skb)->frags[i]);
>    117                  off = skb_shinfo(skb)->frags[i].page_offset;
>    118                  memcpy(page_address(page) + off,
>    119                         vaddr + off,
>    120                         skb_shinfo(skb)->frags[i].size);
> 
> because the PTE for the vaddr returned by kmap_skb_frag() for the first frag 
> is 0.
> 
> It appears somehow related to NFS and closing/reopening TCP connections for 
> NFS.

Sorry, I don't see any connection to the above, but then again I didn't
find the bug above due to actual misbehavior, but just due to having
worked on that code while moving our patch set forward to 3.2-rc.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel