Re: [Xen-devel] [PATCHv2 2/2] xen/privcmd: add ioctls for locking/unlocking hypercall buffers

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.08.16 at 17:16, <david.vrabel@xxxxxxxxxx> wrote:
> Using mlock() for hypercall buffers is not sufficient since mlocked
> pages are still subject to compaction and page migration.  Page
> migration can be prevented by taking additional references to the
> pages.
> 
> Introduce two new ioctls: IOCTL_PRIVCMD_HCALL_BUF_LOCK and
> IOCTL_PRIVCMD_HCALL_BUF_UNLOCK which get and put the necessary page
> references.  The buffers do not need to be page aligned and they may
> overlap with other buffers.  However, it is not possible to partially
> unlock a buffer (i.e., the LOCK/UNLOCK must always be paired).  Any
> locked buffers are automatically unlocked when the file descriptor is
> closed.
> 
> An alternative approach would be to extend the driver with an ioctl to
> populate a VMA with "special", non-migratable pages.  But the
> LOCK/UNLOCK ioctls are more flexible as they allow any page to be used
> for hypercalls (e.g., stack, mmap'd files, etc.).  This could be used
> to minimize bouncing for performance critical hypercalls.
> 
> Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

with two remarks: Do you not care about compat mode callers? In
case you indeed mean to not support them, does the default
handling ensure they will see an error instead of you mis-interpreting
(and overrunning) the presented structure?

> +static struct privcmd_hbuf *privcmd_hbuf_alloc(struct privcmd_hcall_buf *buf)
> +{
> +     struct privcmd_hbuf *hbuf;
> +     unsigned long start, end, nr_pages;
> +     int ret;
> +
> +     start = (unsigned long)buf->start & PAGE_MASK;
> +     end = ALIGN((unsigned long)buf->start + buf->len, PAGE_SIZE);
> +     nr_pages = (end - start) / PAGE_SIZE;

So entry re-use is based on the actual length the caller passed,
while page tracking of course is page granular. Wouldn't it make
sense to re-use entries based on the pages they encompass,
which in particular would mean that two distinct buffers on the
same page would get just a single entry?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel