svn commit: r297396 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs

Andriy Gapon avg at FreeBSD.org
Tue Mar 29 20:05:56 UTC 2016


On 29/03/2016 22:18, Alexander Motin wrote:
> Author: mav
> Date: Tue Mar 29 19:18:34 2016
> New Revision: 297396
> URL: https://svnweb.freebsd.org/changeset/base/297396
> 
> Log:
>   Modify "4958 zdb trips assert on pools with ashift >= 0xe".
>   
>   Unlike Illumos FreeBSD has concept of logical ashift, that specifies
>   really minimal vdev block size that can be accessed.  This knowledge
>   allows properly pad physical I/O and correctly assert its alignment.
>   
>   This change fixes L2ARC write errors when device has logical sector
>   size above 512 bytes.
>   
>   MFC after:	1 month
> 
> Modified:
>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zio.c
> 
> Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zio.c
> ==============================================================================
> --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zio.c	Tue Mar 29 19:11:04 2016	(r297395)
> +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zio.c	Tue Mar 29 19:18:34 2016	(r297396)
> @@ -2775,10 +2775,19 @@ zio_vdev_io_start(zio_t *zio)
>  			(void) atomic_cas_64(&spa->spa_last_io, old, new);
>  	}
>  
> +#ifdef illumos
>  	align = 1ULL << vd->vdev_top->vdev_ashift;
>  
>  	if (!(zio->io_flags & ZIO_FLAG_PHYSICAL) &&
>  	    P2PHASE(zio->io_size, align) != 0) {
> +#else
> +	if (zio->io_flags & ZIO_FLAG_PHYSICAL)
> +		align = 1ULL << vd->vdev_top->vdev_logical_ashift;

Alexander,

I believe that this is wrong.
We must not modify any parameters of a physical zio including its size.
Otherwise we are going to overwrite something that an originator of the I/O did
not intend to overwrite.  Here we should have a check (or just the assertion)
that the zio conforms to disk characteristics.  It's the code that creates
physical zio-s that should be made aware of the alignment requirements.

Please see my work in this direction: https://reviews.freebsd.org/D2789
(BTW, I added you as a reviewer there).

> +	else
> +		align = 1ULL << vd->vdev_top->vdev_ashift;
> +
> +	if (P2PHASE(zio->io_size, align) != 0) {
> +#endif
>  		/* Transform logical writes to be a full physical block size. */
>  		uint64_t asize = P2ROUNDUP(zio->io_size, align);
>  		char *abuf = NULL;
> @@ -2794,6 +2803,7 @@ zio_vdev_io_start(zio_t *zio)
>  		    zio_subblock);
>  	}
>  
> +#ifdef illumos
>  	/*
>  	 * If this is not a physical io, make sure that it is properly aligned
>  	 * before proceeding.
> @@ -2809,6 +2819,10 @@ zio_vdev_io_start(zio_t *zio)
>  		ASSERT0(P2PHASE(zio->io_offset, SPA_MINBLOCKSIZE));
>  		ASSERT0(P2PHASE(zio->io_size, SPA_MINBLOCKSIZE));
>  	}
> +#else
> +	ASSERT0(P2PHASE(zio->io_offset, align));
> +	ASSERT0(P2PHASE(zio->io_size, align));
> +#endif
>  
>  	VERIFY(zio->io_type == ZIO_TYPE_READ || spa_writeable(spa));
>  
> 


-- 
Andriy Gapon


More information about the svn-src-all mailing list