[PATCH] libfs: Fix DIO mode aligment

* [PATCH] libfs: Fix DIO mode aligment
@ 2020-10-23 11:26 Artem Blagodarenko
  2020-11-17 15:30 ` Благодаренко Артём
  2021-02-26 23:17 ` Theodore Ts'o
  0 siblings, 2 replies; 10+ messages in thread
From: Artem Blagodarenko @ 2020-10-23 11:26 UTC (permalink / raw)
  To: linux-ext4; +Cc: adilger.kernel, Alexey Lyashkov

From: Alexey Lyashkov <alexey.lyashkov@hpe.com>

Bounce buffer must be extended to hold a whole disk block size.
Read/write operations with unaligned offsets is prohobined
in the DIO mode, so read/write blocks should be adjusted to
reflect it.

Change-Id: Ic573c9ff0d476028dd2293f8b814c6112705db0e
Signed-off-by: Alexey Lyashkov <alexey.lyashkov@hpe.com>
HPE-bug-id: LUS-9241
---
 lib/ext2fs/io_manager.c |   5 +-
 lib/ext2fs/unix_io.c    | 296 +++++++++++++++++++++++++---------------
 2 files changed, 190 insertions(+), 111 deletions(-)

diff --git a/lib/ext2fs/io_manager.c b/lib/ext2fs/io_manager.c
index c395d615..84399a12 100644
--- a/lib/ext2fs/io_manager.c
+++ b/lib/ext2fs/io_manager.c
@@ -20,6 +20,9 @@
 #include "ext2_fs.h"
 #include "ext2fs.h"
 
+#define max(a, b) ((a) > (b) ? (a) : (b))
+
+
 errcode_t io_channel_set_options(io_channel channel, const char *opts)
 {
 	errcode_t retval = 0;
@@ -128,7 +131,7 @@ errcode_t io_channel_alloc_buf(io_channel io, int count, void *ptr)
 	size_t	size;
 
 	if (count == 0)
-		size = io->block_size;
+		size = max(io->block_size, io->align);
 	else if (count > 0)
 		size = io->block_size * count;
 	else
diff --git a/lib/ext2fs/unix_io.c b/lib/ext2fs/unix_io.c
index 628e60c3..53647a22 100644
--- a/lib/ext2fs/unix_io.c
+++ b/lib/ext2fs/unix_io.c
@@ -154,46 +154,30 @@ static char *safe_getenv(const char *arg)
 /*
  * Here are the raw I/O functions
  */
-static errcode_t raw_read_blk(io_channel channel,
+static errcode_t raw_aligned_read_blk(io_channel channel,
 			      struct unix_private_data *data,
-			      unsigned long long block,
-			      int count, void *bufv)
+			      ext2_loff_t location,
+			      ssize_t size, void *bufv,
+			      int *asize)
 {
-	errcode_t	retval;
-	ssize_t		size;
-	ext2_loff_t	location;
 	int		actual = 0;
+	errcode_t	retval;
 	unsigned char	*buf = bufv;
-	ssize_t		really_read = 0;
-
-	size = (count < 0) ? -count : (ext2_loff_t) count * channel->block_size;
-	data->io_stats.bytes_read += size;
-	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) {
-			retval = errno ? errno : EXT2_ET_LLSEEK_FAILED;
-			goto error_out;
-		}
-		goto bounce_read;
-	}
+#ifdef ALIGN_DEBUG
+	printf("raw_aligned_read_blk: %p %lu<>%lu\n", buf,
+		location, (unsigned long) size);
+#endif
 
 #ifdef HAVE_PREAD64
 	/* Try an aligned pread */
-	if ((channel->align == 0) ||
-	    (IS_ALIGNED(buf, channel->align) &&
-	     IS_ALIGNED(size, channel->align))) {
-		actual = pread64(data->dev, buf, size, location);
-		if (actual == size)
-			return 0;
-		actual = 0;
-	}
+	actual = pread64(data->dev, buf, size, location);
+	if (actual == size)
+		return 0;
+	actual = 0;
 #elif HAVE_PREAD
 	/* Try an aligned pread */
-	if ((sizeof(off_t) >= sizeof(ext2_loff_t)) &&
-	    ((channel->align == 0) ||
-	     (IS_ALIGNED(buf, channel->align) &&
-	      IS_ALIGNED(size, channel->align)))) {
+	if (sizeof(off_t) >= sizeof(ext2_loff_t)) {
 		actual = pread(data->dev, buf, size, location);
 		if (actual == size)
 			return 0;
@@ -205,47 +189,100 @@ static errcode_t raw_read_blk(io_channel channel,
 		retval = errno ? errno : EXT2_ET_LLSEEK_FAILED;
 		goto error_out;
 	}
+
+	actual = read(data->dev, buf, size);
+	if (actual != size) {
+		if (actual < 0) {
+			retval = errno;
+			actual = 0;
+		} else {
+			retval = EXT2_ET_SHORT_READ;
+		}
+	}
+
+error_out:
+	*asize = actual;
+	return retval;
+}
+
+
+/*
+ * Here are the raw I/O functions
+ */
+static errcode_t raw_read_blk(io_channel channel,
+			      struct unix_private_data *data,
+			      unsigned long long block,
+			      int count, void *bufv)
+{
+	errcode_t	retval;
+	ssize_t		size;
+	ext2_loff_t	location;
+	int		actual = 0;
+	unsigned char	*buf = bufv;
+	unsigned int	align = channel->align;
+	ssize_t		really_read = 0;
+	blk64_t		blk;
+	loff_t		offset;
+
+	size = (count < 0) ? -count : count * channel->block_size;
+	data->io_stats.bytes_read += size;
+	location = ((ext2_loff_t) block * channel->block_size) + data->offset;
+
+	if (data->flags & IO_FLAG_FORCE_BOUNCE) {
+		align = channel->block_size;
+		goto bounce_read;
+	}
+
 	if ((channel->align == 0) ||
-	    (IS_ALIGNED(buf, channel->align) &&
+	    (IS_ALIGNED(location, channel->align) &&
+	     IS_ALIGNED(buf, channel->align) &&
 	     IS_ALIGNED(size, channel->align))) {
-		actual = read(data->dev, buf, size);
-		if (actual != size) {
-		short_read:
-			if (actual < 0) {
-				retval = errno;
-				actual = 0;
-			} else
-				retval = EXT2_ET_SHORT_READ;
+		retval = raw_aligned_read_blk(channel, data, location,
+						size, bufv, &actual);
+		if (retval != 0)
 			goto error_out;
-		}
-		return 0;
+
+		return retval;
 	}
 
+bounce_read:
 #ifdef ALIGN_DEBUG
-	printf("raw_read_blk: O_DIRECT fallback: %p %lu\n", buf,
-	       (unsigned long) size);
+	printf("raw_read_blk: O_DIRECT fallback: %p %lu<>%lu\n", buf,
+		location, (unsigned long) size);
 #endif
-
 	/*
 	 * The buffer or size which we're trying to read isn't aligned
 	 * to the O_DIRECT rules, so we need to do this the hard way...
+	 * read / write must be aligned to the block device sector size
 	 */
-bounce_read:
+
+	blk = location / align;
+	offset = location % align;
+
+	if (lseek(data->dev, blk * align, SEEK_SET) < 0)
+		return errno;
+
 	while (size > 0) {
-		actual = read(data->dev, data->bounce, channel->block_size);
-		if (actual != channel->block_size) {
+		actual = read(data->dev, data->bounce, align);
+		if (actual != align) {
 			actual = really_read;
 			buf -= really_read;
 			size += really_read;
-			goto short_read;
+			retval = EXT2_ET_SHORT_READ;
+			goto error_out;
 		}
 		actual = size;
-		if (size > channel->block_size)
-			actual = channel->block_size;
-		memcpy(buf, data->bounce, actual);
+		if ((actual + offset) > align)
+			actual = align - offset;
+		if (actual > size)
+			actual = size;
+
+		memcpy(buf, data->bounce + offset, actual);
 		really_read += actual;
 		size -= actual;
 		buf += actual;
+		offset = 0;
+		blk++;
 	}
 	return 0;
 
@@ -258,6 +295,58 @@ error_out:
 	return retval;
 }
 
+static errcode_t raw_aligned_write_blk(io_channel channel,
+			       struct unix_private_data *data,
+			       ext2_loff_t  location,
+			       ssize_t size, const void *bufv,
+			       int *asize)
+
+{
+	int		actual = 0;
+	errcode_t	retval;
+	const unsigned char *buf = (void *)bufv;
+
+#ifdef ALIGN_DEBUG
+	printf("raw_aligned_write_blk: %p %lu %lu\n", buf,
+	       location, (unsigned long) size);
+#endif
+
+#ifdef HAVE_PWRITE64
+	/* Try an aligned pwrite */
+	actual = pwrite64(data->dev, buf, size, location);
+	if (actual == size)
+		return 0;
+#elif HAVE_PWRITE
+	/* Try an aligned pwrite */
+	if ((sizeof(off_t) >= sizeof(ext2_loff_t)) {
+		actual = pwrite(data->dev, buf, size, location);
+		if (actual == size)
+			return 0;
+	}
+#endif /* HAVE_PWRITE */
+
+	if (ext2fs_llseek(data->dev, location, SEEK_SET) != location) {
+		retval = errno ? errno : EXT2_ET_LLSEEK_FAILED;
+		goto error_out;
+	}
+
+	actual = write(data->dev, buf, size);
+	if (actual < 0) {
+		retval = errno;
+		goto error_out;
+	}
+	if (actual != size) {
+		retval = EXT2_ET_SHORT_WRITE;
+		goto error_out;
+	}
+	retval = 0;
+error_out:
+	*asize = actual;
+	return retval;
+
+}
+
+
 static errcode_t raw_write_blk(io_channel channel,
 			       struct unix_private_data *data,
 			       unsigned long long block,
@@ -268,6 +357,9 @@ static errcode_t raw_write_blk(io_channel channel,
 	int		actual = 0;
 	errcode_t	retval;
 	const unsigned char *buf = bufv;
+	unsigned int	align = channel->align;
+	blk64_t		blk;
+	loff_t		offset;
 
 	if (count == 1)
 		size = channel->block_size;
@@ -282,95 +374,79 @@ 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) {
-			retval = errno ? errno : EXT2_ET_LLSEEK_FAILED;
-			goto error_out;
-		}
+		align = channel->block_size;
 		goto bounce_write;
 	}
 
-#ifdef HAVE_PWRITE64
-	/* Try an aligned pwrite */
 	if ((channel->align == 0) ||
-	    (IS_ALIGNED(buf, channel->align) &&
+	    (IS_ALIGNED(location, channel->align) &&
+	     IS_ALIGNED(buf, channel->align) &&
 	     IS_ALIGNED(size, channel->align))) {
-		actual = pwrite64(data->dev, buf, size, location);
-		if (actual == size)
-			return 0;
-	}
-#elif HAVE_PWRITE
-	/* Try an aligned pwrite */
-	if ((sizeof(off_t) >= sizeof(ext2_loff_t)) &&
-	    ((channel->align == 0) ||
-	     (IS_ALIGNED(buf, channel->align) &&
-	      IS_ALIGNED(size, channel->align)))) {
-		actual = pwrite(data->dev, buf, size, location);
-		if (actual == size)
-			return 0;
-	}
-#endif /* HAVE_PWRITE */
+		retval = raw_aligned_write_blk(channel, data, location,
+						size, bufv, &actual);
+		if (retval != 0)
+			goto error_out;
 
-	if (ext2fs_llseek(data->dev, location, SEEK_SET) != location) {
-		retval = errno ? errno : EXT2_ET_LLSEEK_FAILED;
-		goto error_out;
+		return retval;
 	}
 
-	if ((channel->align == 0) ||
-	    (IS_ALIGNED(buf, channel->align) &&
-	     IS_ALIGNED(size, channel->align))) {
-		actual = write(data->dev, buf, size);
-		if (actual < 0) {
-			retval = errno;
-			goto error_out;
-		}
-		if (actual != size) {
-		short_write:
-			retval = EXT2_ET_SHORT_WRITE;
-			goto error_out;
-		}
-		return 0;
-	}
 
+bounce_write:
 #ifdef ALIGN_DEBUG
 	printf("raw_write_blk: O_DIRECT fallback: %p %lu\n", buf,
 	       (unsigned long) size);
 #endif
+
+	/* logical offset may don't aligned with block device block size */
+	blk = location / align;
+	offset = location % align;
+
+	if (lseek(data->dev, blk * align, SEEK_SET) != blk * align) {
+		retval = errno ? errno : EXT2_ET_LLSEEK_FAILED;
+		goto error_out;
+	}
+
 	/*
 	 * The buffer or size which we're trying to write isn't aligned
 	 * to the O_DIRECT rules, so we need to do this the hard way...
 	 */
-bounce_write:
 	while (size > 0) {
-		if (size < channel->block_size) {
-			actual = read(data->dev, data->bounce,
-				      channel->block_size);
-			if (actual != channel->block_size) {
-				if (actual < 0) {
-					retval = errno;
-					goto error_out;
-				}
-				memset((char *) data->bounce + actual, 0,
-				       channel->block_size - actual);
+		int actual_w;
+
+		memset((char *) data->bounce, 0, align);
+		if (offset || (size < align)) {
+			actual = read(data->dev, data->bounce, align);
+			if (actual < 0) {
+				retval = errno;
+				goto error_out;
 			}
 		}
 		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)
+			actual = align - offset;
+		if (actual > size)
+			actual = size;
+		memcpy(((char *)data->bounce) + offset, buf, actual);
+
+		if (lseek(data->dev, blk * align, SEEK_SET) != blk * align) {
 			retval = errno ? errno : EXT2_ET_LLSEEK_FAILED;
 			goto error_out;
 		}
-		actual = write(data->dev, data->bounce, channel->block_size);
-		if (actual < 0) {
+
+		actual_w = write(data->dev, data->bounce, align);
+		if (actual_w < 0) {
 			retval = errno;
 			goto error_out;
 		}
-		if (actual != channel->block_size)
-			goto short_write;
+		if (actual_w != align) {
+			retval = EXT2_ET_SHORT_WRITE;
+			goto error_out;
+		}
 		size -= actual;
 		buf += actual;
 		location += actual;
+		blk ++;
+		offset = 0;
 	}
 	return 0;
 
-- 
2.18.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread