Doing so will cause the grant to be unmapped and then, during
fault handling, the fault to be mistakenly treated as NUMA hint
fault.
In addition, even if those maps could partcipate in NUMA
balancing, it wouldn't provide any benefit since we are unable
to determine physical page's node (even if/when VNUMA is
implemented).
Marking grant maps' VMAs as VM_IO will exclude them from being
part of NUMA balancing.
Signed-off-by: Boris Ostrovsky <[email protected]>
Cc: [email protected]
---
- Added [email protected]
drivers/xen/gntdev.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 67b9163..bf312df 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -802,7 +802,7 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma)
vma->vm_ops = &gntdev_vmops;
- vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
+ vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP | VM_IO;
if (use_ptemod)
vma->vm_flags |= VM_DONTCOPY;
--
1.7.1
On 10/11/15 20:10, Boris Ostrovsky wrote:
> Doing so will cause the grant to be unmapped and then, during
> fault handling, the fault to be mistakenly treated as NUMA hint
> fault.
>
> In addition, even if those maps could partcipate in NUMA
> balancing, it wouldn't provide any benefit since we are unable
> to determine physical page's node (even if/when VNUMA is
> implemented).
>
> Marking grant maps' VMAs as VM_IO will exclude them from being
> part of NUMA balancing.
Applied to for-linus-4.4, thanks.
David
On Tue, Nov 10, Boris Ostrovsky wrote:
> Doing so will cause the grant to be unmapped and then, during
> fault handling, the fault to be mistakenly treated as NUMA hint
> fault.
>
> In addition, even if those maps could partcipate in NUMA
> balancing, it wouldn't provide any benefit since we are unable
> to determine physical page's node (even if/when VNUMA is
> implemented).
>
> Marking grant maps' VMAs as VM_IO will exclude them from being
> part of NUMA balancing.
This breaks qdisk+aio because now such pages are rejected 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
domU.cfg:
builder=hvm
disk=['vdev=xvda, direct-io-safe, backendtype=qdisk, target=img.raw']
> @@ -802,7 +802,7 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma)
> - vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
> + vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP | VM_IO;
Olaf
On 11/10/2016 11:26 AM, Olaf Hering wrote:
> On Tue, Nov 10, Boris Ostrovsky wrote:
Perfect timing. This is from Nov. 10 2015.
>
>> Doing so will cause the grant to be unmapped and then, during
>> fault handling, the fault to be mistakenly treated as NUMA hint
>> fault.
>>
>> In addition, even if those maps could partcipate in NUMA
>> balancing, it wouldn't provide any benefit since we are unable
>> to determine physical page's node (even if/when VNUMA is
>> implemented).
>>
>> Marking grant maps' VMAs as VM_IO will exclude them from being
>> part of NUMA balancing.
> This breaks qdisk+aio because now such pages are rejected with -EFAULT:
Is this something new? Because this patch has been there for a year.
-boris
>
> 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
>
> domU.cfg:
> builder=hvm
> disk=['vdev=xvda, direct-io-safe, backendtype=qdisk, target=img.raw']
>
>> @@ -802,7 +802,7 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma)
>> - vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
>> + vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP | VM_IO;
>
> Olaf
On Thu, Nov 10, Boris Ostrovsky wrote:
> Is this something new? Because this patch has been there for a year.
It was just tested now, cycling through all the combinations for a
disk=[]. Removing "direct-is-save" will use different code paths and the
error is not seen.
Olaf
On 11/10/2016 11:42 AM, Olaf Hering wrote:
> On Thu, Nov 10, Boris Ostrovsky wrote:
>
>> Is this something new? Because this patch has been there for a year.
> It was just tested now, cycling through all the combinations for a
> disk=[]. Removing "direct-is-save" will use different code paths and the
> error is not seen.
Are you sure it's this patch that causes the failure?
I commented out '| VM_IO' and still unable to boot with this option.
-boris
On Thu, Nov 10, Boris Ostrovsky wrote:
> Are you sure it's this patch that causes the failure?
>
> I commented out '| VM_IO' and still unable to boot with this option.
Yes, this works for me, sles12sp2 dom0+domU, which is linux-4.4 based:
+++ b/drivers/xen/gntdev.c
@@ -804,7 +804,7 @@ 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 /*| VM_IO*/;
if (use_ptemod)
vma->vm_flags |= VM_DONTCOPY;
with this domU.cfg:
name="x"
memory=1024
serial="pty"
builder="hvm"
disk=[ 'vdev=xvda, direct-io-safe, backendtype=qdisk, target=x.raw', ]
vif=[ 'bridge=br0' ]
keymap="de"
cmdline="linemode=1 console=ttyS0,115200 ignore_loglevel install=http://host/sles_dvd1/ start_shell"
kernel= "/sles_dvd1/boot/x86_64/vmlinuz-xen"
ramdisk="/sles_dvd1/boot/x86_64/initrd-xen"
Without VM_IO 'fdisk -l /dev/xvda' works, with VM_IO 'fdisk -l
/dev/xvda' gives IO errors.
Olaf
On 10/11/16 17:47, Olaf Hering wrote:
> On Thu, Nov 10, Boris Ostrovsky wrote:
>
>> Are you sure it's this patch that causes the failure?
>>
>> I commented out '| VM_IO' and still unable to boot with this option.
>
> Yes, this works for me, sles12sp2 dom0+domU, which is linux-4.4 based:
>
> +++ b/drivers/xen/gntdev.c
> @@ -804,7 +804,7 @@ 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 /*| VM_IO*/;
>
> if (use_ptemod)
> vma->vm_flags |= VM_DONTCOPY;
I think we need a custom policy for this VMA with MPOL_F_MOF cleared.
David
On 11/10/2016 12:49 PM, David Vrabel wrote:
> On 10/11/16 17:47, Olaf Hering wrote:
>> On Thu, Nov 10, Boris Ostrovsky wrote:
>>
>>> Are you sure it's this patch that causes the failure?
>>>
>>> I commented out '| VM_IO' and still unable to boot with this option.
>> Yes, this works for me, sles12sp2 dom0+domU, which is linux-4.4 based:
I've never tested on 4.4, this was added for 4.5 (with CC to stable#4.4)
and now I am testing on 4.7)
>>
>> +++ b/drivers/xen/gntdev.c
>> @@ -804,7 +804,7 @@ 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 /*| VM_IO*/;
>>
>> if (use_ptemod)
>> vma->vm_flags |= VM_DONTCOPY;
> I think we need a custom policy for this VMA with MPOL_F_MOF cleared.
I think you are right, I remember when I was looking at this adding a
policy was the other option. Let me look at this again.
-boris