* [Revert] Re: [PATCH] mm: sync vmalloc address space page tables in alloc_vm_area()
[not found] <1314877863-21977-1-git-send-email-david.vrabel@citrix.com>
@ 2011-09-01 16:11 ` Konrad Rzeszutek Wilk
2011-09-01 20:37 ` Jeremy Fitzhardinge
0 siblings, 1 reply; 14+ messages in thread
From: Konrad Rzeszutek Wilk @ 2011-09-01 16:11 UTC (permalink / raw)
To: David Vrabel, linux-kernel, akpm, namhyung, rientjes, linux-mm
Cc: xen-devel, Jeremy Fitzhardinge, paulmck
On Thu, Sep 01, 2011 at 12:51:03PM +0100, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
Andrew,
I was wondering if you would be Ok with this patch for 3.1.
It is a revert (I can prepare a proper revert if you would like
that instead of this patch).
The users of this particular function (alloc_vm_area) are just
Xen. There are no others.
>
> Xen backend drivers (e.g., blkback and netback) would sometimes fail
> to map grant pages into the vmalloc address space allocated with
> alloc_vm_area(). The GNTTABOP_map_grant_ref would fail because Xen
> could not find the page (in the L2 table) containing the PTEs it
> needed to update.
>
> (XEN) mm.c:3846:d0 Could not find L1 PTE for address fbb42000
>
> netback and blkback were making the hypercall from a kernel thread
> where task->active_mm != &init_mm and alloc_vm_area() was only
> updating the page tables for init_mm. The usual method of deferring
> the update to the page tables of other processes (i.e., after taking a
> fault) doesn't work as a fault cannot occur during the hypercall.
>
> This would work on some systems depending on what else was using
> vmalloc.
>
> Fix this by reverting ef691947d8a3d479e67652312783aedcf629320a
> (vmalloc: remove vmalloc_sync_all() from alloc_vm_area()) and add a
> comment to explain why it's needed.
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> ---
> mm/vmalloc.c | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 7ef0903..5016f19 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -2140,6 +2140,14 @@ struct vm_struct *alloc_vm_area(size_t size)
> return NULL;
> }
>
> + /*
> + * If the allocated address space is passed to a hypercall
> + * before being used then we cannot rely on a page fault to
> + * trigger an update of the page tables. So sync all the page
> + * tables here.
> + */
> + vmalloc_sync_all();
> +
> return area;
> }
> EXPORT_SYMBOL_GPL(alloc_vm_area);
> --
> 1.7.2.5
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Revert] Re: [PATCH] mm: sync vmalloc address space page tables in alloc_vm_area()
2011-09-01 16:11 ` [Revert] Re: [PATCH] mm: sync vmalloc address space page tables in alloc_vm_area() Konrad Rzeszutek Wilk
@ 2011-09-01 20:37 ` Jeremy Fitzhardinge
2011-09-01 21:17 ` Andrew Morton
2011-09-02 7:22 ` Ian Campbell
0 siblings, 2 replies; 14+ messages in thread
From: Jeremy Fitzhardinge @ 2011-09-01 20:37 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: David Vrabel, linux-kernel@vger.kernel.org,
akpm@linux-foundation.org, namhyung@gmail.com,
rientjes@google.com, linux-mm@kvack.org,
xen-devel@lists.xensource.com, paulmck@linux.vnet.ibm.com
On 09/01/2011 09:11 AM, Konrad Rzeszutek Wilk wrote:
> On Thu, Sep 01, 2011 at 12:51:03PM +0100, David Vrabel wrote:
>> From: David Vrabel <david.vrabel@citrix.com>
> Andrew,
>
> I was wondering if you would be Ok with this patch for 3.1.
>
> It is a revert (I can prepare a proper revert if you would like
> that instead of this patch).
>
> The users of this particular function (alloc_vm_area) are just
> Xen. There are no others.
I'd prefer to put explicit vmalloc_sync_all()s in the callsites where
necessary, and ultimately try to work out ways of avoiding it altogether
(like have some hypercall wrapper which touches the arg memory to make
sure its mapped?).
J
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Revert] Re: [PATCH] mm: sync vmalloc address space page tables in alloc_vm_area()
2011-09-01 20:37 ` Jeremy Fitzhardinge
@ 2011-09-01 21:17 ` Andrew Morton
2011-09-02 11:39 ` David Vrabel
2011-09-02 7:22 ` Ian Campbell
1 sibling, 1 reply; 14+ messages in thread
From: Andrew Morton @ 2011-09-01 21:17 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: Konrad Rzeszutek Wilk, David Vrabel, linux-kernel@vger.kernel.org,
namhyung@gmail.com, rientjes@google.com, linux-mm@kvack.org,
xen-devel@lists.xensource.com, paulmck@linux.vnet.ibm.com
On Thu, 01 Sep 2011 13:37:46 -0700
Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> On 09/01/2011 09:11 AM, Konrad Rzeszutek Wilk wrote:
> > On Thu, Sep 01, 2011 at 12:51:03PM +0100, David Vrabel wrote:
> >> From: David Vrabel <david.vrabel@citrix.com>
> > Andrew,
> >
> > I was wondering if you would be Ok with this patch for 3.1.
> >
> > It is a revert (I can prepare a proper revert if you would like
> > that instead of this patch).
David's patch looks better than a straight reversion.
Problem is, I can't find David's original email anywhere. Someone's
been playing games with To: headers?
> > The users of this particular function (alloc_vm_area) are just
> > Xen. There are no others.
>
> I'd prefer to put explicit vmalloc_sync_all()s in the callsites where
> necessary,
What would that patch look like? Bear in mind that we'll need something
suitable for 3.1 and for a 3.0 backport.
> and ultimately try to work out ways of avoiding it altogether
> (like have some hypercall wrapper which touches the arg memory to make
> sure its mapped?).
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Xen-devel] Re: [Revert] Re: [PATCH] mm: sync vmalloc address space page tables in alloc_vm_area()
2011-09-01 20:37 ` Jeremy Fitzhardinge
2011-09-01 21:17 ` Andrew Morton
@ 2011-09-02 7:22 ` Ian Campbell
2011-09-02 7:31 ` Keir Fraser
1 sibling, 1 reply; 14+ messages in thread
From: Ian Campbell @ 2011-09-02 7:22 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: Konrad Rzeszutek Wilk, xen-devel@lists.xensource.com,
namhyung@gmail.com, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, David Vrabel, rientjes@google.com,
akpm@linux-foundation.org, paulmck@linux.vnet.ibm.com
On Thu, 2011-09-01 at 21:37 +0100, Jeremy Fitzhardinge wrote:
> On 09/01/2011 09:11 AM, Konrad Rzeszutek Wilk wrote:
> > On Thu, Sep 01, 2011 at 12:51:03PM +0100, David Vrabel wrote:
> >> From: David Vrabel <david.vrabel@citrix.com>
> > Andrew,
> >
> > I was wondering if you would be Ok with this patch for 3.1.
> >
> > It is a revert (I can prepare a proper revert if you would like
> > that instead of this patch).
> >
> > The users of this particular function (alloc_vm_area) are just
> > Xen. There are no others.
>
> I'd prefer to put explicit vmalloc_sync_all()s in the callsites where
> necessary, and ultimately try to work out ways of avoiding it altogether
> (like have some hypercall wrapper which touches the arg memory to make
> sure its mapped?).
That only syncs the current pagetable though. If that is sufficient (and
it could well be) then perhaps just doing a vmalloc_sync_one on the
current page tables directly would be better than faulting to do it?
It's the sort of thing you could hide inside the gnttab_set_map_op type
helpers I guess?
Ian.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Xen-devel] Re: [Revert] Re: [PATCH] mm: sync vmalloc address space page tables in alloc_vm_area()
2011-09-02 7:22 ` Ian Campbell
@ 2011-09-02 7:31 ` Keir Fraser
0 siblings, 0 replies; 14+ messages in thread
From: Keir Fraser @ 2011-09-02 7:31 UTC (permalink / raw)
To: Ian Campbell, Jeremy Fitzhardinge
Cc: xen-devel@lists.xensource.com, Konrad Rzeszutek Wilk,
namhyung@gmail.com, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, David Vrabel, rientjes@google.com,
akpm@linux-foundation.org, paulmck@linux.vnet.ibm.com
On 02/09/2011 08:22, "Ian Campbell" <Ian.Campbell@citrix.com> wrote:
> On Thu, 2011-09-01 at 21:37 +0100, Jeremy Fitzhardinge wrote:
>> On 09/01/2011 09:11 AM, Konrad Rzeszutek Wilk wrote:
>>> On Thu, Sep 01, 2011 at 12:51:03PM +0100, David Vrabel wrote:
>>>> From: David Vrabel <david.vrabel@citrix.com>
>>> Andrew,
>>>
>>> I was wondering if you would be Ok with this patch for 3.1.
>>>
>>> It is a revert (I can prepare a proper revert if you would like
>>> that instead of this patch).
>>>
>>> The users of this particular function (alloc_vm_area) are just
>>> Xen. There are no others.
>>
>> I'd prefer to put explicit vmalloc_sync_all()s in the callsites where
>> necessary, and ultimately try to work out ways of avoiding it altogether
>> (like have some hypercall wrapper which touches the arg memory to make
>> sure its mapped?).
>
> That only syncs the current pagetable though. If that is sufficient (and
> it could well be) then perhaps just doing a vmalloc_sync_one on the
> current page tables directly would be better than faulting to do it?
It's probably sufficient unless the kernel has non-voluntary preemption, and
we are not preempt_disable()d.
-- Keir
> It's the sort of thing you could hide inside the gnttab_set_map_op type
> helpers I guess?
>
> Ian.
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Revert] Re: [PATCH] mm: sync vmalloc address space page tables in alloc_vm_area()
2011-09-01 21:17 ` Andrew Morton
@ 2011-09-02 11:39 ` David Vrabel
2011-09-02 22:32 ` Andrew Morton
0 siblings, 1 reply; 14+ messages in thread
From: David Vrabel @ 2011-09-02 11:39 UTC (permalink / raw)
To: Andrew Morton
Cc: Jeremy Fitzhardinge, Konrad Rzeszutek Wilk,
linux-kernel@vger.kernel.org, namhyung@gmail.com,
rientjes@google.com, linux-mm@kvack.org,
xen-devel@lists.xensource.com, paulmck@linux.vnet.ibm.com
On 01/09/11 22:17, Andrew Morton wrote:
> On Thu, 01 Sep 2011 13:37:46 -0700
> Jeremy Fitzhardinge <jeremy@goop.org> wrote:
>
>> On 09/01/2011 09:11 AM, Konrad Rzeszutek Wilk wrote:
>>> On Thu, Sep 01, 2011 at 12:51:03PM +0100, David Vrabel wrote:
>>>> From: David Vrabel <david.vrabel@citrix.com>
>>> Andrew,
>>>
>>> I was wondering if you would be Ok with this patch for 3.1.
>>>
>>> It is a revert (I can prepare a proper revert if you would like
>>> that instead of this patch).
>
> David's patch looks better than a straight reversion.
>
> Problem is, I can't find David's original email anywhere. Someone's
> been playing games with To: headers?
Sorry, I should have Cc'd linux-kernel and others on the original patch.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Revert] Re: [PATCH] mm: sync vmalloc address space page tables in alloc_vm_area()
2011-09-02 11:39 ` David Vrabel
@ 2011-09-02 22:32 ` Andrew Morton
2011-09-02 23:04 ` Jeremy Fitzhardinge
2011-09-06 16:35 ` [Xen-devel] " Konrad Rzeszutek Wilk
0 siblings, 2 replies; 14+ messages in thread
From: Andrew Morton @ 2011-09-02 22:32 UTC (permalink / raw)
To: David Vrabel
Cc: Jeremy Fitzhardinge, Konrad Rzeszutek Wilk,
linux-kernel@vger.kernel.org, namhyung@gmail.com,
rientjes@google.com, linux-mm@kvack.org,
xen-devel@lists.xensource.com, paulmck@linux.vnet.ibm.com
On Fri, 2 Sep 2011 12:39:19 +0100
David Vrabel <david.vrabel@citrix.com> wrote:
> Xen backend drivers (e.g., blkback and netback) would sometimes fail
> to map grant pages into the vmalloc address space allocated with
> alloc_vm_area(). The GNTTABOP_map_grant_ref would fail because Xen
> could not find the page (in the L2 table) containing the PTEs it
> needed to update.
>
> (XEN) mm.c:3846:d0 Could not find L1 PTE for address fbb42000
>
> netback and blkback were making the hypercall from a kernel thread
> where task->active_mm != &init_mm and alloc_vm_area() was only
> updating the page tables for init_mm. The usual method of deferring
> the update to the page tables of other processes (i.e., after taking a
> fault) doesn't work as a fault cannot occur during the hypercall.
>
> This would work on some systems depending on what else was using
> vmalloc.
>
> Fix this by reverting ef691947d8a3d479e67652312783aedcf629320a
> (vmalloc: remove vmalloc_sync_all() from alloc_vm_area()) and add a
> comment to explain why it's needed.
oookay, I queued this for 3.1 and tagged it for a 3.0.x backport. I
*think* that's the outcome of this discussion, for the short-term?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Revert] Re: [PATCH] mm: sync vmalloc address space page tables in alloc_vm_area()
2011-09-02 22:32 ` Andrew Morton
@ 2011-09-02 23:04 ` Jeremy Fitzhardinge
2011-09-06 16:35 ` [Xen-devel] " Konrad Rzeszutek Wilk
1 sibling, 0 replies; 14+ messages in thread
From: Jeremy Fitzhardinge @ 2011-09-02 23:04 UTC (permalink / raw)
To: Andrew Morton
Cc: David Vrabel, Konrad Rzeszutek Wilk, linux-kernel@vger.kernel.org,
namhyung@gmail.com, rientjes@google.com, linux-mm@kvack.org,
xen-devel@lists.xensource.com, paulmck@linux.vnet.ibm.com
On 09/02/2011 03:32 PM, Andrew Morton wrote:
> On Fri, 2 Sep 2011 12:39:19 +0100
> David Vrabel <david.vrabel@citrix.com> wrote:
>
>> Xen backend drivers (e.g., blkback and netback) would sometimes fail
>> to map grant pages into the vmalloc address space allocated with
>> alloc_vm_area(). The GNTTABOP_map_grant_ref would fail because Xen
>> could not find the page (in the L2 table) containing the PTEs it
>> needed to update.
>>
>> (XEN) mm.c:3846:d0 Could not find L1 PTE for address fbb42000
>>
>> netback and blkback were making the hypercall from a kernel thread
>> where task->active_mm != &init_mm and alloc_vm_area() was only
>> updating the page tables for init_mm. The usual method of deferring
>> the update to the page tables of other processes (i.e., after taking a
>> fault) doesn't work as a fault cannot occur during the hypercall.
>>
>> This would work on some systems depending on what else was using
>> vmalloc.
>>
>> Fix this by reverting ef691947d8a3d479e67652312783aedcf629320a
>> (vmalloc: remove vmalloc_sync_all() from alloc_vm_area()) and add a
>> comment to explain why it's needed.
> oookay, I queued this for 3.1 and tagged it for a 3.0.x backport. I
> *think* that's the outcome of this discussion, for the short-term?
Sure, that will get things going for now.
Thanks,
J
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Xen-devel] Re: [Revert] Re: [PATCH] mm: sync vmalloc address space page tables in alloc_vm_area()
2011-09-02 22:32 ` Andrew Morton
2011-09-02 23:04 ` Jeremy Fitzhardinge
@ 2011-09-06 16:35 ` Konrad Rzeszutek Wilk
2011-11-05 13:38 ` Konrad Rzeszutek Wilk
1 sibling, 1 reply; 14+ messages in thread
From: Konrad Rzeszutek Wilk @ 2011-09-06 16:35 UTC (permalink / raw)
To: Andrew Morton
Cc: David Vrabel, Jeremy Fitzhardinge, xen-devel@lists.xensource.com,
namhyung@gmail.com, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, rientjes@google.com,
paulmck@linux.vnet.ibm.com
On Fri, Sep 02, 2011 at 03:32:04PM -0700, Andrew Morton wrote:
> On Fri, 2 Sep 2011 12:39:19 +0100
> David Vrabel <david.vrabel@citrix.com> wrote:
>
> > Xen backend drivers (e.g., blkback and netback) would sometimes fail
> > to map grant pages into the vmalloc address space allocated with
> > alloc_vm_area(). The GNTTABOP_map_grant_ref would fail because Xen
> > could not find the page (in the L2 table) containing the PTEs it
> > needed to update.
> >
> > (XEN) mm.c:3846:d0 Could not find L1 PTE for address fbb42000
> >
> > netback and blkback were making the hypercall from a kernel thread
> > where task->active_mm != &init_mm and alloc_vm_area() was only
> > updating the page tables for init_mm. The usual method of deferring
> > the update to the page tables of other processes (i.e., after taking a
> > fault) doesn't work as a fault cannot occur during the hypercall.
> >
> > This would work on some systems depending on what else was using
> > vmalloc.
> >
> > Fix this by reverting ef691947d8a3d479e67652312783aedcf629320a
> > (vmalloc: remove vmalloc_sync_all() from alloc_vm_area()) and add a
> > comment to explain why it's needed.
>
> oookay, I queued this for 3.1 and tagged it for a 3.0.x backport. I
> *think* that's the outcome of this discussion, for the short-term?
<nods> Yup. Thanks!
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Xen-devel] Re: [Revert] Re: [PATCH] mm: sync vmalloc address space page tables in alloc_vm_area()
2011-09-06 16:35 ` [Xen-devel] " Konrad Rzeszutek Wilk
@ 2011-11-05 13:38 ` Konrad Rzeszutek Wilk
2011-11-07 20:36 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 14+ messages in thread
From: Konrad Rzeszutek Wilk @ 2011-11-05 13:38 UTC (permalink / raw)
To: Andrew Morton
Cc: David Vrabel, Jeremy Fitzhardinge, xen-devel@lists.xensource.com,
namhyung@gmail.com, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, rientjes@google.com,
paulmck@linux.vnet.ibm.com
On Tue, Sep 06, 2011 at 12:35:53PM -0400, Konrad Rzeszutek Wilk wrote:
> On Fri, Sep 02, 2011 at 03:32:04PM -0700, Andrew Morton wrote:
> > On Fri, 2 Sep 2011 12:39:19 +0100
> > David Vrabel <david.vrabel@citrix.com> wrote:
> >
> > > Xen backend drivers (e.g., blkback and netback) would sometimes fail
> > > to map grant pages into the vmalloc address space allocated with
> > > alloc_vm_area(). The GNTTABOP_map_grant_ref would fail because Xen
> > > could not find the page (in the L2 table) containing the PTEs it
> > > needed to update.
> > >
> > > (XEN) mm.c:3846:d0 Could not find L1 PTE for address fbb42000
> > >
> > > netback and blkback were making the hypercall from a kernel thread
> > > where task->active_mm != &init_mm and alloc_vm_area() was only
> > > updating the page tables for init_mm. The usual method of deferring
> > > the update to the page tables of other processes (i.e., after taking a
> > > fault) doesn't work as a fault cannot occur during the hypercall.
> > >
> > > This would work on some systems depending on what else was using
> > > vmalloc.
> > >
> > > Fix this by reverting ef691947d8a3d479e67652312783aedcf629320a
> > > (vmalloc: remove vmalloc_sync_all() from alloc_vm_area()) and add a
> > > comment to explain why it's needed.
> >
> > oookay, I queued this for 3.1 and tagged it for a 3.0.x backport. I
> > *think* that's the outcome of this discussion, for the short-term?
>
> <nods> Yup. Thanks!
Hey Andrew,
The long term outcome is the patchset that David worked on. I've sent
a GIT PULL to Linus to pick up the Xen related patches that switch over
the users of the right API:
(xen) stable/vmalloc-3.2 for Linux 3.2-rc0
(https://lkml.org/lkml/2011/10/29/82)
And then on top of that use this patch:
[Note, I am still waiting for Linus to pull that patchset above.. so not
sure on the outcome. perhaps a better way would be for you to pull
all patches in your tree?]
Also, not sure what you thought of this patch below?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Xen-devel] Re: [Revert] Re: [PATCH] mm: sync vmalloc address space page tables in alloc_vm_area()
2011-11-05 13:38 ` Konrad Rzeszutek Wilk
@ 2011-11-07 20:36 ` Konrad Rzeszutek Wilk
2011-11-08 23:01 ` Andrew Morton
0 siblings, 1 reply; 14+ messages in thread
From: Konrad Rzeszutek Wilk @ 2011-11-07 20:36 UTC (permalink / raw)
To: Andrew Morton
Cc: David Vrabel, Jeremy Fitzhardinge, xen-devel@lists.xensource.com,
namhyung@gmail.com, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, rientjes@google.com,
paulmck@linux.vnet.ibm.com
[-- Attachment #1: Type: text/plain, Size: 6489 bytes --]
> > >
> > > oookay, I queued this for 3.1 and tagged it for a 3.0.x backport. I
> > > *think* that's the outcome of this discussion, for the short-term?
> >
> > <nods> Yup. Thanks!
>
> Hey Andrew,
>
> The long term outcome is the patchset that David worked on. I've sent
> a GIT PULL to Linus to pick up the Xen related patches that switch over
> the users of the right API:
>
> (xen) stable/vmalloc-3.2 for Linux 3.2-rc0
>
> (https://lkml.org/lkml/2011/10/29/82)
And Linus picked it up.
.. snip..
>
> Also, not sure what you thought of this patch below?
Patch included as attachment for easier review..
>
> From b9acd3abc12972be0d938d7bc2466d899023e757 Mon Sep 17 00:00:00 2001
> From: David Vrabel <david.vrabel@citrix.com>
> Date: Thu, 29 Sep 2011 16:53:32 +0100
> Subject: [PATCH] xen: map foreign pages for shared rings by updating the PTEs
> directly
>
> When mapping a foreign page with xenbus_map_ring_valloc() with the
> GNTTABOP_map_grant_ref hypercall, set the GNTMAP_contains_pte flag and
> pass a pointer to the PTE (in init_mm).
>
> After the page is mapped, the usual fault mechanism can be used to
> update additional MMs. This allows the vmalloc_sync_all() to be
> removed from alloc_vm_area().
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Jeremy Fitzhardinge <jeremy@goop.org>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> arch/x86/xen/grant-table.c | 2 +-
> drivers/xen/xenbus/xenbus_client.c | 11 ++++++++---
> include/linux/vmalloc.h | 2 +-
> mm/vmalloc.c | 27 +++++++++++++--------------
> 4 files changed, 23 insertions(+), 19 deletions(-)
>
> diff --git a/arch/x86/xen/grant-table.c b/arch/x86/xen/grant-table.c
> index 6bbfd7a..5a40d24 100644
> --- a/arch/x86/xen/grant-table.c
> +++ b/arch/x86/xen/grant-table.c
> @@ -71,7 +71,7 @@ int arch_gnttab_map_shared(unsigned long *frames, unsigned long nr_gframes,
>
> if (shared == NULL) {
> struct vm_struct *area =
> - alloc_vm_area(PAGE_SIZE * max_nr_gframes);
> + alloc_vm_area(PAGE_SIZE * max_nr_gframes, NULL);
> BUG_ON(area == NULL);
> shared = area->addr;
> *__shared = shared;
> diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c
> index 229d3ad..52bc57f 100644
> --- a/drivers/xen/xenbus/xenbus_client.c
> +++ b/drivers/xen/xenbus/xenbus_client.c
> @@ -34,6 +34,7 @@
> #include <linux/types.h>
> #include <linux/vmalloc.h>
> #include <asm/xen/hypervisor.h>
> +#include <asm/xen/page.h>
> #include <xen/interface/xen.h>
> #include <xen/interface/event_channel.h>
> #include <xen/events.h>
> @@ -435,19 +436,20 @@ EXPORT_SYMBOL_GPL(xenbus_free_evtchn);
> int xenbus_map_ring_valloc(struct xenbus_device *dev, int gnt_ref, void **vaddr)
> {
> struct gnttab_map_grant_ref op = {
> - .flags = GNTMAP_host_map,
> + .flags = GNTMAP_host_map | GNTMAP_contains_pte,
> .ref = gnt_ref,
> .dom = dev->otherend_id,
> };
> struct vm_struct *area;
> + pte_t *pte;
>
> *vaddr = NULL;
>
> - area = alloc_vm_area(PAGE_SIZE);
> + area = alloc_vm_area(PAGE_SIZE, &pte);
> if (!area)
> return -ENOMEM;
>
> - op.host_addr = (unsigned long)area->addr;
> + op.host_addr = arbitrary_virt_to_machine(pte).maddr;
>
> if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
> BUG();
> @@ -526,6 +528,7 @@ int xenbus_unmap_ring_vfree(struct xenbus_device *dev, void *vaddr)
> struct gnttab_unmap_grant_ref op = {
> .host_addr = (unsigned long)vaddr,
> };
> + unsigned int level;
>
> /* It'd be nice if linux/vmalloc.h provided a find_vm_area(void *addr)
> * method so that we don't have to muck with vmalloc internals here.
> @@ -547,6 +550,8 @@ int xenbus_unmap_ring_vfree(struct xenbus_device *dev, void *vaddr)
> }
>
> op.handle = (grant_handle_t)area->phys_addr;
> + op.host_addr = arbitrary_virt_to_machine(
> + lookup_address((unsigned long)vaddr, &level)).maddr;
>
> if (HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1))
> BUG();
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index 9332e52..1a77252 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -118,7 +118,7 @@ unmap_kernel_range(unsigned long addr, unsigned long size)
> #endif
>
> /* Allocate/destroy a 'vmalloc' VM area. */
> -extern struct vm_struct *alloc_vm_area(size_t size);
> +extern struct vm_struct *alloc_vm_area(size_t size, pte_t **ptes);
> extern void free_vm_area(struct vm_struct *area);
>
> /* for /dev/kmem */
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 5016f19..b5deec6 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -2105,23 +2105,30 @@ void __attribute__((weak)) vmalloc_sync_all(void)
>
> static int f(pte_t *pte, pgtable_t table, unsigned long addr, void *data)
> {
> - /* apply_to_page_range() does all the hard work. */
> + pte_t ***p = data;
> +
> + if (p) {
> + *(*p) = pte;
> + (*p)++;
> + }
> return 0;
> }
>
> /**
> * alloc_vm_area - allocate a range of kernel address space
> * @size: size of the area
> + * @ptes: returns the PTEs for the address space
> *
> * Returns: NULL on failure, vm_struct on success
> *
> * This function reserves a range of kernel address space, and
> * allocates pagetables to map that range. No actual mappings
> - * are created. If the kernel address space is not shared
> - * between processes, it syncs the pagetable across all
> - * processes.
> + * are created.
> + *
> + * If @ptes is non-NULL, pointers to the PTEs (in init_mm)
> + * allocated for the VM area are returned.
> */
> -struct vm_struct *alloc_vm_area(size_t size)
> +struct vm_struct *alloc_vm_area(size_t size, pte_t **ptes)
> {
> struct vm_struct *area;
>
> @@ -2135,19 +2142,11 @@ struct vm_struct *alloc_vm_area(size_t size)
> * of kernel virtual address space and mapped into init_mm.
> */
> if (apply_to_page_range(&init_mm, (unsigned long)area->addr,
> - area->size, f, NULL)) {
> + size, f, ptes ? &ptes : NULL)) {
> free_vm_area(area);
> return NULL;
> }
>
> - /*
> - * If the allocated address space is passed to a hypercall
> - * before being used then we cannot rely on a page fault to
> - * trigger an update of the page tables. So sync all the page
> - * tables here.
> - */
> - vmalloc_sync_all();
> -
> return area;
> }
> EXPORT_SYMBOL_GPL(alloc_vm_area);
> --
> 1.7.7.1
>
[-- Attachment #2: 0001-xen-map-foreign-pages-for-shared-rings-by-updating-t.patch --]
[-- Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Xen-devel] Re: [Revert] Re: [PATCH] mm: sync vmalloc address space page tables in alloc_vm_area()
2011-11-07 20:36 ` Konrad Rzeszutek Wilk
@ 2011-11-08 23:01 ` Andrew Morton
2011-11-08 23:31 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 14+ messages in thread
From: Andrew Morton @ 2011-11-08 23:01 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: David Vrabel, Jeremy Fitzhardinge, xen-devel@lists.xensource.com,
namhyung@gmail.com, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, rientjes@google.com,
paulmck@linux.vnet.ibm.com
On Mon, 7 Nov 2011 15:36:13 -0500
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> > > >
> > > > oookay, I queued this for 3.1 and tagged it for a 3.0.x backport. I
> > > > *think* that's the outcome of this discussion, for the short-term?
> > >
> > > <nods> Yup. Thanks!
> >
> > Hey Andrew,
> >
> > The long term outcome is the patchset that David worked on. I've sent
> > a GIT PULL to Linus to pick up the Xen related patches that switch over
> > the users of the right API:
> >
> > (xen) stable/vmalloc-3.2 for Linux 3.2-rc0
> >
> > (https://lkml.org/lkml/2011/10/29/82)
>
> And Linus picked it up.
I've no idea what's going on here.
> .. snip..
> >
> > Also, not sure what you thought of this patch below?
>
> Patch included as attachment for easier review..
The patch "xen: map foreign pages for shared rings by updating the PTEs
directly" (whcih looks harnless enough) is not present in 3.2-rc1 or linux-next.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Xen-devel] Re: [Revert] Re: [PATCH] mm: sync vmalloc address space page tables in alloc_vm_area()
2011-11-08 23:01 ` Andrew Morton
@ 2011-11-08 23:31 ` Konrad Rzeszutek Wilk
2011-11-08 23:36 ` Andrew Morton
0 siblings, 1 reply; 14+ messages in thread
From: Konrad Rzeszutek Wilk @ 2011-11-08 23:31 UTC (permalink / raw)
To: Andrew Morton
Cc: Jeremy Fitzhardinge, xen-devel@lists.xensource.com,
namhyung@gmail.com, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, David Vrabel, rientjes@google.com,
paulmck@linux.vnet.ibm.com
> > And Linus picked it up.
>
> I've no idea what's going on here.
Heh. Sorry for being so confusing. Merge windows are a bit stressful and
I sometimes end up writing run-on sentences.
>
> > .. snip..
> > >
> > > Also, not sure what you thought of this patch below?
> >
> > Patch included as attachment for easier review..
>
> The patch "xen: map foreign pages for shared rings by updating the PTEs
> directly" (whcih looks harnless enough) is not present in 3.2-rc1 or linux-next.
<nods>. That is b/c it does not have your Ack. The patch applies cleanly to
3.2-rc1 (as all the other patches that it depends on are now in 3.2-rc1).
I am humbly asking for you to:
a) review the patch (which you did) and get an idea whether you are OK (sounds like you are)
b) pick it up in your -mm tree.
or alternately:
b) give an Ack on the patch so I can put it in my linux-next and push it
for 3.2-rc1.
Thank you for you consideration!
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Xen-devel] Re: [Revert] Re: [PATCH] mm: sync vmalloc address space page tables in alloc_vm_area()
2011-11-08 23:31 ` Konrad Rzeszutek Wilk
@ 2011-11-08 23:36 ` Andrew Morton
0 siblings, 0 replies; 14+ messages in thread
From: Andrew Morton @ 2011-11-08 23:36 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Jeremy Fitzhardinge, xen-devel@lists.xensource.com,
namhyung@gmail.com, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, David Vrabel, rientjes@google.com,
paulmck@linux.vnet.ibm.com
On Tue, 8 Nov 2011 18:31:32 -0500
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> > > And Linus picked it up.
> >
> > I've no idea what's going on here.
>
> Heh. Sorry for being so confusing. Merge windows are a bit stressful and
> I sometimes end up writing run-on sentences.
> >
> > > .. snip..
> > > >
> > > > Also, not sure what you thought of this patch below?
> > >
> > > Patch included as attachment for easier review..
> >
> > The patch "xen: map foreign pages for shared rings by updating the PTEs
> > directly" (whcih looks harnless enough) is not present in 3.2-rc1 or linux-next.
>
> <nods>. That is b/c it does not have your Ack. The patch applies cleanly to
> 3.2-rc1 (as all the other patches that it depends on are now in 3.2-rc1).
>
> I am humbly asking for you to:
> a) review the patch (which you did) and get an idea whether you are OK (sounds like you are)
Yup.
> b) pick it up in your -mm tree.
No added benefit there.
> or alternately:
> b) give an Ack on the patch so I can put it in my linux-next and push it
> for 3.2-rc1.
That's OK by me.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2011-11-08 23:36 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1314877863-21977-1-git-send-email-david.vrabel@citrix.com>
2011-09-01 16:11 ` [Revert] Re: [PATCH] mm: sync vmalloc address space page tables in alloc_vm_area() Konrad Rzeszutek Wilk
2011-09-01 20:37 ` Jeremy Fitzhardinge
2011-09-01 21:17 ` Andrew Morton
2011-09-02 11:39 ` David Vrabel
2011-09-02 22:32 ` Andrew Morton
2011-09-02 23:04 ` Jeremy Fitzhardinge
2011-09-06 16:35 ` [Xen-devel] " Konrad Rzeszutek Wilk
2011-11-05 13:38 ` Konrad Rzeszutek Wilk
2011-11-07 20:36 ` Konrad Rzeszutek Wilk
2011-11-08 23:01 ` Andrew Morton
2011-11-08 23:31 ` Konrad Rzeszutek Wilk
2011-11-08 23:36 ` Andrew Morton
2011-09-02 7:22 ` Ian Campbell
2011-09-02 7:31 ` Keir Fraser
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).