Re: [PATCH V2] mkfs: handle 4k sector devices more cleanly

[Christoph Hellwig <hch@infradead.org>]

On Fri, Jan 08, 2010 at 10:46:41AM -0600, Eric Sandeen wrote:
> +extern void platform_findsizes (char *path, int fd, long long *sz, int *bsz);

Can you move the prototype from libxfs/init.h to include/libxfs.h
instead of adding it to the .c file?

> +	/*
> +	 * MD wants sector size set == block size to avoid switching.
> +	 * Otherwise, if not specfied via command, use device sectorsize
> +	 */
> +	if (ft.sectoralign || !ssflag) {
> +		if (ft.sectoralign)
> +			sectorsize = blocksize;
> +		else
> +			sectorsize = ft.sectorsize;

This still confuses the heck out of me. What do you think about the
incremental patch at the end of the mail?

>  	if (slflag || ssflag)
>  		xi.setblksize = sectorsize;
> -	else
> -		xi.setblksize = 1;

So for the defaul case we now never set the sector size in the libxfs
init.  This seems safe to me, but why did we do it before?  Could
a previous user have left it set to a wrong value?

Maye we should just do the xi.setblksize = sectorsize unconditionally?


Index: xfsprogs-dev/mkfs/xfs_mkfs.c
===================================================================
--- xfsprogs-dev.orig/mkfs/xfs_mkfs.c	2010-01-08 18:33:53.619277529 +0100
+++ xfsprogs-dev/mkfs/xfs_mkfs.c	2010-01-08 18:39:37.758005711 +0100
@@ -1561,21 +1561,32 @@ main(
 	memset(&ft, 0, sizeof(ft));
 	get_topology(&xi, &ft);
 
-	/*
-	 * MD wants sector size set == block size to avoid switching.
-	 * Otherwise, if not specfied via command, use device sectorsize
-	 */
+	if (ft.sectoralign) {
+		/*
+		 * Older Linux software RAID versions want the sector size
+		 * to match the block size to avoid switching I/O sizes.
+		 * For the legacy libdisk case we thus set the sector size to
+		 * match the block size.  For systems using libblkid we assume
+		 * that the kernel is recent enough to not require this and
+		 * ft.sectoralign will never be set.
+		 */
+		sectorsize = blocksize;
+	} else if (!ssflag) {
+		/*
+		 * Unless specified manually on the command line use the
+		 * advertised sector size of the device.
+		 */
+		sectorsize = ft.sectorsize;
+	}
+
 	if (ft.sectoralign || !ssflag) {
-		if (ft.sectoralign)
-			sectorsize = blocksize;
-		else
-			sectorsize = ft.sectorsize;
 		sectorlog = libxfs_highbit32(sectorsize);
 		if (loginternal) {
 			lsectorsize = sectorsize;
 			lsectorlog = sectorlog;
 		}
 	}
+
 	if (sectorsize < XFS_MIN_SECTORSIZE ||
 	    sectorsize > XFS_MAX_SECTORSIZE || sectorsize > blocksize) {
 		fprintf(stderr, _("illegal sector size %d\n"), sectorsize);

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs