svn commit: r269814 - head/sys/dev/xen/blkfront

Alexander Motin mav at FreeBSD.org
Thu Aug 28 18:23:10 UTC 2014


Hi, Roger.

It looks to me like this commit does not work as it should. I got
problem when I just tried `newfs /dev/ada0 ; mount /dev/ada0 /mnt`.
Somehow newfs does not produce valid filesystem. Problem is reliably
repeatable and reverting this commit fixes it.

I found at least one possible cause there: If original data buffer is
unmapped, misaligned and not physically contiguous, then present x86
bus_dmamap_load_bio() implementation will process each physically
contiguous segment separately. Due to the misalignment first and last
physical segments may have size not multiple to 512 bytes. Since each
segment processed separately, they are not joined together, and
xbd_queue_cb() is getting segments not multiple to 512 bytes. Attempt to
convert them to exact number of sectors in the driver cause data corruption.

This is a bug of the busdma code, and until it is fixed I don't see
solution other then temporary reverting this commit.

On 11.08.2014 18:37, Roger Pau Monné   wrote:
> Author: royger
> Date: Mon Aug 11 15:37:02 2014
> New Revision: 269814
> URL: http://svnweb.freebsd.org/changeset/base/269814
> 
> Log:
>   blkfront: add support for unmapped IO
>   
>   Using unmapped IO is really beneficial when running inside of a VM,
>   since it avoids IPIs to other vCPUs in order to invalidate the
>   mappings.
>   
>   This patch adds unmapped IO support to blkfront. The following tests
>   results have been obtained when running on a Xen host without HAP:
>   
>   PVHVM
>        3165.84 real      6354.17 user      4483.32 sys
>   PVHVM with unmapped IO
>        2099.46 real      4624.52 user      2967.38 sys
>   
>   This is because when running using shadow page tables TLB flushes and
>   range invalidations are much more expensive, so using unmapped IO
>   provides a very important performance boost.
>   
>   Sponsored by:	Citrix Systems R&D
>   Tested by:	robak
>   MFC after:	1 week
>   PR:		191173
>   
>   dev/xen/blkfront/blkfront.c:
>    - Add and announce support for unmapped IO.
> 
> Modified:
>   head/sys/dev/xen/blkfront/blkfront.c
> 
> Modified: head/sys/dev/xen/blkfront/blkfront.c
> ==============================================================================
> --- head/sys/dev/xen/blkfront/blkfront.c	Mon Aug 11 15:06:07 2014	(r269813)
> +++ head/sys/dev/xen/blkfront/blkfront.c	Mon Aug 11 15:37:02 2014	(r269814)
> @@ -272,8 +272,12 @@ xbd_queue_request(struct xbd_softc *sc, 
>  {
>  	int error;
>  
> -	error = bus_dmamap_load(sc->xbd_io_dmat, cm->cm_map, cm->cm_data,
> -	    cm->cm_datalen, xbd_queue_cb, cm, 0);
> +	if (cm->cm_bp != NULL)
> +		error = bus_dmamap_load_bio(sc->xbd_io_dmat, cm->cm_map,
> +		    cm->cm_bp, xbd_queue_cb, cm, 0);
> +	else
> +		error = bus_dmamap_load(sc->xbd_io_dmat, cm->cm_map,
> +		    cm->cm_data, cm->cm_datalen, xbd_queue_cb, cm, 0);
>  	if (error == EINPROGRESS) {
>  		/*
>  		 * Maintain queuing order by freezing the queue.  The next
> @@ -333,8 +337,6 @@ xbd_bio_command(struct xbd_softc *sc)
>  	}
>  
>  	cm->cm_bp = bp;
> -	cm->cm_data = bp->bio_data;
> -	cm->cm_datalen = bp->bio_bcount;
>  	cm->cm_sector_number = (blkif_sector_t)bp->bio_pblkno;
>  
>  	switch (bp->bio_cmd) {
> @@ -993,7 +995,7 @@ xbd_instance_create(struct xbd_softc *sc
>  
>  	sc->xbd_disk->d_mediasize = sectors * sector_size;
>  	sc->xbd_disk->d_maxsize = sc->xbd_max_request_size;
> -	sc->xbd_disk->d_flags = 0;
> +	sc->xbd_disk->d_flags = DISKFLAG_UNMAPPED_BIO;
>  	if ((sc->xbd_flags & (XBDF_FLUSH|XBDF_BARRIER)) != 0) {
>  		sc->xbd_disk->d_flags |= DISKFLAG_CANFLUSHCACHE;
>  		device_printf(sc->xbd_dev,
> 


-- 
Alexander Motin


More information about the svn-src-all mailing list