On Thu, Feb 01, 2018 at 07:58:10PM +0200, Marcel Apfelbaum wrote:
> On 01/02/2018 19:36, Eduardo Habkost wrote:
> > On Thu, Feb 01, 2018 at 07:12:45PM +0200, Michael S. Tsirkin wrote:
> >> On Thu, Feb 01, 2018 at 03:01:36PM -0200, Eduardo Habkost wrote:
> >>> On Thu, Feb 01, 2018 at 06:59:07PM +0200, Michael S. Tsirkin wrote:
> >>>> On Thu, Feb 01, 2018 at 02:57:39PM -0200, Eduardo Habkost wrote:
> >>>>> On Thu, Feb 01, 2018 at 06:48:54PM +0200, Michael S. Tsirkin wrote:
> >>>>>> On Thu, Feb 01, 2018 at 02:31:32PM -0200, Eduardo Habkost wrote:
> >>>>>>> On Thu, Feb 01, 2018 at 04:24:30PM +0200, Michael S. Tsirkin wrote:
> >>>>>>> [...]
> >>>>>>>> The full fix would be to allow QEMU to map a list of
> >>>>>>>> pages to a guest supplied IOVA.
> >>>>>>>
> >>>>>>> Thanks, that's what I expected.
> >>>>>>>
> >>>>>>> While this is not possible, the only requests I have for this
> >>>>>>> patch is that we clearly document:
> >>>>>>> * What's the only purpose of share=on on a host-memory-backend
> >>>>>>> object (due to pvrdma limitations).
> >>>>>>> * The potential undesirable side-effects of setting share=on.
> >>>>>>> * On the commit message and other comments, clearly distinguish
> >>>>>>> HVAs in the QEMU address-space from IOVAs, to avoid confusion.
> >>>>>>
> >>>>>> Looking forward, when we do support it, how will management find out
> >>>>>> it no longer needs to pass the share parameter?
> >>>>>>
> >>>>>> Further, if the side effects of the share parameter go away,
> >>>>>> how will it know these no longer hold?
> >>>>>
> >>>>> A query-host-capabilities or similar QMP command seems necessary
> >>>>> for that.
> >>>>
> >>>> Is anyone working on that?
> >>>
> >>> Not yet.
> >>>
> >>> --
> >>> Eduardo
> >>
> >> Do these patches need to wait until we do have that command?
> >
> > I don't think so. The command will be needed only when
> > support for pvrdma without share=on gets implemented.
> >
> > Right now, all we need is clear documentation.
> >
> >>
> >> I'm thinking it's better to have "share=on required with rdma"
> >> and "hugetlbfs not supported with rdma"
> >> than the reverse, this way new hosts do not need to carry
> >> thus stuff around forever.
> >
> > What do you mean by "the reverse"?
> >
> > IIUC, the requirements/limitations are:
> >
> > * share=on required for pvrdma. Already documented and enforced
> > by pvrdma code in this series.
>
> Right.
>
> > * hugetlbfs not supported with rdma. Is this detected/reported by
> > QEMU? Is it documented?
>
> Yes, enforced by the pvrdma device initialization and documented in the
> corresponding pvrdma doc.
>
> > * side-effects of share=on. This is not detected nor documented,
> > and probably already applies to other memory backends.
> > * Nice to have: document when share=on is useful (answer:
> > because of pvrdma), when adding share=on support to
> > host-memory-backend.
> >
>
> The documentation is part of the pvrdma doc.
> What are the side-effects of share=on? I missed that.
> (share=on is new for the memory backed RAM, the file
> backed RAM already had the share parameter)
>
> One can just grep for "share=on" in the docs directory
> and can easily see the only current usage. But maybe will
> be more, maybe we don't want to limit it for now.
>
> I am planning to re-spin today/tomorrow before sending
> a pull-request, can you please point me on what documentation
> to add and what side-effects I should document?
>
The full list of side-effects is not clear to me. For some of
them, see Documentation/vm/numa_memory_policy.txt on the kernel
tree.
The documentation for memory backend options is at
qemu-options.hx. Maybe something like this, extending the
existing paragraph:
The @option{share} boolean option determines whether the memory
region is marked as private to QEMU, or shared (mapped using
the MAP_SHARED flag). The latter allows a co-operating
external process to access the QEMU memory region.
@option{share} is also required for pvrdma devices due to
limitations in the RDMA API provided by Linux.
Setting share=on might affect the ability to configure NUMA
bindings for the memory backend under some circumstances, see
Documentation/vm/numa_memory_policy.txt on the Linux kernel
source tree for additional details.
I hate to point users to low-level documentation on the kernel
tree, but it's better than nothing.
We also need to list "share" as a valid option at the
"@item -object memory-backend-ram,[...]" line.
--
Eduardo
