RE: [Xen-devel][Pv-ops][PATCH 2/4 v2] Netback: Introduce a new struct type page_ext.

["Xu, Dongxiao" <dongxiao.xu@xxxxxxxxx>]

I will add this check, thanks for your reminder.

Thanks,
Dongxiao

Konrad Rzeszutek Wilk wrote:
> On Thu, Apr 29, 2010 at 10:28:36PM +0800, Xu, Dongxiao wrote:
>> Netback: Introduce a new struct type page_ext.
>> 
>> struct page_ext is used to store the group and idx information by
>> which a specified page could be identified.
>> 
>> Signed-off-by: Dongxiao Xu <dongxiao.xu@xxxxxxxxx>
> 
> .. snip..
> 
> 
> -static inline int netif_page_index(struct page *pg)
> -{
> -     unsigned long idx = (unsigned long)pg->mapping - 1;
> -
> -     if (!PageForeign(pg))
> -             return -1;
> 
> You are taking the check to see if the page is foreign. Is that OK?
> -
> -     if ((idx >= MAX_PENDING_REQS) || (netbk->mmap_pages[idx] != pg))
> -             return -1;
> -
> -     return idx;
> +     pg->mapping = (struct address_space *)page_extinfo;;
>  }
> 
>  /*
> @@ -1411,9 +1399,10 @@ static void netif_idx_release(u16 pending_idx)
> 
>  static void netif_page_release(struct page *page, unsigned int order)
>  {
> -     int idx = netif_page_index(page);
> 
> This did this nice little check to see if the page had the PageForeign
> bit set. Is it OK to remove that check?
> 
> -     BUG_ON(order);
> -     BUG_ON(idx < 0);
> +     int idx = ((struct page_ext *)(page->mapping))->idx;
> +     BUG_ON(order ||
> +            idx < 0 || idx >= MAX_PENDING_REQS ||
> +            netbk->mmap_pages[idx] != page);
>       netif_idx_release(idx);
>  }
> 
> @@ -1565,7 +1554,9 @@ static int __init netback_init(void)
>       for (i = 0; i < MAX_PENDING_REQS; i++) {
>               page = netbk->mmap_pages[i];
>               SetPageForeign(page, netif_page_release);
> -             netif_set_page_index(page, i);
> +             netbk->page_extinfo[i].group = 0;
> +             netbk->page_extinfo[i].idx = i;
> +             netif_set_page_index(page, &netbk->page_extinfo[i]);
>               INIT_LIST_HEAD(&netbk->pending_inuse[i].list);
>       }
> 
> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@xxxxxxxxxxxxxxxxxxx
>> http://lists.xensource.com/xen-devel


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