linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] xen/gntdev: Use mempolicy instead of VM_IO flag to avoid NUMA balancing
@ 2016-11-16 17:46 Boris Ostrovsky
  2016-11-17 11:28 ` Olaf Hering
  0 siblings, 1 reply; 5+ messages in thread
From: Boris Ostrovsky @ 2016-11-16 17:46 UTC (permalink / raw)
  To: david.vrabel, jgross; +Cc: xen-devel, linux-kernel, olaf, Boris Ostrovsky

Commit 9c17d96500f7 ("xen/gntdev: Grant maps should not be subject to
NUMA balancing") set VM_IO flag to prevent grant maps from being
subjected to NUMA balancing.

It was discovered recently that this flag causes get_user_pages() to
always fail with -EFAULT.

check_vma_flags
__get_user_pages
__get_user_pages_locked
__get_user_pages_unlocked
get_user_pages_fast
iov_iter_get_pages
dio_refill_pages
do_direct_IO
do_blockdev_direct_IO
do_blockdev_direct_IO
ext4_direct_IO_read
generic_file_read_iter
aio_run_iocb

(which can happen if guest's vdisk has direct-io-safe option).

To avoid this, instead of setting VM_IO use mempolicy that prohibits page
migration (i.e. clear policy's MPOL_F_MOF|MPOL_F_MORON)

Reported-by: Olaf Hering <olaf@aepfle.de>
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
---
Changes in v2:
* Corrected the commit message

Unfortunately I haven't been able to trigger NUMA balancing
so while I tested this in general I am not sure I actually
exercised the code path.

 drivers/xen/gntdev.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index bb95212..743e6f0 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -35,6 +35,7 @@
 #include <linux/spinlock.h>
 #include <linux/slab.h>
 #include <linux/highmem.h>
+#include <linux/mempolicy.h>
 
 #include <xen/xen.h>
 #include <xen/grant_table.h>
@@ -1007,8 +1008,20 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma)
 
 	vma->vm_ops = &gntdev_vmops;
 
-	vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP | VM_IO;
+	vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
 
+#ifdef CONFIG_NUMA
+	/* Prevent NUMA balancing */
+	if (vma->vm_policy)
+		vma->vm_policy->flags &= ~(MPOL_F_MOF | MPOL_F_MORON);
+	else {
+		struct mempolicy *pol = get_task_policy(current);
+
+		vma->vm_policy = mpol_dup(pol);
+		if (vma->vm_policy)
+			vma->vm_policy->flags &= ~(MPOL_F_MOF | MPOL_F_MORON);
+	}
+#endif
 	if (use_ptemod)
 		vma->vm_flags |= VM_DONTCOPY;
 
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] xen/gntdev: Use mempolicy instead of VM_IO flag to avoid NUMA balancing
  2016-11-16 17:46 [PATCH v2] xen/gntdev: Use mempolicy instead of VM_IO flag to avoid NUMA balancing Boris Ostrovsky
@ 2016-11-17 11:28 ` Olaf Hering
  2016-11-17 16:01   ` Boris Ostrovsky
  0 siblings, 1 reply; 5+ messages in thread
From: Olaf Hering @ 2016-11-17 11:28 UTC (permalink / raw)
  To: Boris Ostrovsky; +Cc: david.vrabel, jgross, xen-devel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 747 bytes --]

On Wed, Nov 16, Boris Ostrovsky wrote:

> Unfortunately I haven't been able to trigger NUMA balancing
> so while I tested this in general I am not sure I actually
> exercised the code path.

Thanks for the patch!

Would be nice to actually test the code path which caused the initial
addition of VM_IO. I think I lack the hardware to excersise them.


In my 4.4 based sources I get the following unresolved symbols. If that
happens to work in mainline the failures should at least be considered
during backporting of this proposed patch to the stable trees.

ERROR: "__mpol_dup" [drivers/xen/xen-gntdev.ko] undefined!
ERROR: "get_task_policy" [drivers/xen/xen-gntdev.ko] undefined!

Appearently these symbols lack just an EXPORT_SYMBOL_GPL.

Olaf

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 163 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] xen/gntdev: Use mempolicy instead of VM_IO flag to avoid NUMA balancing
  2016-11-17 11:28 ` Olaf Hering
@ 2016-11-17 16:01   ` Boris Ostrovsky
  2016-11-17 16:01     ` Olaf Hering
  0 siblings, 1 reply; 5+ messages in thread
From: Boris Ostrovsky @ 2016-11-17 16:01 UTC (permalink / raw)
  To: Olaf Hering; +Cc: david.vrabel, jgross, xen-devel, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 1053 bytes --]

On 11/17/2016 06:28 AM, Olaf Hering wrote:
> On Wed, Nov 16, Boris Ostrovsky wrote:
>
>> Unfortunately I haven't been able to trigger NUMA balancing
>> so while I tested this in general I am not sure I actually
>> exercised the code path.
> Thanks for the patch!
>
> Would be nice to actually test the code path which caused the initial
> addition of VM_IO. I think I lack the hardware to excersise them.

To trip the original bug is even more difficult than to trigger NUMA
balancing. It was very sensitive to memory allocation.

>
>
> In my 4.4 based sources I get the following unresolved symbols. If that
> happens to work in mainline the failures should at least be considered
> during backporting of this proposed patch to the stable trees.
>
> ERROR: "__mpol_dup" [drivers/xen/xen-gntdev.ko] undefined!
> ERROR: "get_task_policy" [drivers/xen/xen-gntdev.ko] undefined!
>
> Appearently these symbols lack just an EXPORT_SYMBOL_GPL.


I just built 4.4.11 with this patch applied and haven't had any problems.

-boris


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] xen/gntdev: Use mempolicy instead of VM_IO flag to avoid NUMA balancing
  2016-11-17 16:01   ` Boris Ostrovsky
@ 2016-11-17 16:01     ` Olaf Hering
  2016-11-17 16:09       ` Boris Ostrovsky
  0 siblings, 1 reply; 5+ messages in thread
From: Olaf Hering @ 2016-11-17 16:01 UTC (permalink / raw)
  To: Boris Ostrovsky; +Cc: david.vrabel, jgross, xen-devel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 465 bytes --]

On Thu, Nov 17, Boris Ostrovsky wrote:

> On 11/17/2016 06:28 AM, Olaf Hering wrote:
> > ERROR: "__mpol_dup" [drivers/xen/xen-gntdev.ko] undefined!
> > ERROR: "get_task_policy" [drivers/xen/xen-gntdev.ko] undefined!
> I just built 4.4.11 with this patch applied and haven't had any problems.

Are these functions exported? How would the driver module call into the
core kernel? Maybe the added functionality should be provided by an
exported helper function.

Olaf

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 163 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] xen/gntdev: Use mempolicy instead of VM_IO flag to avoid NUMA balancing
  2016-11-17 16:01     ` Olaf Hering
@ 2016-11-17 16:09       ` Boris Ostrovsky
  0 siblings, 0 replies; 5+ messages in thread
From: Boris Ostrovsky @ 2016-11-17 16:09 UTC (permalink / raw)
  To: Olaf Hering; +Cc: david.vrabel, jgross, xen-devel, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 646 bytes --]

On 11/17/2016 11:01 AM, Olaf Hering wrote:
> On Thu, Nov 17, Boris Ostrovsky wrote:
>
>> On 11/17/2016 06:28 AM, Olaf Hering wrote:
>>> ERROR: "__mpol_dup" [drivers/xen/xen-gntdev.ko] undefined!
>>> ERROR: "get_task_policy" [drivers/xen/xen-gntdev.ko] undefined!
>> I just built 4.4.11 with this patch applied and haven't had any problems.
> Are these functions exported? How would the driver module call into the
> core kernel? Maybe the added functionality should be provided by an
> exported helper function.

You are right --- I didn't realize was building with grant device
built-in. So yes, this needs to be fixed.

-boris


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-11-17 17:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-16 17:46 [PATCH v2] xen/gntdev: Use mempolicy instead of VM_IO flag to avoid NUMA balancing Boris Ostrovsky
2016-11-17 11:28 ` Olaf Hering
2016-11-17 16:01   ` Boris Ostrovsky
2016-11-17 16:01     ` Olaf Hering
2016-11-17 16:09       ` Boris Ostrovsky

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).