| Message ID | 4B70914B.3000102@redhat.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Mon, Feb 08, 2010 at 04:33:47PM -0600, Eric Sandeen wrote: > 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. 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 -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
tytso@mit.edu wrote: > On Mon, Feb 08, 2010 at 04:33:47PM -0600, Eric Sandeen wrote: >> 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. > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Eric Sandeen wrote: > 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 <sandeen@redhat.com> ping on this one? -Eric > --- > > (V2: just forgot to update the subject after changing the patch) > > diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h > index 213a819..9c06048 100644 > --- a/lib/ext2fs/ext2fs.h > +++ b/lib/ext2fs/ext2fs.h > @@ -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, > diff --git a/lib/ext2fs/getsectsize.c b/lib/ext2fs/getsectsize.c > index ae9139d..0c7fa82 100644 > --- a/lib/ext2fs/getsectsize.c > +++ b/lib/ext2fs/getsectsize.c > @@ -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; > +} > diff --git a/lib/ext2fs/tst_getsectsize.c b/lib/ext2fs/tst_getsectsize.c > index cb1b8c6..31599d2 100644 > --- a/lib/ext2fs/tst_getsectsize.c > +++ b/lib/ext2fs/tst_getsectsize.c > @@ -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], §size); > + 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); > } > diff --git a/misc/mke2fs.c b/misc/mke2fs.c > index 94b4c81..1a1307b 100644 > --- a/misc/mke2fs.c > +++ b/misc/mke2fs.c > @@ -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, §or_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 majordomo@vger.kernel.org > 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 majordomo@vger.kernel.org > 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 17, 2010 at 04:44:55PM -0500, Eric Sandeen wrote: > > ping on this one? > Applied to the maint branch for the 1.41.12 release. Thanks for reminding me. - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
tytso@mit.edu wrote: > On Mon, May 17, 2010 at 04:44:55PM -0500, Eric Sandeen wrote: >> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h index 213a819..9c06048 100644 --- a/lib/ext2fs/ext2fs.h +++ b/lib/ext2fs/ext2fs.h @@ -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, diff --git a/lib/ext2fs/getsectsize.c b/lib/ext2fs/getsectsize.c index ae9139d..0c7fa82 100644 --- a/lib/ext2fs/getsectsize.c +++ b/lib/ext2fs/getsectsize.c @@ -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; +} diff --git a/lib/ext2fs/tst_getsectsize.c b/lib/ext2fs/tst_getsectsize.c index cb1b8c6..31599d2 100644 --- a/lib/ext2fs/tst_getsectsize.c +++ b/lib/ext2fs/tst_getsectsize.c @@ -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], §size); + 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); } diff --git a/misc/mke2fs.c b/misc/mke2fs.c index 94b4c81..1a1307b 100644 --- a/misc/mke2fs.c +++ b/misc/mke2fs.c @@ -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, §or_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) {
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 <sandeen@redhat.com> --- (V2: just forgot to update the subject after changing the patch) -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html