Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1519111 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [Xen-devel] [PATCH RESEND] xen/gntdev: Grant maps should not be  subject to NUMA balancing Olaf Hering <olaf@aepfle.de> - 2016-11-10 17:30 +0100
    Re: [Xen-devel] [PATCH RESEND] xen/gntdev: Grant maps should not be  subject to NUMA balancing Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-11-10 17:40 +0100
      Re: [Xen-devel] [PATCH RESEND] xen/gntdev: Grant maps should not be  subject to NUMA balancing Olaf Hering <olaf@aepfle.de> - 2016-11-10 17:50 +0100
        Re: [Xen-devel] [PATCH RESEND] xen/gntdev: Grant maps should not be  subject to NUMA balancing Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-11-10 18:40 +0100
          Re: [Xen-devel] [PATCH RESEND] xen/gntdev: Grant maps should not be  subject to NUMA balancing Olaf Hering <olaf@aepfle.de> - 2016-11-10 18:50 +0100
            Re: [Xen-devel] [PATCH RESEND] xen/gntdev: Grant maps should not be  subject to NUMA balancing David Vrabel <david.vrabel@citrix.com> - 2016-11-10 18:50 +0100
              Re: [Xen-devel] [PATCH RESEND] xen/gntdev: Grant maps should not be  subject to NUMA balancing Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-11-10 19:10 +0100

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

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [next] | [standalone]

#1519126

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

#1519129

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

#1519175

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

#1519226

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

#1519237

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

[toc] | [prev] | [next] | [standalone]

#1519251

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web