I've rewritten the patch because it was simplest way to review the
code. The resulting code is simpler and has a smaller number of lines
of code. I don't have any 4k advanced format disks handy, so I'd
appreciate it if someone can test it. It passes the existing
regression tests, and I've run a number of manual tests to simulate a
advanced format HDD, with the tests being run with clang and UBSAN and
ADDRSAN enabled.
If someone with access to an advanced format disk can test running
debugfs -D on an advanced format disk, that would be great, thanks.
- Ted
commit c001596110e834a85b01a47a20811b318cb3b9e4
Author: Theodore Ts'o <tytso@xxxxxxx>
Date: Fri Feb 26 17:41:06 2021 -0500
libext2fs: fix unix_io's Direct I/O support
The previous Direct I/O support worked on HDD's with 512 byte logical
sector sizes, and on FreeBSD which required 4k aligned memory buffers.
However, it was incomplete and was not correctly working on HDD's with
4k logical sector sizes (aka Advanced Format Disks).
Based on a patch from Alexey Lyashkov <alexey.lyashkov@xxxxxxx> but
rewritten to work with the latest e2fsprogs and to minimize changes to
make it easier to review.
Signed-off-by: Theodore Ts'o <tytso@xxxxxxx>
Reported-by: Artem Blagodarenko <artem.blagodarenko@xxxxxxxxx>
diff --git a/lib/ext2fs/io_manager.c b/lib/ext2fs/io_manager.c
index c395d615..996c31a1 100644
--- a/lib/ext2fs/io_manager.c
+++ b/lib/ext2fs/io_manager.c
@@ -134,9 +134,11 @@ errcode_t io_channel_alloc_buf(io_channel io, int count, void *ptr)
else
size = -count;
- if (io->align)
+ if (io->align) {
+ if (io->align > size)
+ size = io->align;
return ext2fs_get_memalign(size, io->align, ptr);
- else
+ } else
return ext2fs_get_mem(size, ptr);
}
diff --git a/lib/ext2fs/unix_io.c b/lib/ext2fs/unix_io.c
index 73f5667e..8965535c 100644
--- a/lib/ext2fs/unix_io.c
+++ b/lib/ext2fs/unix_io.c
@@ -218,6 +218,8 @@ static errcode_t raw_read_blk(io_channel channel,
int actual = 0;
unsigned char *buf = bufv;
ssize_t really_read = 0;
+ unsigned long long aligned_blk;
+ int align_size, offset;
size = (count < 0) ? -count : (ext2_loff_t) count * channel->block_size;
mutex_lock(data, STATS_MTX);
@@ -226,7 +228,7 @@ static errcode_t raw_read_blk(io_channel channel,
location = ((ext2_loff_t) block * channel->block_size) + data->offset;
if (data->flags & IO_FLAG_FORCE_BOUNCE) {
- if (ext2fs_llseek(data->dev, location, SEEK_SET) != location) {
+ if (ext2fs_llseek(data->dev, location, SEEK_SET) < 0) {
retval = errno ? errno : EXT2_ET_LLSEEK_FAILED;
goto error_out;
}
@@ -237,6 +239,7 @@ static errcode_t raw_read_blk(io_channel channel,
/* Try an aligned pread */
if ((channel->align == 0) ||
(IS_ALIGNED(buf, channel->align) &&
+ IS_ALIGNED(location, channel->align) &&
IS_ALIGNED(size, channel->align))) {
actual = pread64(data->dev, buf, size, location);
if (actual == size)
@@ -248,6 +251,7 @@ static errcode_t raw_read_blk(io_channel channel,
if ((sizeof(off_t) >= sizeof(ext2_loff_t)) &&
((channel->align == 0) ||
(IS_ALIGNED(buf, channel->align) &&
+ IS_ALIGNED(location, channel->align) &&
IS_ALIGNED(size, channel->align)))) {
actual = pread(data->dev, buf, size, location);
if (actual == size)
@@ -256,12 +260,13 @@ static errcode_t raw_read_blk(io_channel channel,
}
#endif /* HAVE_PREAD */
- if (ext2fs_llseek(data->dev, location, SEEK_SET) != location) {
+ if (ext2fs_llseek(data->dev, location, SEEK_SET) < 0) {
retval = errno ? errno : EXT2_ET_LLSEEK_FAILED;
goto error_out;
}
if ((channel->align == 0) ||
(IS_ALIGNED(buf, channel->align) &&
+ IS_ALIGNED(location, channel->align) &&
IS_ALIGNED(size, channel->align))) {
actual = read(data->dev, buf, size);
if (actual != size) {
@@ -286,23 +291,39 @@ static errcode_t raw_read_blk(io_channel channel,
* to the O_DIRECT rules, so we need to do this the hard way...
*/
bounce_read:
+ if ((channel->block_size > channel->align) &&
+ (channel->block_size % channel->align) == 0)
+ align_size = channel->block_size;
+ else
+ align_size = channel->align;
+ aligned_blk = location / align_size;
+ offset = location % align_size;
+
+ if (ext2fs_llseek(data->dev, aligned_blk * align_size, SEEK_SET) < 0) {
+ retval = errno ? errno : EXT2_ET_LLSEEK_FAILED;
+ goto error_out;
+ }
while (size > 0) {
mutex_lock(data, BOUNCE_MTX);
- actual = read(data->dev, data->bounce, channel->block_size);
- if (actual != channel->block_size) {
+ actual = read(data->dev, data->bounce, align_size);
+ if (actual != align_size) {
mutex_unlock(data, BOUNCE_MTX);
actual = really_read;
buf -= really_read;
size += really_read;
goto short_read;
}
- actual = size;
- if (size > channel->block_size)
- actual = channel->block_size;
- memcpy(buf, data->bounce, actual);
+ if ((actual + offset) > align_size)
+ actual = align_size - offset;
+ if (actual > size)
+ actual = size;
+ memcpy(buf, data->bounce + offset, actual);
+
really_read += actual;
size -= actual;
buf += actual;
+ offset = 0;
+ aligned_blk++;
mutex_unlock(data, BOUNCE_MTX);
}
return 0;
@@ -326,6 +347,8 @@ static errcode_t raw_write_blk(io_channel channel,
int actual = 0;
errcode_t retval;
const unsigned char *buf = bufv;
+ unsigned long long aligned_blk;
+ int align_size, offset;
if (count == 1)
size = channel->block_size;
@@ -342,7 +365,7 @@ static errcode_t raw_write_blk(io_channel channel,
location = ((ext2_loff_t) block * channel->block_size) + data->offset;
if (data->flags & IO_FLAG_FORCE_BOUNCE) {
- if (ext2fs_llseek(data->dev, location, SEEK_SET) != location) {
+ if (ext2fs_llseek(data->dev, location, SEEK_SET) < 0) {
retval = errno ? errno : EXT2_ET_LLSEEK_FAILED;
goto error_out;
}
@@ -353,6 +376,7 @@ static errcode_t raw_write_blk(io_channel channel,
/* Try an aligned pwrite */
if ((channel->align == 0) ||
(IS_ALIGNED(buf, channel->align) &&
+ IS_ALIGNED(location, channel->align) &&
IS_ALIGNED(size, channel->align))) {
actual = pwrite64(data->dev, buf, size, location);
if (actual == size)
@@ -363,6 +387,7 @@ static errcode_t raw_write_blk(io_channel channel,
if ((sizeof(off_t) >= sizeof(ext2_loff_t)) &&
((channel->align == 0) ||
(IS_ALIGNED(buf, channel->align) &&
+ IS_ALIGNED(location, channel->align) &&
IS_ALIGNED(size, channel->align)))) {
actual = pwrite(data->dev, buf, size, location);
if (actual == size)
@@ -370,13 +395,14 @@ static errcode_t raw_write_blk(io_channel channel,
}
#endif /* HAVE_PWRITE */
- if (ext2fs_llseek(data->dev, location, SEEK_SET) != location) {
+ if (ext2fs_llseek(data->dev, location, SEEK_SET) < 0) {
retval = errno ? errno : EXT2_ET_LLSEEK_FAILED;
goto error_out;
}
if ((channel->align == 0) ||
(IS_ALIGNED(buf, channel->align) &&
+ IS_ALIGNED(location, channel->align) &&
IS_ALIGNED(size, channel->align))) {
actual = write(data->dev, buf, size);
if (actual < 0) {
@@ -400,40 +426,59 @@ static errcode_t raw_write_blk(io_channel channel,
* to the O_DIRECT rules, so we need to do this the hard way...
*/
bounce_write:
+ if ((channel->block_size > channel->align) &&
+ (channel->block_size % channel->align) == 0)
+ align_size = channel->block_size;
+ else
+ align_size = channel->align;
+ aligned_blk = location / align_size;
+ offset = location % align_size;
+
while (size > 0) {
+ int actual_w;
+
mutex_lock(data, BOUNCE_MTX);
- if (size < channel->block_size) {
+ if (size < align_size || offset) {
+ if (ext2fs_llseek(data->dev, aligned_blk * align_size,
+ SEEK_SET) < 0) {
+ retval = errno ? errno : EXT2_ET_LLSEEK_FAILED;
+ goto error_out;
+ }
actual = read(data->dev, data->bounce,
- channel->block_size);
- if (actual != channel->block_size) {
+ align_size);
+ if (actual != align_size) {
if (actual < 0) {
mutex_unlock(data, BOUNCE_MTX);
retval = errno;
goto error_out;
}
memset((char *) data->bounce + actual, 0,
- channel->block_size - actual);
+ align_size - actual);
}
}
actual = size;
- if (size > channel->block_size)
- actual = channel->block_size;
- memcpy(data->bounce, buf, actual);
- if (ext2fs_llseek(data->dev, location, SEEK_SET) != location) {
+ if ((actual + offset) > align_size)
+ actual = align_size - offset;
+ if (actual > size)
+ actual = size;
+ memcpy(((char *)data->bounce) + offset, buf, actual);
+ if (ext2fs_llseek(data->dev, aligned_blk * align_size, SEEK_SET) < 0) {
retval = errno ? errno : EXT2_ET_LLSEEK_FAILED;
goto error_out;
}
- actual = write(data->dev, data->bounce, channel->block_size);
+ actual_w = write(data->dev, data->bounce, align_size);
mutex_unlock(data, BOUNCE_MTX);
- if (actual < 0) {
+ if (actual_w < 0) {
retval = errno;
goto error_out;
}
- if (actual != channel->block_size)
+ if (actual_w != align_size)
goto short_write;
size -= actual;
buf += actual;
location += actual;
+ aligned_blk++;
+ offset = 0;
}
return 0;
