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 <[email protected]>
Signed-off-by: Boris Ostrovsky <[email protected]>
Reviewed-by: David Vrabel <[email protected]>
---
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
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
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
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
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