libext2fs: try to get physical sector size first in ext2fs_get_device_sectsize()

PATCH New Eric Sandeen 2 participants 7 messages Feb 8, 2010 – May 18, 2010

lore

Share & tools

lore

Eric Sandeen

author

2010-02-08 22:31 UTC #0 lore raw

PATCH 1/1 libext2fs: try to get physical sector size first in ext2fs_get_device_sectsize()

Some devices, notably 4k sector drives, may have a 512 logical sector size, mapped onto a 4k physical sector size.

When mke2fs is ratcheting down the blocksize for small filesystems, or when a blocksize is specified on the commandline, we should not willingly go below the physical sector size of the device.

When a blocksize is specified, we -must- not go below the logical sector size of the device.

Add a new library function, ext2fs_get_device_phys_sectsize() to get the physical sector size if possible, and adjust the logic in mke2fs to enforce the above rules.

Signed-off-by Eric Sandeen <[email protected]>

---

lib/ext2fs/ext2fs.h +1 -0

index 213a819..9c06048 100644
@@ -1086,6 +1086,7 @@ extern errcode_t ext2fs_get_device_size2(const char *file, int blocksize,
 
 /* getsectsize.c */
 errcode_t ext2fs_get_device_sectsize(const char *file, int *sectsize);
+errcode_t ext2fs_get_device_phys_sectsize(const char *file, int *sectsize);
 
 /* i_block.c */
 errcode_t ext2fs_iblk_add_blocks(ext2_filsys fs, struct ext2_inode *inode,

lib/ext2fs/getsectsize.c +33 -2

index ae9139d..0c7fa82 100644
@@ -26,15 +26,20 @@
 #include <linux/fd.h>
 #endif
 
-#if defined(__linux__) && defined(_IO) && !defined(BLKSSZGET)
+#if defined(__linux__) && defined(_IO)
+#if !defined(BLKSSZGET)
 #define BLKSSZGET  _IO(0x12,104)/* get block device sector size */
 #endif
+#if !defined(BLKPBSZGET)
+#define BLKPBSZGET _IO(0x12,123)/* get block physical sector size */
+#endif
+#endif
 
 #include "ext2_fs.h"
 #include "ext2fs.h"
 
 /*
- * Returns the number of blocks in a partition
+ * Returns the logical sector size of a device 
  */
 errcode_t ext2fs_get_device_sectsize(const char *file, int *sectsize)
 {

@@ -58,3 +63,29 @@ errcode_t ext2fs_get_device_sectsize(const char *file, int *sectsize)
 	close(fd);
 	return 0;
 }
+
+/*
+ * Returns the physical sector size of a device 
+ */
+errcode_t ext2fs_get_device_phys_sectsize(const char *file, int *sectsize)
+{
+	int	fd;
+
+#ifdef HAVE_OPEN64
+	fd = open64(file, O_RDONLY);
+#else
+	fd = open(file, O_RDONLY);
+#endif
+	if (fd < 0)
+		return errno;
+
+#ifdef BLKPBSZGET
+	if (ioctl(fd, BLKPBSZGET, sectsize) >= 0) {
+		close(fd);
+		return 0;
+	}
+#endif
+	*sectsize = 0;
+	close(fd);
+	return 0;
+}

lib/ext2fs/tst_getsectsize.c +10 -4

index cb1b8c6..31599d2 100644
@@ -27,7 +27,7 @@
 
 int main(int argc, char **argv)
 {
-	int	sectsize;
+	int	lsectsize, psectsize;
 	int	retval;
 
 	if (argc < 2) {

@@ -35,13 +35,19 @@ int main(int argc, char **argv)
 		exit(1);
 	}
 
-	retval = ext2fs_get_device_sectsize(argv[1], &sectsize);
+	retval = ext2fs_get_device_sectsize(argv[1], &lsectsize);
 	if (retval) {
 		com_err(argv[0], retval,
 			"while calling ext2fs_get_device_sectsize");
 		exit(1);
 	}
-	printf("Device %s has a hardware sector size of %d.\n",
-	       argv[1], sectsize);
+	retval = ext2fs_get_device_phys_sectsize(argv[1], &psectsize);
+	if (retval) {
+		com_err(argv[0], retval,
+			"while calling ext2fs_get_device_phys_sectsize");
+		exit(1);
+	}
+	printf("Device %s has logical/physical sector size of %d/%d.\n",
+	       argv[1], lsectsize, psectsize);
 	exit(0);
 }

misc/mke2fs.c +26 -6

index 94b4c81..1a1307b 100644
@@ -1068,7 +1068,7 @@ static void PRS(int argc, char *argv[])
 	int		inode_size = 0;
 	unsigned long	flex_bg_size = 0;
 	double		reserved_ratio = 5.0;
-	int		sector_size = 0;
+	int		lsector_size = 0, psector_size = 0;
 	int		show_version_only = 0;
 	unsigned long long num_inodes = 0; /* unsigned long long to catch too-large input */
 	errcode_t	retval;

@@ -1586,16 +1586,25 @@ got_size:
 	    ((tmp = getenv("MKE2FS_FIRST_META_BG"))))
 		fs_param.s_first_meta_bg = atoi(tmp);
 
-	/* Get the hardware sector size, if available */
-	retval = ext2fs_get_device_sectsize(device_name, &sector_size);
+	/* Get the hardware sector sizes, if available */
+	retval = ext2fs_get_device_sectsize(device_name, &lsector_size);
 	if (retval) {
 		com_err(program_name, retval,
 			_("while trying to determine hardware sector size"));
 		exit(1);
 	}
+	retval = ext2fs_get_device_phys_sectsize(device_name, &psector_size);
+	if (retval) {
+		com_err(program_name, retval,
+			_("while trying to determine physical sector size"));
+		exit(1);
+	}
+	/* Older kernels may not have physical/logical distinction */
+	if (!psector_size)
+		psector_size = lsector_size;
 
 	if ((tmp = getenv("MKE2FS_DEVICE_SECTSIZE")) != NULL)
-		sector_size = atoi(tmp);
+		psector_size = atoi(tmp);
 
 	if (blocksize <= 0) {
 		use_bsize = get_int_from_profile(fs_types, "blocksize", 4096);

@@ -1606,14 +1615,25 @@ got_size:
 			    (use_bsize > 4096))
 				use_bsize = 4096;
 		}
-		if (sector_size && use_bsize < sector_size)
-			use_bsize = sector_size;
+		if (psector_size && use_bsize < psector_size)
+			use_bsize = psector_size;
 		if ((blocksize < 0) && (use_bsize < (-blocksize)))
 			use_bsize = -blocksize;
 		blocksize = use_bsize;
 		ext2fs_blocks_count_set(&fs_param,
 					ext2fs_blocks_count(&fs_param) /
 					(blocksize / 1024));
+	} else {
+		if (blocksize < lsector_size ||			/* Impossible */
+		    (!force && (blocksize < psector_size))) {	/* Suboptimal */
+			com_err(program_name, EINVAL,
+				_("while setting blocksize; too small for device\n"));
+			exit(1);
+		} else if (blocksize < psector_size) {
+			fprintf(stderr, _("Warning: specified blocksize %d is "
+				"less than device physical sectorsize %d, "
+				"forced to continue\n"), blocksize, psector_size);
+		}
 	}
 
 	if (inode_ratio == 0) {

Eric Sandeen

author

2010-02-08 22:33 UTC #1 lore raw

PATCH v2 1/1 mke2fs: account for physical as well as logical sector size

Some devices, notably 4k sector drives, may have a 512 logical sector size, mapped onto a 4k physical sector size.

When mke2fs is ratcheting down the blocksize for small filesystems, or when a blocksize is specified on the commandline, we should not willingly go below the physical sector size of the device.

When a blocksize is specified, we -must- not go below the logical sector size of the device.

Add a new library function, ext2fs_get_device_phys_sectsize() to get the physical sector size if possible, and adjust the logic in mke2fs to enforce the above rules.

Signed-off-by Eric Sandeen <[email protected]>

---

(V2: just forgot to update the subject after changing the patch)

lib/ext2fs/ext2fs.h +1 -0

index 213a819..9c06048 100644
@@ -1086,6 +1086,7 @@ extern errcode_t ext2fs_get_device_size2(const char *file, int blocksize,
 
 /* getsectsize.c */
 errcode_t ext2fs_get_device_sectsize(const char *file, int *sectsize);
+errcode_t ext2fs_get_device_phys_sectsize(const char *file, int *sectsize);
 
 /* i_block.c */
 errcode_t ext2fs_iblk_add_blocks(ext2_filsys fs, struct ext2_inode *inode,

lib/ext2fs/getsectsize.c +33 -2

index ae9139d..0c7fa82 100644
@@ -26,15 +26,20 @@
 #include <linux/fd.h>
 #endif
 
-#if defined(__linux__) && defined(_IO) && !defined(BLKSSZGET)
+#if defined(__linux__) && defined(_IO)
+#if !defined(BLKSSZGET)
 #define BLKSSZGET  _IO(0x12,104)/* get block device sector size */
 #endif
+#if !defined(BLKPBSZGET)
+#define BLKPBSZGET _IO(0x12,123)/* get block physical sector size */
+#endif
+#endif
 
 #include "ext2_fs.h"
 #include "ext2fs.h"
 
 /*
- * Returns the number of blocks in a partition
+ * Returns the logical sector size of a device 
  */
 errcode_t ext2fs_get_device_sectsize(const char *file, int *sectsize)
 {

@@ -58,3 +63,29 @@ errcode_t ext2fs_get_device_sectsize(const char *file, int *sectsize)
 	close(fd);
 	return 0;
 }
+
+/*
+ * Returns the physical sector size of a device 
+ */
+errcode_t ext2fs_get_device_phys_sectsize(const char *file, int *sectsize)
+{
+	int	fd;
+
+#ifdef HAVE_OPEN64
+	fd = open64(file, O_RDONLY);
+#else
+	fd = open(file, O_RDONLY);
+#endif
+	if (fd < 0)
+		return errno;
+
+#ifdef BLKPBSZGET
+	if (ioctl(fd, BLKPBSZGET, sectsize) >= 0) {
+		close(fd);
+		return 0;
+	}
+#endif
+	*sectsize = 0;
+	close(fd);
+	return 0;
+}

lib/ext2fs/tst_getsectsize.c +10 -4

index cb1b8c6..31599d2 100644
@@ -27,7 +27,7 @@
 
 int main(int argc, char **argv)
 {
-	int	sectsize;
+	int	lsectsize, psectsize;
 	int	retval;
 
 	if (argc < 2) {

@@ -35,13 +35,19 @@ int main(int argc, char **argv)
 		exit(1);
 	}
 
-	retval = ext2fs_get_device_sectsize(argv[1], &sectsize);
+	retval = ext2fs_get_device_sectsize(argv[1], &lsectsize);
 	if (retval) {
 		com_err(argv[0], retval,
 			"while calling ext2fs_get_device_sectsize");
 		exit(1);
 	}
-	printf("Device %s has a hardware sector size of %d.\n",
-	       argv[1], sectsize);
+	retval = ext2fs_get_device_phys_sectsize(argv[1], &psectsize);
+	if (retval) {
+		com_err(argv[0], retval,
+			"while calling ext2fs_get_device_phys_sectsize");
+		exit(1);
+	}
+	printf("Device %s has logical/physical sector size of %d/%d.\n",
+	       argv[1], lsectsize, psectsize);
 	exit(0);
 }

misc/mke2fs.c +26 -6

index 94b4c81..1a1307b 100644
@@ -1068,7 +1068,7 @@ static void PRS(int argc, char *argv[])
 	int		inode_size = 0;
 	unsigned long	flex_bg_size = 0;
 	double		reserved_ratio = 5.0;
-	int		sector_size = 0;
+	int		lsector_size = 0, psector_size = 0;
 	int		show_version_only = 0;
 	unsigned long long num_inodes = 0; /* unsigned long long to catch too-large input */
 	errcode_t	retval;

@@ -1586,16 +1586,25 @@ got_size:
 	    ((tmp = getenv("MKE2FS_FIRST_META_BG"))))
 		fs_param.s_first_meta_bg = atoi(tmp);
 
-	/* Get the hardware sector size, if available */
-	retval = ext2fs_get_device_sectsize(device_name, &sector_size);
+	/* Get the hardware sector sizes, if available */
+	retval = ext2fs_get_device_sectsize(device_name, &lsector_size);
 	if (retval) {
 		com_err(program_name, retval,
 			_("while trying to determine hardware sector size"));
 		exit(1);
 	}
+	retval = ext2fs_get_device_phys_sectsize(device_name, &psector_size);
+	if (retval) {
+		com_err(program_name, retval,
+			_("while trying to determine physical sector size"));
+		exit(1);
+	}
+	/* Older kernels may not have physical/logical distinction */
+	if (!psector_size)
+		psector_size = lsector_size;
 
 	if ((tmp = getenv("MKE2FS_DEVICE_SECTSIZE")) != NULL)
-		sector_size = atoi(tmp);
+		psector_size = atoi(tmp);
 
 	if (blocksize <= 0) {
 		use_bsize = get_int_from_profile(fs_types, "blocksize", 4096);

@@ -1606,14 +1615,25 @@ got_size:
 			    (use_bsize > 4096))
 				use_bsize = 4096;
 		}
-		if (sector_size && use_bsize < sector_size)
-			use_bsize = sector_size;
+		if (psector_size && use_bsize < psector_size)
+			use_bsize = psector_size;
 		if ((blocksize < 0) && (use_bsize < (-blocksize)))
 			use_bsize = -blocksize;
 		blocksize = use_bsize;
 		ext2fs_blocks_count_set(&fs_param,
 					ext2fs_blocks_count(&fs_param) /
 					(blocksize / 1024));
+	} else {
+		if (blocksize < lsector_size ||			/* Impossible */
+		    (!force && (blocksize < psector_size))) {	/* Suboptimal */
+			com_err(program_name, EINVAL,
+				_("while setting blocksize; too small for device\n"));
+			exit(1);
+		} else if (blocksize < psector_size) {
+			fprintf(stderr, _("Warning: specified blocksize %d is "
+				"less than device physical sectorsize %d, "
+				"forced to continue\n"), blocksize, psector_size);
+		}
 	}
 
 	if (inode_ratio == 0) {

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

[email protected]

2010-02-09 22:22 UTC #2 lore raw

Re: [PATCH V2] mke2fs: account for physical as well as logical sector size

Quoted 7 lines · Eric Sandeen

Some devices, notably 4k sector drives, may have a 512 logical sector size, mapped onto a 4k physical sector size.
When mke2fs is ratcheting down the blocksize for small filesystems, or when a blocksize is specified on the commandline, we should not willingly go below the physical sector size of the device.

Quoted 6 lines

When a blocksize is specified, we -must- not go below the logical sector size of the device.
Add a new library function, ext2fs_get_device_phys_sectsize() to get the physical sector size if possible, and adjust the logic in mke2fs to enforce the above rules.

Was this something you think is worth trying to slip into 1.41.10?

Just checking... my default is to defer this unless you think its especially important.

- Ted

Eric Sandeen

author

2010-02-09 22:23 UTC #3 lore raw

Re: [PATCH V2] mke2fs: account for physical as well as logical sector size

Quoted 15 lines · [email protected] wrote:

Quoted 7 lines · Eric Sandeen
Some devices, notably 4k sector drives, may have a 512 logical sector size, mapped onto a 4k physical sector size.
When mke2fs is ratcheting down the blocksize for small filesystems, or when a blocksize is specified on the commandline, we should not willingly go below the physical sector size of the device.
Quoted 6 lines
When a blocksize is specified, we -must- not go below the logical sector size of the device.
Add a new library function, ext2fs_get_device_phys_sectsize() to get the physical sector size if possible, and adjust the logic in mke2fs to enforce the above rules.

Quoted 6 lines

Was this something you think is worth trying to slip into 1.41.10?
Just checking... my default is to defer this unless you think its especially important.
- Ted

I'm inclined to defer it. I can get it in as a patch if we need to.

I'd meant to send it earlier but well, you know ... ;)

Thanks,
-Eric

Eric Sandeen

author

2010-05-17 21:45 UTC #4 lore raw

Re: [PATCH V2] mke2fs: account for physical as well as logical sector size

In reply to Eric Sandeen

Quoted 7 lines · Eric Sandeen

Some devices, notably 4k sector drives, may have a 512 logical sector size, mapped onto a 4k physical sector size.
When mke2fs is ratcheting down the blocksize for small filesystems, or when a blocksize is specified on the commandline, we should not willingly go below the physical sector size of the device.

Quoted 8 lines

When a blocksize is specified, we -must- not go below the logical sector size of the device.
Add a new library function, ext2fs_get_device_phys_sectsize() to get the physical sector size if possible, and adjust the logic in mke2fs to enforce the above rules.
Signed-off-by Eric Sandeen <[email protected]>

ping on this one?

-Eric

Quoted 189 lines

---

(V2: just forgot to update the subject after changing the patch)

lib/ext2fs/ext2fs.h +1 -0

index 213a819..9c06048 100644
@@ -1086,6 +1086,7 @@ extern errcode_t ext2fs_get_device_size2(const char *file, int blocksize,
 
 /* getsectsize.c */
 errcode_t ext2fs_get_device_sectsize(const char *file, int *sectsize);
+errcode_t ext2fs_get_device_phys_sectsize(const char *file, int *sectsize);
 
 /* i_block.c */
 errcode_t ext2fs_iblk_add_blocks(ext2_filsys fs, struct ext2_inode *inode,

lib/ext2fs/getsectsize.c +33 -2

index ae9139d..0c7fa82 100644
@@ -26,15 +26,20 @@
 #include <linux/fd.h>
 #endif
 
-#if defined(__linux__) && defined(_IO) && !defined(BLKSSZGET)
+#if defined(__linux__) && defined(_IO)
+#if !defined(BLKSSZGET)
 #define BLKSSZGET  _IO(0x12,104)/* get block device sector size */
 #endif
+#if !defined(BLKPBSZGET)
+#define BLKPBSZGET _IO(0x12,123)/* get block physical sector size */
+#endif
+#endif
 
 #include "ext2_fs.h"
 #include "ext2fs.h"
 
 /*
- * Returns the number of blocks in a partition
+ * Returns the logical sector size of a device 
  */
 errcode_t ext2fs_get_device_sectsize(const char *file, int *sectsize)
 {

@@ -58,3 +63,29 @@ errcode_t ext2fs_get_device_sectsize(const char *file, int *sectsize)
 	close(fd);
 	return 0;
 }
+
+/*
+ * Returns the physical sector size of a device 
+ */
+errcode_t ext2fs_get_device_phys_sectsize(const char *file, int *sectsize)
+{
+	int	fd;
+
+#ifdef HAVE_OPEN64
+	fd = open64(file, O_RDONLY);
+#else
+	fd = open(file, O_RDONLY);
+#endif
+	if (fd < 0)
+		return errno;
+
+#ifdef BLKPBSZGET
+	if (ioctl(fd, BLKPBSZGET, sectsize) >= 0) {
+		close(fd);
+		return 0;
+	}
+#endif
+	*sectsize = 0;
+	close(fd);
+	return 0;
+}

lib/ext2fs/tst_getsectsize.c +10 -4

index cb1b8c6..31599d2 100644
@@ -27,7 +27,7 @@
 
 int main(int argc, char **argv)
 {
-	int	sectsize;
+	int	lsectsize, psectsize;
 	int	retval;
 
 	if (argc < 2) {

@@ -35,13 +35,19 @@ int main(int argc, char **argv)
 		exit(1);
 	}
 
-	retval = ext2fs_get_device_sectsize(argv[1], &sectsize);
+	retval = ext2fs_get_device_sectsize(argv[1], &lsectsize);
 	if (retval) {
 		com_err(argv[0], retval,
 			"while calling ext2fs_get_device_sectsize");
 		exit(1);
 	}
-	printf("Device %s has a hardware sector size of %d.\n",
-	       argv[1], sectsize);
+	retval = ext2fs_get_device_phys_sectsize(argv[1], &psectsize);
+	if (retval) {
+		com_err(argv[0], retval,
+			"while calling ext2fs_get_device_phys_sectsize");
+		exit(1);
+	}
+	printf("Device %s has logical/physical sector size of %d/%d.\n",
+	       argv[1], lsectsize, psectsize);
 	exit(0);
 }

misc/mke2fs.c +26 -7

index 94b4c81..1a1307b 100644
@@ -1068,7 +1068,7 @@ static void PRS(int argc, char *argv[])
 	int		inode_size = 0;
 	unsigned long	flex_bg_size = 0;
 	double		reserved_ratio = 5.0;
-	int		sector_size = 0;
+	int		lsector_size = 0, psector_size = 0;
 	int		show_version_only = 0;
 	unsigned long long num_inodes = 0; /* unsigned long long to catch too-large input */
 	errcode_t	retval;

@@ -1586,16 +1586,25 @@ got_size:
 	    ((tmp = getenv("MKE2FS_FIRST_META_BG"))))
 		fs_param.s_first_meta_bg = atoi(tmp);
 
-	/* Get the hardware sector size, if available */
-	retval = ext2fs_get_device_sectsize(device_name, &sector_size);
+	/* Get the hardware sector sizes, if available */
+	retval = ext2fs_get_device_sectsize(device_name, &lsector_size);
 	if (retval) {
 		com_err(program_name, retval,
 			_("while trying to determine hardware sector size"));
 		exit(1);
 	}
+	retval = ext2fs_get_device_phys_sectsize(device_name, &psector_size);
+	if (retval) {
+		com_err(program_name, retval,
+			_("while trying to determine physical sector size"));
+		exit(1);
+	}
+	/* Older kernels may not have physical/logical distinction */
+	if (!psector_size)
+		psector_size = lsector_size;
 
 	if ((tmp = getenv("MKE2FS_DEVICE_SECTSIZE")) != NULL)
-		sector_size = atoi(tmp);
+		psector_size = atoi(tmp);
 
 	if (blocksize <= 0) {
 		use_bsize = get_int_from_profile(fs_types, "blocksize", 4096);

@@ -1606,14 +1615,25 @@ got_size:
 			    (use_bsize > 4096))
 				use_bsize = 4096;
 		}
-		if (sector_size && use_bsize < sector_size)
-			use_bsize = sector_size;
+		if (psector_size && use_bsize < psector_size)
+			use_bsize = psector_size;
 		if ((blocksize < 0) && (use_bsize < (-blocksize)))
 			use_bsize = -blocksize;
 		blocksize = use_bsize;
 		ext2fs_blocks_count_set(&fs_param,
 					ext2fs_blocks_count(&fs_param) /
 					(blocksize / 1024));
+	} else {
+		if (blocksize < lsector_size ||			/* Impossible */
+		    (!force && (blocksize < psector_size))) {	/* Suboptimal */
+			com_err(program_name, EINVAL,
+				_("while setting blocksize; too small for device\n"));
+			exit(1);
+		} else if (blocksize < psector_size) {
+			fprintf(stderr, _("Warning: specified blocksize %d is "
+				"less than device physical sectorsize %d, "
+				"forced to continue\n"), blocksize, psector_size);
+		}
 	}
 
 	if (inode_ratio == 0) {
 
--

To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

[email protected]

Applied

2010-05-18 02:35 UTC #5 lore raw

Re: [PATCH V2] mke2fs: account for physical as well as logical sector size

Quoted 3 lines · Eric Sandeen

ping on this one?

Applied to the maint branch for the 1.41.12 release. Thanks for reminding me.

- Ted

Eric Sandeen

author

2010-05-18 03:13 UTC #6 lore raw

Re: [PATCH V2] mke2fs: account for physical as well as logical sector size

Quoted 8 lines · [email protected] wrote:

Quoted 2 lines · Eric Sandeen
ping on this one?
Applied to the maint branch for the 1.41.12 release. Thanks for reminding me.
- Ted

no problemo, thanks for merging it :)

-Eric