Re: [Xen-devel] linux c/s 866

[Isaku Yamahata <yamahata@xxxxxxxxxxxxx>]

On Mon, May 04, 2009 at 10:14:17AM +0100, Jan Beulich wrote:
> >@@ -329,11 +329,15 @@ static pte_t blktap_clear_pte(struct vm_
> >      * If the address is before the start of the grant mapped region or
> >      * if vm_file is NULL (meaning mmap failed and we have nothing to do)
> >      */
> >-    if (uvaddr < uvstart || vma->vm_file == NULL)
> >+    if (vma->vm_file != NULL) {
> >+            info = vma->vm_file->private_data;
> >+            uvstart = info->rings_vstart + (RING_PAGES << PAGE_SHIFT);
> >+    } else
> >+            uvstart = uvaddr;       /* make the following if clause true */
> >+    if (uvaddr < uvstart)
> >             return ptep_get_and_clear_full(vma->vm_mm, uvaddr, 
> >                                            ptep, is_fullmm);
> 
> It would appear to me as if the comment and the code are out of sync:
> Assigning uvaddr to uvstart makes the if clause's condition false whenever
> vm->vm_file == NULL. According to how the old code worked, I'd say the
> code needs to be fixed, but I'm not certain, hence I'm hesitant to submit a
> patch...

You're quite right. Thank you for pointing out.


linux/blktap: fix blktap_clear_pte().

fix blktap_clear_pte(). In case of vma->vm_file == NULL
case wasn't handled correctly.

Signed-off-by: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>

diff --git a/drivers/xen/blktap/blktap.c b/drivers/xen/blktap/blktap.c
--- a/drivers/xen/blktap/blktap.c
+++ b/drivers/xen/blktap/blktap.c
@@ -314,7 +314,7 @@ static pte_t blktap_clear_pte(struct vm_
        pte_t copy;
        tap_blkif_t *info = NULL;
        int offset, seg, usr_idx, pending_idx, mmap_idx;
-       unsigned long uvstart;
+       unsigned long uvstart = 0;
        unsigned long kvaddr;
        struct page *pg;
        struct grant_handle_pair *khandle;
@@ -328,9 +328,8 @@ static pte_t blktap_clear_pte(struct vm_
        if (vma->vm_file != NULL) {
                info = vma->vm_file->private_data;
                uvstart = info->rings_vstart + (RING_PAGES << PAGE_SHIFT);
-       } else
-               uvstart = uvaddr;       /* make the following if clause true */
-       if (uvaddr < uvstart)
+       }
+       if (vma->vm_file == NULL || uvaddr < uvstart)
                return ptep_get_and_clear_full(vma->vm_mm, uvaddr, 
                                               ptep, is_fullmm);
 

-- 
yamahata

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