[Qemu-devel] [PATCH v7 0/2] qemu-qdisk: Implementation of grant copy operation.

* [Qemu-devel] [PATCH v7 0/2] qemu-qdisk: Implementation of grant copy operation.
@ 2016-09-14 19:10 ` Paulina Szubarczyk
  0 siblings, 0 replies; 16+ messages in thread
From: Paulina Szubarczyk @ 2016-09-14 19:10 UTC (permalink / raw)
  To: xen-devel
  Cc: roger.pau, wei.liu2, ian.jackson, david.vrabel, sstabellini,
	anthony.perard, qemu-devel, Paulina Szubarczyk

Hi,

It is a proposition for implementation of grant copy operation in qemu-qdisk and interface in libxc/libs. 

Changes since v6:
qemu-qdisk:
-removed blank lines
-renamed functions free_buffers -> ioreq_free_copy_buffers,
 ioreq_copy -> ioreq_grant_copy
-merged the if(ioreq_copy) with the conditions above

Changes since v5:
qemu-qdisk:
-added checking of every interface in the configure file. Based on
 the Roger's comment that xengnttab_map_grant_ref was added prior
 xengnttab_grant_copy, thus do not need to be check again here
 I dropped this check.

Changes since v4:
Interface:
- changed the subject line
- changed the comment in libs/gnttab/include/xengnttab.h according
  to the David's suggestion.
- removed unnecessary braces.

qemu-qdisk:
- in the configure file check only if xengnttab_grant_copy is
  implemented to verify 480 version of Xen.
- remove r variable and initialization of count to 0 in
  ioreq_copy.

- surround free_buffers, ioreq_init_copy_buffers and ioreq_copy
  by "#if CONFIG_XEN_CTRL_INTERFACE_VERSION >= 480" abort in else
  path because the function should not be called in that case.
- replace the definition of struct xengnttab_grant_copy_segment
  and a typedef to it with
  'typedef void* xengnttab_grant_copy_segment_t'.
- moved the new code in the xen_common.h to the end of the file.

Changes since v3:
Interface:
- revert to cast from xengnttab_grant_copy_segment_t
  to ioctl_gntdev_grant_copy.
- added compile-time check to compare the libs
  xengnttab_grant_copy_segment_t with the ioctl structure.
  The patch relies on Wei patch introducing XENGNTTAB_BUILD_BUG_ON in libs/gnttab.

qemu-qdisk:
- qemu_memalign/qemu_free is used instead function allocating
  memory from xc.
- removed the get_buffer function instead there is a direct call
  to qemu_memalign.
- moved ioreq_copy for write operation to ioreq_runio_qemu_aio.
- added struct xengnttab_grant_copy_segment_t and stub in
  xen_common.h for version of Xen earlier then 480.
- added checking for version 480 to configure. The test repeats
  all the operation that are required for version < 480 and
  checks if xengnttab_grant_copy() is implemented.

Changes since v2:
Interface:
- dropped the changes in libxc/include/xenctrl_compat
- changed the MINOR version in Makefile
- replaced 'return -1' -> 'abort()'in libs/gnttab/gnttab_unimp.c
- moved the struct 'xengnttab_copy_grant_segment' to 
  libs/gnttab/include/xengnttab.h
- added explicit assingment to ioctl_gntdev_grant_copy_segment 
  to the linux part

qemu-qdisk:
- to use the xengnttab_* function directly added -lxengnttab to configure
  and include <xengnttab.h> in include/hw/xen/xen_common.h
- in ioreq_copy removed an out path, changed a log level, made explicit 
  assignement to 'xengnttab_copy_grant_segment'
* I did not change the way of testing if grant_copy operation is implemented.
  As far as I understand if the code from gnttab_unimp.c is used then the gnttab 
  device is unavailable and the handler to gntdev would be invalid. But 
  if the handler is valid then the ioctl should return operation unimplemented 
  if the gntdev does not implement the operation.

Changes since v1:
Interface:
- changed the interface to call grant copy operation to match ioctl
	int xengnttab_grant_copy(xengnttab_handle *xgt,
                         	 uint32_t count,
                         	 xengnttab_grant_copy_segment_t* segs)

- added a struct 'xengnttab_copy_grant_segment' definition to tools/libs	
  /gnttab/private.h, tools/libxc/include/xenctrl_compat.h

- changed the function 'osdep_gnttab_grant_copy' which right now just
  call the ioctl

- added a new VER1.1 to tools/libs/gnttab/libxengnttab.map 

qemu-qdisk:
- removed the 'ioreq_write','ioreq_read_init','ioreq_read' functions 
- implemented 'ioreq_init_copy_buffers', 'ioreq_copy' 
- reverted the removal of grant map and introduced conditional invoking
  grant copy or grant map
- resigned from caching the local buffers on behalf of allocating the 
  required amount of pages at once. The cached structure would require 
  to have an lock guard and I suppose that the performance improvement 
  would degraded. 

For the functional test I attached the device with a qdisk backend to the guest, mounted, performed some reads and writes.

I run fio tests[1] with different iodepth and size of the block. The test can be 
accessed on my github[2] but mainly after the warm up I run for 60 seconds:
    fio --time_based \
		--clocksource=clock_gettime \
		--rw=randread \
		--random_distribution=pareto:0.9 \
		--size=10g \
	    --direct='1' \
	    --ioengine=libaio \
		--filename=$DEV \
		--iodepth=$IODEPTH \
		--bs=$BS \
		--name=$NAME \
		--runtime=$RUNTIME >> $FILENAME
The test were repeated at least three times. 

[1] https://docs.google.com/spreadsheets/d/1E6AMiB8ceJpExL6jWpH9u2yy6DZxzhmDUyFf-eUuJ0c/edit?usp=sharing

[2] https://github.com/paulina-szubarczyk/xen-benchmark
    - multitest_with_iodepth.sh

Thanks and regards, 
Paulina

^ permalink raw reply	[flat|nested] 16+ messages in thread