* RE: A race condition in xenlinux exit_mmap
@ 2006-08-01 10:39 Li, Xin B
2006-08-01 10:51 ` Keir Fraser
0 siblings, 1 reply; 6+ messages in thread
From: Li, Xin B @ 2006-08-01 10:39 UTC (permalink / raw)
To: Keir Fraser; +Cc: hanzhu, xen-devel
>Do you mind creating a patch to do this? I can send you more
>details if you like.
Sure, pls send more info on this.
Thanks
-Xin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: A race condition in xenlinux exit_mmap
2006-08-01 10:39 A race condition in xenlinux exit_mmap Li, Xin B
@ 2006-08-01 10:51 ` Keir Fraser
2006-08-01 13:02 ` Ben Thomas
0 siblings, 1 reply; 6+ messages in thread
From: Keir Fraser @ 2006-08-01 10:51 UTC (permalink / raw)
To: Li, Xin B; +Cc: hanzhu, xen-devel
On 1 Aug 2006, at 11:39, Li, Xin B wrote:
>> Do you mind creating a patch to do this? I can send you more
>> details if you like.
>
> Sure, pls send more info on this.
1. Add an 'int has_foreign_mappings' to mmu_context structure
2. Ensure the field is initialised in init_new_context() (e.g.,
memset-zero the whole structure; already done on x86/64)
3. Set the field in direct_remap_pfn_range()
4. Check the field in _arch_exit_mmap() to avoid calling mm_unpin().
That's it. Just needs testing.
-- Keir
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Re: A race condition in xenlinux exit_mmap
2006-08-01 10:51 ` Keir Fraser
@ 2006-08-01 13:02 ` Ben Thomas
2006-08-01 13:08 ` hanzhu
0 siblings, 1 reply; 6+ messages in thread
From: Ben Thomas @ 2006-08-01 13:02 UTC (permalink / raw)
To: Li, Xin B; +Cc: hanzhu, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 1275 bytes --]
Xin,
I'm attaching a patch that we've been using since late May/early June to
address an "Eeek" issues. Since we applied the patch, we haven't seen the
issue. As this was some time ago, I cannot recall if this is the same
problem that you're seeing now. The patch wasn't submitted, as it isn't
particularly clean. It's one of the many "some day soon" patches that we
need to get resubmitted after a bit more work. I attach it here, not because
I believe that it is "the answer", but as a data point for you.
-b
On 8/1/06, Keir Fraser <Keir.Fraser@cl.cam.ac.uk> wrote:
>
>
> On 1 Aug 2006, at 11:39, Li, Xin B wrote:
>
> >> Do you mind creating a patch to do this? I can send you more
> >> details if you like.
> >
> > Sure, pls send more info on this.
>
> 1. Add an 'int has_foreign_mappings' to mmu_context structure
> 2. Ensure the field is initialised in init_new_context() (e.g.,
> memset-zero the whole structure; already done on x86/64)
> 3. Set the field in direct_remap_pfn_range()
> 4. Check the field in _arch_exit_mmap() to avoid calling mm_unpin().
>
> That's it. Just needs testing.
>
> -- Keir
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
[-- Attachment #1.2: Type: text/html, Size: 2025 bytes --]
[-- Attachment #2: eeek.patch --]
[-- Type: text/x-patch, Size: 1977 bytes --]
# HG changeset patch
# User lively@dlively2
# Node ID 2d63622421e93895f9f5c99d900e80e78943b0c6
# Parent 0b79b3c194b17dcf4d7916e33628feb321cc1e05
Robert's workaround for the infamous Eeek! page_mapcount(page) went negative! bug.
[Bug id:] 3914
[Reviewed By:] Ben & Dave L
diff -r 0b79b3c194b1 -r 2d63622421e9 linux-2.6-xen-sparse/drivers/xen/privcmd/privcmd.c
--- a/linux-2.6-xen-sparse/drivers/xen/privcmd/privcmd.c Sat Jun 10 13:23:11 2006 -0400
+++ b/linux-2.6-xen-sparse/drivers/xen/privcmd/privcmd.c Tue Jun 13 11:05:44 2006 -0400
@@ -244,7 +244,7 @@ static int privcmd_mmap(struct file * fi
static int privcmd_mmap(struct file * file, struct vm_area_struct * vma)
{
/* DONTCOPY is essential for Xen as copy_page_range is broken. */
- vma->vm_flags |= VM_RESERVED | VM_IO | VM_DONTCOPY;
+ vma->vm_flags |= VM_RESERVED | VM_IO | VM_DONTCOPY | VM_PRIVCMD;
return 0;
}
diff -r 0b79b3c194b1 -r 2d63622421e9 linux-2.6-xen-sparse/include/linux/mm.h
--- a/linux-2.6-xen-sparse/include/linux/mm.h Sat Jun 10 13:23:11 2006 -0400
+++ b/linux-2.6-xen-sparse/include/linux/mm.h Tue Jun 13 11:05:44 2006 -0400
@@ -169,6 +169,7 @@ extern unsigned int kobjsize(const void
#ifdef CONFIG_XEN
#define VM_FOREIGN 0x04000000 /* Has pages belonging to another VM */
#endif
+#define VM_PRIVCMD 0x08000000 /* Pages belong to privcmd mmap */
#ifndef VM_STACK_DEFAULT_FLAGS /* arch can override this */
#define VM_STACK_DEFAULT_FLAGS VM_DATA_DEFAULT_FLAGS
diff -r 0b79b3c194b1 -r 2d63622421e9 linux-2.6-xen-sparse/mm/memory.c
--- a/linux-2.6-xen-sparse/mm/memory.c Sat Jun 10 13:23:11 2006 -0400
+++ b/linux-2.6-xen-sparse/mm/memory.c Tue Jun 13 11:05:44 2006 -0400
@@ -409,6 +409,10 @@ struct page *vm_normal_page(struct vm_ar
print_bad_pte(vma, pte, addr);
return NULL;
}
+
+ /* This vma points to foreign pages */
+ if (vma->vm_flags & VM_PRIVCMD)
+ return NULL;
/*
* NOTE! We still have PageReserved() pages in the page
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Re: A race condition in xenlinux exit_mmap
2006-08-01 13:02 ` Ben Thomas
@ 2006-08-01 13:08 ` hanzhu
2006-08-01 13:23 ` Ben Thomas
0 siblings, 1 reply; 6+ messages in thread
From: hanzhu @ 2006-08-01 13:08 UTC (permalink / raw)
To: ben; +Cc: Li, Xin B, xen-devel
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=gb18030; format=flowed, Size: 3840 bytes --]
This patch just fail the sanity check. It should fix the bug. However,
it didn't fix the root cause.
I'm afraid Keir will not allow to add another VM_XXX flag.
_______________________________________________________
Best Regards,
hanzhu
Ben Thomas дµÀ:
> Xin,
>
> I'm attaching a patch that we've been using since late May/early June to
> address an "Eeek" issues. Since we applied the patch, we haven't seen the
> issue. As this was some time ago, I cannot recall if this is the same
> problem that you're seeing now. The patch wasn't submitted, as it isn't
> particularly clean. It's one of the many "some day soon" patches that we
> need to get resubmitted after a bit more work. I attach it here, not
> because
> I believe that it is "the answer", but as a data point for you.
>
> -b
>
>
> On 8/1/06, Keir Fraser <Keir.Fraser@cl.cam.ac.uk> wrote:
>>
>>
>> On 1 Aug 2006, at 11:39, Li, Xin B wrote:
>>
>> >> Do you mind creating a patch to do this? I can send you more
>> >> details if you like.
>> >
>> > Sure, pls send more info on this.
>>
>> 1. Add an 'int has_foreign_mappings' to mmu_context structure
>> 2. Ensure the field is initialised in init_new_context() (e.g.,
>> memset-zero the whole structure; already done on x86/64)
>> 3. Set the field in direct_remap_pfn_range()
>> 4. Check the field in _arch_exit_mmap() to avoid calling mm_unpin().
>>
>> That's it. Just needs testing.
>>
>> -- Keir
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>>
>
> ------------------------------------------------------------------------
>
> # HG changeset patch
> # User lively@dlively2
> # Node ID 2d63622421e93895f9f5c99d900e80e78943b0c6
> # Parent 0b79b3c194b17dcf4d7916e33628feb321cc1e05
> Robert's workaround for the infamous Eeek! page_mapcount(page) went negative! bug.
>
> [Bug id:] 3914
>
> [Reviewed By:] Ben & Dave L
>
> diff -r 0b79b3c194b1 -r 2d63622421e9 linux-2.6-xen-sparse/drivers/xen/privcmd/privcmd.c
> --- a/linux-2.6-xen-sparse/drivers/xen/privcmd/privcmd.c Sat Jun 10 13:23:11 2006 -0400
> +++ b/linux-2.6-xen-sparse/drivers/xen/privcmd/privcmd.c Tue Jun 13 11:05:44 2006 -0400
> @@ -244,7 +244,7 @@ static int privcmd_mmap(struct file * fi
> static int privcmd_mmap(struct file * file, struct vm_area_struct * vma)
> {
> /* DONTCOPY is essential for Xen as copy_page_range is broken. */
> - vma->vm_flags |= VM_RESERVED | VM_IO | VM_DONTCOPY;
> + vma->vm_flags |= VM_RESERVED | VM_IO | VM_DONTCOPY | VM_PRIVCMD;
>
> return 0;
> }
> diff -r 0b79b3c194b1 -r 2d63622421e9 linux-2.6-xen-sparse/include/linux/mm.h
> --- a/linux-2.6-xen-sparse/include/linux/mm.h Sat Jun 10 13:23:11 2006 -0400
> +++ b/linux-2.6-xen-sparse/include/linux/mm.h Tue Jun 13 11:05:44 2006 -0400
> @@ -169,6 +169,7 @@ extern unsigned int kobjsize(const void
> #ifdef CONFIG_XEN
> #define VM_FOREIGN 0x04000000 /* Has pages belonging to another VM */
> #endif
> +#define VM_PRIVCMD 0x08000000 /* Pages belong to privcmd mmap */
>
> #ifndef VM_STACK_DEFAULT_FLAGS /* arch can override this */
> #define VM_STACK_DEFAULT_FLAGS VM_DATA_DEFAULT_FLAGS
> diff -r 0b79b3c194b1 -r 2d63622421e9 linux-2.6-xen-sparse/mm/memory.c
> --- a/linux-2.6-xen-sparse/mm/memory.c Sat Jun 10 13:23:11 2006 -0400
> +++ b/linux-2.6-xen-sparse/mm/memory.c Tue Jun 13 11:05:44 2006 -0400
> @@ -409,6 +409,10 @@ struct page *vm_normal_page(struct vm_ar
> print_bad_pte(vma, pte, addr);
> return NULL;
> }
> +
> + /* This vma points to foreign pages */
> + if (vma->vm_flags & VM_PRIVCMD)
> + return NULL;
>
> /*
> * NOTE! We still have PageReserved() pages in the page
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Re: A race condition in xenlinux exit_mmap
2006-08-01 13:08 ` hanzhu
@ 2006-08-01 13:23 ` Ben Thomas
0 siblings, 0 replies; 6+ messages in thread
From: Ben Thomas @ 2006-08-01 13:23 UTC (permalink / raw)
To: hanzhu; +Cc: Li, Xin B, xen-devel, ben
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=gb18030; format=flowed, Size: 1422 bytes --]
I agree. This is why we never submitted it, and I
wanted you to understand that it was a "data point"
and not an answer.
regards,
-b
hanzhu wrote:
> This patch just fail the sanity check. It should fix the bug. However,
> it didn't fix the root cause.
> I'm afraid Keir will not allow to add another VM_XXX flag.
>
> _______________________________________________________
> Best Regards,
> hanzhu
>
>
>
> Ben Thomas дµÀ:
>
>> Xin,
>>
>> I'm attaching a patch that we've been using since late May/early June to
>> address an "Eeek" issues. Since we applied the patch, we haven't seen the
>> issue. As this was some time ago, I cannot recall if this is the same
>> problem that you're seeing now. The patch wasn't submitted, as it isn't
>> particularly clean. It's one of the many "some day soon" patches that we
>> need to get resubmitted after a bit more work. I attach it here, not
>> because
>> I believe that it is "the answer", but as a data point for you.
>>
>> -b
> /*
>> * NOTE! We still have PageReserved() pages in the page
--
------------------------------------------------------------------------
Ben Thomas Virtual Iron Software
bthomas@virtualiron.com Tower 1, Floor 2
978-849-1214 900 Chelmsford Street
Lowell, MA 01851
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: Re: A race condition in xenlinux exit_mmap
@ 2006-08-01 13:53 Li, Xin B
0 siblings, 0 replies; 6+ messages in thread
From: Li, Xin B @ 2006-08-01 13:53 UTC (permalink / raw)
To: ben; +Cc: hanzhu, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 1635 bytes --]
yes, elegant fix is not easy.
-Xin
________________________________
From: bjthomas3@gmail.com [mailto:bjthomas3@gmail.com] On Behalf Of Ben Thomas
Sent: 2006年8月1日 21:03
To: Li, Xin B
Cc: hanzhu@sjtu.edu.cn; xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] Re: A race condition in xenlinux exit_mmap
Xin,
I'm attaching a patch that we've been using since late May/early June to address an "Eeek" issues. Since we applied the patch, we haven't seen the issue. As this was some time ago, I cannot recall if this is the same problem that you're seeing now. The patch wasn't submitted, as it isn't particularly clean. It's one of the many "some day soon" patches that we need to get resubmitted after a bit more work. I attach it here, not because I believe that it is "the answer", but as a data point for you.
-b
On 8/1/06, Keir Fraser <Keir.Fraser@cl.cam.ac.uk > wrote:
On 1 Aug 2006, at 11:39, Li, Xin B wrote:
>> Do you mind creating a patch to do this? I can send you more
>> details if you like.
>
> Sure, pls send more info on this.
1. Add an 'int has_foreign_mappings' to mmu_context structure
2. Ensure the field is initialised in init_new_context() (e.g.,
memset-zero the whole structure; already done on x86/64)
3. Set the field in direct_remap_pfn_range()
4. Check the field in _arch_exit_mmap() to avoid calling mm_unpin().
That's it. Just needs testing.
-- Keir
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
[-- Attachment #1.2: Type: text/html, Size: 3185 bytes --]
[-- Attachment #2: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-08-01 13:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-08-01 10:39 A race condition in xenlinux exit_mmap Li, Xin B
2006-08-01 10:51 ` Keir Fraser
2006-08-01 13:02 ` Ben Thomas
2006-08-01 13:08 ` hanzhu
2006-08-01 13:23 ` Ben Thomas
-- strict thread matches above, loose matches on Subject: below --
2006-08-01 13:53 Li, Xin B
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.