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


Groups > linux.kernel > #1168588 > unrolled thread

Re: [PATCH 3/5] vhost: support upto 509 memory regions

Started by"Michael S. Tsirkin" <mst@redhat.com>
First post2015-06-19 10:00 +0200
Last post2015-06-19 15:40 +0200
Articles 5 — 2 participants

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: [PATCH 3/5] vhost: support upto 509 memory regions "Michael S. Tsirkin" <mst@redhat.com> - 2015-06-19 10:00 +0200
    Re: [PATCH 3/5] vhost: support upto 509 memory regions Paolo Bonzini <pbonzini@redhat.com> - 2015-06-19 10:00 +0200
      Re: [PATCH 3/5] vhost: support upto 509 memory regions "Michael S. Tsirkin" <mst@redhat.com> - 2015-06-19 10:10 +0200
        Re: [PATCH 3/5] vhost: support upto 509 memory regions Paolo Bonzini <pbonzini@redhat.com> - 2015-06-19 11:00 +0200
          Re: [PATCH 3/5] vhost: support upto 509 memory regions "Michael S. Tsirkin" <mst@redhat.com> - 2015-06-19 15:40 +0200

#1168588 — Re: [PATCH 3/5] vhost: support upto 509 memory regions

From"Michael S. Tsirkin" <mst@redhat.com>
Date2015-06-19 10:00 +0200
SubjectRe: [PATCH 3/5] vhost: support upto 509 memory regions
Message-ID<pCZnr-17v-3@gated-at.bofh.it>
On Thu, Jun 18, 2015 at 06:02:46PM +0200, Paolo Bonzini wrote:
> 
> 
> On 18/06/2015 16:47, Michael S. Tsirkin wrote:
> >> However, with Igor's patches a memory_region_del_subregion will cause a
> >> mmap(MAP_NORESERVE), which _does_ have the effect of making the hva go away.
> >>
> >> I guess one way to do it would be to alias the same page in two places,
> >> one for use by vhost and one for use by everything else.  However, the
> >> kernel does not provide the means to do this kind of aliasing for
> >> anonymous mmaps.
> > 
> > Basically pages go away on munmap, so won't simple
> > 	lock
> > 	munmap
> > 	mmap(MAP_NORESERVE)
> > 	unlock
> > do the trick?
> 
> Not sure I follow.  Here we have this:
> 
>     VCPU 1                             VCPU 2                          I/O worker
>     ----------------------------------------------------------------------------------------
>     take big QEMU lock
>     p = address_space_map(hva, len)
>     pass I/O request to worker thread
>                                                                        read(fd, p, len)
>     release big QEMU lock
> 
>                                         memory_region_del_subregion
>                                           mmap(MAP_NORESERVE)
> 
>                                                                        read returns EFAULT

Why doesn't it EFAULT without mmap(MAP_NORESERVE)?
Doesn't memory_region_del_subregion free the memory?

>                                                                        wake up VCPU 1
>     take big QEMU lock
>     EFAULT?  What's that?
> 
> In another scenario you are less lucky: the memory accesses
> between address_space_map/unmap aren't done in the kernel and
> you get a plain old SIGSEGV.
> 
> This is not something that you can fix with a lock.  The very
> purpose of the map/unmap API is to do stuff asynchronously while
> the lock is released.
> 
> Thanks,
> 
> Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1168589

FromPaolo Bonzini <pbonzini@redhat.com>
Date2015-06-19 10:00 +0200
Message-ID<pCZnr-17v-5@gated-at.bofh.it>
In reply to#1168588

On 19/06/2015 09:56, Michael S. Tsirkin wrote:
> On Thu, Jun 18, 2015 at 06:02:46PM +0200, Paolo Bonzini wrote:
>>
>>
>> On 18/06/2015 16:47, Michael S. Tsirkin wrote:
>>>> However, with Igor's patches a memory_region_del_subregion will cause a
>>>> mmap(MAP_NORESERVE), which _does_ have the effect of making the hva go away.
>>>>
>>>> I guess one way to do it would be to alias the same page in two places,
>>>> one for use by vhost and one for use by everything else.  However, the
>>>> kernel does not provide the means to do this kind of aliasing for
>>>> anonymous mmaps.
>>>
>>> Basically pages go away on munmap, so won't simple
>>> 	lock
>>> 	munmap
>>> 	mmap(MAP_NORESERVE)
>>> 	unlock
>>> do the trick?
>>
>> Not sure I follow.  Here we have this:
>>
>>     VCPU 1                             VCPU 2                          I/O worker
>>     ----------------------------------------------------------------------------------------
>>     take big QEMU lock
>>     p = address_space_map(hva, len)
>>     pass I/O request to worker thread
>>                                                                        read(fd, p, len)
>>     release big QEMU lock
>>
>>                                         memory_region_del_subregion
>>                                           mmap(MAP_NORESERVE)
>>
>>                                                                        read returns EFAULT
> 
> Why doesn't it EFAULT without mmap(MAP_NORESERVE)?
> Doesn't memory_region_del_subregion free the memory?

No, only destruction of the memory region frees it.  address_space_map
takes a reference to the memory region and address_space_unmap releases it.

Paolo

>>                                                                        wake up VCPU 1
>>     take big QEMU lock
>>     EFAULT?  What's that?
>>
>> In another scenario you are less lucky: the memory accesses
>> between address_space_map/unmap aren't done in the kernel and
>> you get a plain old SIGSEGV.
>>
>> This is not something that you can fix with a lock.  The very
>> purpose of the map/unmap API is to do stuff asynchronously while
>> the lock is released.
>>
>> Thanks,
>>
>> Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1168598

From"Michael S. Tsirkin" <mst@redhat.com>
Date2015-06-19 10:10 +0200
Message-ID<pCZx8-1y4-19@gated-at.bofh.it>
In reply to#1168589
On Fri, Jun 19, 2015 at 09:57:22AM +0200, Paolo Bonzini wrote:
> 
> 
> On 19/06/2015 09:56, Michael S. Tsirkin wrote:
> > On Thu, Jun 18, 2015 at 06:02:46PM +0200, Paolo Bonzini wrote:
> >>
> >>
> >> On 18/06/2015 16:47, Michael S. Tsirkin wrote:
> >>>> However, with Igor's patches a memory_region_del_subregion will cause a
> >>>> mmap(MAP_NORESERVE), which _does_ have the effect of making the hva go away.
> >>>>
> >>>> I guess one way to do it would be to alias the same page in two places,
> >>>> one for use by vhost and one for use by everything else.  However, the
> >>>> kernel does not provide the means to do this kind of aliasing for
> >>>> anonymous mmaps.
> >>>
> >>> Basically pages go away on munmap, so won't simple
> >>> 	lock
> >>> 	munmap
> >>> 	mmap(MAP_NORESERVE)
> >>> 	unlock
> >>> do the trick?
> >>
> >> Not sure I follow.  Here we have this:
> >>
> >>     VCPU 1                             VCPU 2                          I/O worker
> >>     ----------------------------------------------------------------------------------------
> >>     take big QEMU lock
> >>     p = address_space_map(hva, len)
> >>     pass I/O request to worker thread
> >>                                                                        read(fd, p, len)
> >>     release big QEMU lock
> >>
> >>                                         memory_region_del_subregion
> >>                                           mmap(MAP_NORESERVE)
> >>
> >>                                                                        read returns EFAULT
> > 
> > Why doesn't it EFAULT without mmap(MAP_NORESERVE)?
> > Doesn't memory_region_del_subregion free the memory?
> 
> No, only destruction of the memory region frees it.  address_space_map
> takes a reference to the memory region and address_space_unmap releases it.
> 
> Paolo

Confused. So can we call mmap(MAP_NORESERVE) in address_space_unmap
after we detect refcount is 0?

> >>                                                                        wake up VCPU 1
> >>     take big QEMU lock
> >>     EFAULT?  What's that?
> >>
> >> In another scenario you are less lucky: the memory accesses
> >> between address_space_map/unmap aren't done in the kernel and
> >> you get a plain old SIGSEGV.
> >>
> >> This is not something that you can fix with a lock.  The very
> >> purpose of the map/unmap API is to do stuff asynchronously while
> >> the lock is released.
> >>
> >> Thanks,
> >>
> >> Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1168637

FromPaolo Bonzini <pbonzini@redhat.com>
Date2015-06-19 11:00 +0200
Message-ID<pD0jw-2sP-3@gated-at.bofh.it>
In reply to#1168598

On 19/06/2015 10:05, Michael S. Tsirkin wrote:
> > No, only destruction of the memory region frees it.  address_space_map
> > takes a reference to the memory region and address_space_unmap releases it.
> > 
> > Paolo
> 
> Confused. So can we call mmap(MAP_NORESERVE) in address_space_unmap
> after we detect refcount is 0?

No, because in the meanwhile another DIMM could have been hotplugged
at the same place where the old one was.  This is legal:

    user                      guest                 QEMU
    ----------------------------------------------------------------------------------------
                              start I/O
                                  '---------------> address_space_map
    device_del
        '-------------------> receives SCI
                              executes _EJ0
                                  '---------------> memory_region_del_subregion
                                                    object_unparent
    device_add
        '-----------------------------------------> device_set_realized
                                                      hotplug_handler_plug
                                                        pc_machine_device_plug_cb
                                                          pc_dimm_plug
                                                            memory_region_add_subregion

                                                    I/O finishes
                                                      address_space_unmap

Surprise removal similarly could be done in QEMU, but it will hold to
some resources for as long as the device backends need them.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1168782

From"Michael S. Tsirkin" <mst@redhat.com>
Date2015-06-19 15:40 +0200
Message-ID<pD4Gu-n9-7@gated-at.bofh.it>
In reply to#1168637
On Fri, Jun 19, 2015 at 12:44:26PM +0200, Paolo Bonzini wrote:
> 
> 
> On 19/06/2015 12:14, Michael S. Tsirkin wrote:
> > On Fri, Jun 19, 2015 at 10:52:47AM +0200, Paolo Bonzini wrote:
> >>
> >>
> >> On 19/06/2015 10:05, Michael S. Tsirkin wrote:
> >>>> No, only destruction of the memory region frees it.  address_space_map
> >>>> takes a reference to the memory region and address_space_unmap releases it.
> >>>>
> >>>> Paolo
> >>>
> >>> Confused. So can we call mmap(MAP_NORESERVE) in address_space_unmap
> >>> after we detect refcount is 0?
> >>
> >> No, because in the meanwhile another DIMM could have been hotplugged
> >> at the same place where the old one was.  This is legal:
> >>
> >>     user                      guest                 QEMU
> >>     ----------------------------------------------------------------------------------------
> >>                               start I/O
> >>                                   '---------------> address_space_map
> >>     device_del
> >>         '-------------------> receives SCI
> >>                               executes _EJ0
> >>                                   '---------------> memory_region_del_subregion
> >>                                                     object_unparent
> > 
> > So guest started DMA into memory, then ejected this memory while DMA
> > is in progress?
> 
> Yes.  There is nothing that forbids doing that.
> 
> Paolo

Can we simply defer the next device_add using a hva until all IO completes?

> >>     device_add
> >>         '-----------------------------------------> device_set_realized
> >>                                                       hotplug_handler_plug
> >>                                                         pc_machine_device_plug_cb
> >>                                                           pc_dimm_plug
> >>                                                             memory_region_add_subregion
> >>
> >>                                                     I/O finishes
> >>                                                       address_space_unmap
> >>
> >> Surprise removal similarly could be done in QEMU, but it will hold to
> >> some resources for as long as the device backends need them.
> >>
> >> Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web