[PATCH RESEND] xen/gntdev: Grant maps should not be subject to NUMA balancing

[PATCH RESEND] xen/gntdev: Grant maps should not be subject to NUMA balancing

[Boris Ostrovsky]

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 <boris.ostrovsky@oracle.com>
Cc: stable@vger.kernel.org
---

- Added stable@vger.kernel.org

 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

Re: [Xen-devel] [PATCH RESEND] xen/gntdev: Grant maps should not be subject to NUMA balancing

[David Vrabel]

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

Re: [Xen-devel] [PATCH RESEND] xen/gntdev: Grant maps should not be subject to NUMA balancing

[Olaf Hering]

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

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

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

Re: [Xen-devel] [PATCH RESEND] xen/gntdev: Grant maps should not be subject to NUMA balancing

[Boris Ostrovsky]

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

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



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

Re: [Xen-devel] [PATCH RESEND] xen/gntdev: Grant maps should not be subject to NUMA balancing

[Olaf Hering]

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

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

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

Re: [Xen-devel] [PATCH RESEND] xen/gntdev: Grant maps should not be subject to NUMA balancing

[Boris Ostrovsky]

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

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


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

Re: [Xen-devel] [PATCH RESEND] xen/gntdev: Grant maps should not be subject to NUMA balancing

[Olaf Hering]

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

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

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

Re: [Xen-devel] [PATCH RESEND] xen/gntdev: Grant maps should not be subject to NUMA balancing

[David Vrabel]

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

Re: [Xen-devel] [PATCH RESEND] xen/gntdev: Grant maps should not be subject to NUMA balancing

[Boris Ostrovsky]

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