[PATCH 0/5] Clean up I/O buffer allocation in e2fsprogs

[PATCH 0/5] Clean up I/O buffer allocation in e2fsprogs

[Theodore Ts'o]

Andreas pointed out that original version of the last patch in this
series would break the 16k/64k blocksize regressiont tests on MacOS
10.5.  When I looked into addressing this, I realized that we didn't
need to request 16k or 64k alignment, and in general we often requested
aligned buffers when they were not necessary, and/or requested a larger
alignment factor than necessary.

This patch series cleans up this mess by refactoring the code so that
DIO alignment is calculated in one place, and so we have a common
routine for allocating I/O buffers used by e2fsprogs's I/O layer.

	    	       	   	   	   - Ted


Theodore Ts'o (5):
  libext2fs: move the alignment field from unix_io to the io_manager
  libext2fs: refactor Direct I/O alignment requirement calculations
  libext2fs: make read_bitmaps() more efficient when using direct I/O
  libext2fs: factor out I/O buffer allocation
  Support systems without posix_memalign() and memalign()

 lib/ext2fs/Makefile.in   |    9 +++++++-
 lib/ext2fs/ext2_io.h     |    3 +++
 lib/ext2fs/ext2fs.h      |    1 +
 lib/ext2fs/getsectsize.c |   26 +++++++++++++++++++++
 lib/ext2fs/inline.c      |   56 ++++++++++++++++++++++++++++++++++++++++++++--
 lib/ext2fs/inode.c       |    4 ++--
 lib/ext2fs/io_manager.c  |   17 ++++++++++++++
 lib/ext2fs/mmp.c         |   37 ++++++++----------------------
 lib/ext2fs/openfs.c      |    2 +-
 lib/ext2fs/rw_bitmaps.c  |   24 ++++++++------------
 lib/ext2fs/test_io.c     |    7 +++++-
 lib/ext2fs/unix_io.c     |   42 +++++++++++++++-------------------
 12 files changed, 154 insertions(+), 74 deletions(-)

-- 
1.7.10.rc3

[PATCH 1/5] libext2fs: move the alignment field from unix_io to the io_manager

[Theodore Ts'o]

The align field which indicated the required data alignment of data
buffers was stored in a field specific to the unix_io manager.  Move
it to the top-level io_channel structure so it can be better
generalized.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 lib/ext2fs/ext2_io.h |    1 +
 lib/ext2fs/test_io.c |    7 ++++++-
 lib/ext2fs/unix_io.c |   25 ++++++++++++++-----------
 3 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/lib/ext2fs/ext2_io.h b/lib/ext2fs/ext2_io.h
index bcc2f87..95b8de3 100644
--- a/lib/ext2fs/ext2_io.h
+++ b/lib/ext2fs/ext2_io.h
@@ -58,6 +58,7 @@ struct struct_io_channel {
 	long		reserved[14];
 	void		*private_data;
 	void		*app_data;
+	int		align;
 };
 
 struct struct_io_stats {
diff --git a/lib/ext2fs/test_io.c b/lib/ext2fs/test_io.c
index f67f6ae..7d3cdfe 100644
--- a/lib/ext2fs/test_io.c
+++ b/lib/ext2fs/test_io.c
@@ -247,6 +247,9 @@ static errcode_t test_open(const char *name, int flags, io_channel *channel)
 	if ((value = safe_getenv("TEST_IO_WRITE_ABORT")) != NULL)
 		data->write_abort_count = strtoul(value, NULL, 0);
 
+	if (data->real)
+		io->align = data->real->align;
+
 	*channel = io;
 	return 0;
 
@@ -292,8 +295,10 @@ static errcode_t test_set_blksize(io_channel channel, int blksize)
 	data = (struct test_private_data *) channel->private_data;
 	EXT2_CHECK_MAGIC(data, EXT2_ET_MAGIC_TEST_IO_CHANNEL);
 
-	if (data->real)
+	if (data->real) {
 		retval = io_channel_set_blksize(data->real, blksize);
+		channel->align = data->real->align;
+	}
 	if (data->set_blksize)
 		data->set_blksize(blksize, retval);
 	if (data->flags & TEST_FLAG_SET_BLKSIZE)
diff --git a/lib/ext2fs/unix_io.c b/lib/ext2fs/unix_io.c
index da3f8fd..3269392 100644
--- a/lib/ext2fs/unix_io.c
+++ b/lib/ext2fs/unix_io.c
@@ -180,8 +180,9 @@ static errcode_t raw_read_blk(io_channel channel,
 		retval = errno ? errno : EXT2_ET_LLSEEK_FAILED;
 		goto error_out;
 	}
-	if ((data->align == 0) ||
-	    ((IS_ALIGNED(buf, data->align)) && IS_ALIGNED(size, data->align))) {
+	if ((channel->align == 0) ||
+	    (IS_ALIGNED(buf, channel->align) &&
+	     IS_ALIGNED(size, channel->align))) {
 		actual = read(data->dev, buf, size);
 		if (actual != size) {
 		short_read:
@@ -250,8 +251,9 @@ static errcode_t raw_write_blk(io_channel channel,
 		goto error_out;
 	}
 
-	if ((data->align == 0) ||
-	    ((IS_ALIGNED(buf, data->align)) && IS_ALIGNED(size, data->align))) {
+	if ((channel->align == 0) ||
+	    (IS_ALIGNED(buf, channel->align) &&
+	     IS_ALIGNED(size, channel->align))) {
 		actual = write(data->dev, buf, size);
 		if (actual != size) {
 		short_write:
@@ -319,14 +321,15 @@ static errcode_t alloc_cache(io_channel channel,
 		if (cache->buf)
 			ext2fs_free_mem(&cache->buf);
 		retval = ext2fs_get_memalign(channel->block_size,
-					     data->align, &cache->buf);
+					     channel->align, &cache->buf);
 		if (retval)
 			return retval;
 	}
-	if (data->align) {
+	if (channel->align) {
 		if (data->bounce)
 			ext2fs_free_mem(&data->bounce);
-		retval = ext2fs_get_memalign(channel->block_size, data->align,
+		retval = ext2fs_get_memalign(channel->block_size,
+					     channel->align,
 					     &data->bounce);
 	}
 	return retval;
@@ -518,8 +521,8 @@ static errcode_t unix_open(const char *name, int flags, io_channel *channel)
 
 #ifdef BLKSSZGET
 	if (flags & IO_FLAG_DIRECT_IO) {
-		if (ioctl(data->dev, BLKSSZGET, &data->align) != 0)
-			data->align = io->block_size;
+		if (ioctl(data->dev, BLKSSZGET, &io->align) != 0)
+			io->align = io->block_size;
 	}
 #endif
 
@@ -534,7 +537,7 @@ static errcode_t unix_open(const char *name, int flags, io_channel *channel)
 	 * Some operating systems require that the buffers be aligned,
 	 * regardless of O_DIRECT
 	 */
-	data->align = 512;
+	io->align = 512;
 #endif
 
 
@@ -810,7 +813,7 @@ static errcode_t unix_write_byte(io_channel channel, unsigned long offset,
 	data = (struct unix_private_data *) channel->private_data;
 	EXT2_CHECK_MAGIC(data, EXT2_ET_MAGIC_UNIX_IO_CHANNEL);
 
-	if (data->align != 0) {
+	if (channel->align != 0) {
 #ifdef ALIGN_DEBUG
 		printf("unix_write_byte: O_DIRECT fallback\n");
 #endif
-- 
1.7.10.rc3

[PATCH 2/5] libext2fs: refactor Direct I/O alignment requirement calculations

[Theodore Ts'o]

Create a new function, ext2fs_get_dio_alignment(), which returns the
alignment requirements for direct I/O.  This way we can factor out the
code from MMP and the Unix I/O manager.  The two modules weren't
consistently calculating the alignment factors, and in particular MMP
would sometimes use a larger alignment factor than was strictly
necessary.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 lib/ext2fs/ext2fs.h      |    1 +
 lib/ext2fs/getsectsize.c |   26 ++++++++++++++++++++++++++
 lib/ext2fs/mmp.c         |   37 +++++++++----------------------------
 lib/ext2fs/unix_io.c     |   22 ++++++++--------------
 4 files changed, 44 insertions(+), 42 deletions(-)

diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index fda6ade..9a0e736 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -1213,6 +1213,7 @@ extern errcode_t ext2fs_get_device_size2(const char *file, int blocksize,
 					blk64_t *retblocks);
 
 /* getsectsize.c */
+extern int ext2fs_get_dio_alignment(int fd);
 errcode_t ext2fs_get_device_sectsize(const char *file, int *sectsize);
 errcode_t ext2fs_get_device_phys_sectsize(const char *file, int *sectsize);
 
diff --git a/lib/ext2fs/getsectsize.c b/lib/ext2fs/getsectsize.c
index 30faecc..9c3f4a2 100644
--- a/lib/ext2fs/getsectsize.c
+++ b/lib/ext2fs/getsectsize.c
@@ -62,6 +62,32 @@ errcode_t ext2fs_get_device_sectsize(const char *file, int *sectsize)
 }
 
 /*
+ * Return desired alignment for direct I/O
+ */
+int ext2fs_get_dio_alignment(int fd)
+{
+	int align = 0;
+
+#ifdef BLKSSZGET
+	if (ioctl(fd, BLKSSZGET, &align) < 0)
+		align = 0;
+#endif
+
+#ifdef _SC_PAGESIZE
+	if (align <= 0)
+		align = sysconf(_SC_PAGESIZE);
+#endif
+#ifdef HAVE_GETPAGESIZE
+	if (align <= 0)
+		align = getpagesize();
+#endif
+	if (align <= 0)
+		align = 4096;
+
+	return align;
+}
+
+/*
  * Returns the physical sector size of a device
  */
 errcode_t ext2fs_get_device_phys_sectsize(const char *file, int *sectsize)
diff --git a/lib/ext2fs/mmp.c b/lib/ext2fs/mmp.c
index 49a11da..bb3772d 100644
--- a/lib/ext2fs/mmp.c
+++ b/lib/ext2fs/mmp.c
@@ -27,20 +27,6 @@
 #include "ext2fs/ext2_fs.h"
 #include "ext2fs/ext2fs.h"
 
-static int mmp_pagesize(void)
-{
-#ifdef _SC_PAGESIZE
-	int sysval = sysconf(_SC_PAGESIZE);
-	if (sysval > 0)
-		return sysval;
-#endif /* _SC_PAGESIZE */
-#ifdef HAVE_GETPAGESIZE
-	return getpagesize();
-#else
-	return 4096;
-#endif
-}
-
 #ifndef O_DIRECT
 #define O_DIRECT 0
 #endif
@@ -54,20 +40,6 @@ errcode_t ext2fs_mmp_read(ext2_filsys fs, blk64_t mmp_blk, void *buf)
 	    (mmp_blk >= fs->super->s_blocks_count))
 		return EXT2_ET_MMP_BAD_BLOCK;
 
-	if (fs->mmp_cmp == NULL) {
-		/* O_DIRECT in linux 2.4: page aligned
-		 * O_DIRECT in linux 2.6: sector aligned
-		 * A filesystem cannot be created with blocksize < sector size,
-		 * or with blocksize > page_size. */
-		int bufsize = fs->blocksize;
-
-		if (bufsize < mmp_pagesize())
-			bufsize = mmp_pagesize();
-		retval = ext2fs_get_memalign(bufsize, bufsize, &fs->mmp_cmp);
-		if (retval)
-			return retval;
-	}
-
 	/* ext2fs_open() reserves fd0,1,2 to avoid stdio collision, so checking
 	 * mmp_fd <= 0 is OK to validate that the fd is valid.  This opens its
 	 * own fd to read the MMP block to ensure that it is using O_DIRECT,
@@ -81,6 +53,15 @@ errcode_t ext2fs_mmp_read(ext2_filsys fs, blk64_t mmp_blk, void *buf)
 		}
 	}
 
+	if (fs->mmp_cmp == NULL) {
+		int align = ext2fs_get_dio_alignment(fs->mmp_fd);
+
+		retval = ext2fs_get_memalign(fs->blocksize, align,
+					     &fs->mmp_cmp);
+		if (retval)
+			return retval;
+	}
+
 	if (ext2fs_llseek(fs->mmp_fd, mmp_blk * fs->blocksize, SEEK_SET) !=
 	    mmp_blk * fs->blocksize) {
 		retval = EXT2_ET_LLSEEK_FAILED;
diff --git a/lib/ext2fs/unix_io.c b/lib/ext2fs/unix_io.c
index 3269392..8319eba 100644
--- a/lib/ext2fs/unix_io.c
+++ b/lib/ext2fs/unix_io.c
@@ -58,10 +58,6 @@
 #define BLKROGET   _IO(0x12, 94) /* Get read-only status (0 = read_write).  */
 #endif
 
-#if defined(__linux__) && defined(_IO) && !defined(BLKSSZGET)
-#define BLKSSZGET  _IO(0x12,104)/* get block device sector size */
-#endif
-
 #undef ALIGN_DEBUG
 
 #include "ext2_fs.h"
@@ -485,11 +481,15 @@ static errcode_t unix_open(const char *name, int flags, io_channel *channel)
 	if (flags & IO_FLAG_EXCLUSIVE)
 		open_flags |= O_EXCL;
 #if defined(O_DIRECT)
-	if (flags & IO_FLAG_DIRECT_IO)
+	if (flags & IO_FLAG_DIRECT_IO) {
 		open_flags |= O_DIRECT;
+		io->align = ext2fs_get_dio_alignment(data->dev);
+	}
 #elif defined(F_NOCACHE)
-	if (flags & IO_FLAG_DIRECT_IO)
+	if (flags & IO_FLAG_DIRECT_IO) {
 		f_nocache = F_NOCACHE;
+		io->align = 4096;
+	}
 #endif
 	data->flags = flags;
 
@@ -519,13 +519,6 @@ static errcode_t unix_open(const char *name, int flags, io_channel *channel)
 			io->flags |= CHANNEL_FLAGS_DISCARD_ZEROES;
 	}
 
-#ifdef BLKSSZGET
-	if (flags & IO_FLAG_DIRECT_IO) {
-		if (ioctl(data->dev, BLKSSZGET, &io->align) != 0)
-			io->align = io->block_size;
-	}
-#endif

[PATCH 3/5] libext2fs: make read_bitmaps() more efficient when using direct I/O

[Theodore Ts'o]

Read in a full block for each allocation bitmap, to avoid using a
kernel bounce buffer when using direct I/O.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 lib/ext2fs/rw_bitmaps.c |   12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/lib/ext2fs/rw_bitmaps.c b/lib/ext2fs/rw_bitmaps.c
index 1d5f7b2..81e09c0 100644
--- a/lib/ext2fs/rw_bitmaps.c
+++ b/lib/ext2fs/rw_bitmaps.c
@@ -166,6 +166,9 @@ static errcode_t read_bitmaps(ext2_filsys fs, int do_inode, int do_block)
 
 	EXT2_CHECK_MAGIC(fs, EXT2_ET_MAGIC_EXT2FS_FILSYS);
 
+	if ((block_nbytes > fs->blocksize) || (inode_nbytes > fs->blocksize))
+		return EXT2_ET_CORRUPT_SUPERBLOCK;
+
 	fs->write_bitmaps = ext2fs_write_bitmaps;
 
 	if (EXT2_HAS_RO_COMPAT_FEATURE(fs->super,
@@ -186,7 +189,7 @@ static errcode_t read_bitmaps(ext2_filsys fs, int do_inode, int do_block)
 		if (do_image)
 			retval = ext2fs_get_mem(fs->blocksize, &block_bitmap);
 		else
-			retval = ext2fs_get_memalign((unsigned) block_nbytes,
+			retval = ext2fs_get_memalign(fs->blocksize,
 						     fs->blocksize,
 						     &block_bitmap);
 			
@@ -202,8 +205,7 @@ static errcode_t read_bitmaps(ext2_filsys fs, int do_inode, int do_block)
 		retval = ext2fs_allocate_inode_bitmap(fs, buf, &fs->inode_map);
 		if (retval)
 			goto cleanup;
-		retval = ext2fs_get_mem(do_image ? fs->blocksize :
-					(unsigned) inode_nbytes, &inode_bitmap);
+		retval = ext2fs_get_mem(fs->blocksize, &inode_bitmap);
 		if (retval)
 			goto cleanup;
 	} else
@@ -261,7 +263,7 @@ static errcode_t read_bitmaps(ext2_filsys fs, int do_inode, int do_block)
 				blk = 0;
 			if (blk) {
 				retval = io_channel_read_blk64(fs->io, blk,
-					     -block_nbytes, block_bitmap);
+							       1, block_bitmap);
 				if (retval) {
 					retval = EXT2_ET_BLOCK_BITMAP_READ;
 					goto cleanup;
@@ -283,7 +285,7 @@ static errcode_t read_bitmaps(ext2_filsys fs, int do_inode, int do_block)
 				blk = 0;
 			if (blk) {
 				retval = io_channel_read_blk64(fs->io, blk,
-					     -inode_nbytes, inode_bitmap);
+							       1, inode_bitmap);
 				if (retval) {
 					retval = EXT2_ET_INODE_BITMAP_READ;
 					goto cleanup;
-- 
1.7.10.rc3

[PATCH 4/5] libext2fs: factor out I/O buffer allocation

[Theodore Ts'o]

Create a new function, io_channel_alloc_buf() which allocates I/O
buffers with appropriate alignment if we are using direct I/O.  The
original code was sometimes using a larger alignment factor than
necessary, and would always request an aligned memory buffer even when
it was not necessary since the block device was not opened with
O_DIRECT.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 lib/ext2fs/ext2_io.h    |    2 ++
 lib/ext2fs/inode.c      |    4 ++--
 lib/ext2fs/io_manager.c |   17 +++++++++++++++++
 lib/ext2fs/openfs.c     |    2 +-
 lib/ext2fs/rw_bitmaps.c |   16 ++++------------
 lib/ext2fs/unix_io.c    |    7 ++-----
 6 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/lib/ext2fs/ext2_io.h b/lib/ext2fs/ext2_io.h
index 95b8de3..1894fb8 100644
--- a/lib/ext2fs/ext2_io.h
+++ b/lib/ext2fs/ext2_io.h
@@ -122,6 +122,8 @@ extern errcode_t io_channel_write_blk64(io_channel channel,
 extern errcode_t io_channel_discard(io_channel channel,
 				    unsigned long long block,
 				    unsigned long long count);
+extern errcode_t io_channel_alloc_buf(io_channel channel,
+				      int count, void *ptr);
 
 /* unix_io.c */
 extern io_manager unix_io_manager;
diff --git a/lib/ext2fs/inode.c b/lib/ext2fs/inode.c
index 6c524ff..77fc447 100644
--- a/lib/ext2fs/inode.c
+++ b/lib/ext2fs/inode.c
@@ -157,8 +157,8 @@ errcode_t ext2fs_open_inode_scan(ext2_filsys fs, int buffer_blocks,
 			 (fs->blocksize / scan->inode_size - 1)) *
 			scan->inode_size / fs->blocksize;
 	}
-	retval = ext2fs_get_memalign(scan->inode_buffer_blocks * fs->blocksize,
-				     fs->blocksize, &scan->inode_buffer);
+	retval = io_channel_alloc_buf(fs->io, scan->inode_buffer_blocks,
+				      &scan->inode_buffer);
 	scan->done_group = 0;
 	scan->done_group_data = 0;
 	scan->bad_block_ptr = 0;
diff --git a/lib/ext2fs/io_manager.c b/lib/ext2fs/io_manager.c
index 25df59e..34e4859 100644
--- a/lib/ext2fs/io_manager.c
+++ b/lib/ext2fs/io_manager.c
@@ -111,3 +111,20 @@ errcode_t io_channel_discard(io_channel channel, unsigned long long block,
 
 	return EXT2_ET_UNIMPLEMENTED;
 }
+
+errcode_t io_channel_alloc_buf(io_channel io, int count, void *ptr)
+{
+	size_t	size;
+
+	if (count == 0)
+		size = io->block_size;
+	else if (count > 0)
+		size = io->block_size * count;
+	else
+		size = -count;
+
+	if (io->align)
+		return ext2fs_get_memalign(size, io->align, ptr);
+	else
+		return ext2fs_get_mem(size, ptr);
+}
diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
index 220d954..482e4ab 100644
--- a/lib/ext2fs/openfs.c
+++ b/lib/ext2fs/openfs.c
@@ -144,7 +144,7 @@ errcode_t ext2fs_open2(const char *name, const char *io_options,
 		goto cleanup;
 	fs->image_io = fs->io;
 	fs->io->app_data = fs;
-	retval = ext2fs_get_memalign(SUPERBLOCK_SIZE, 512, &fs->super);
+	retval = io_channel_alloc_buf(fs->io, -SUPERBLOCK_SIZE, &fs->super);
 	if (retval)
 		goto cleanup;
 	if (flags & EXT2_FLAG_IMAGE_FILE) {
diff --git a/lib/ext2fs/rw_bitmaps.c b/lib/ext2fs/rw_bitmaps.c
index 81e09c0..53f6ec4 100644
--- a/lib/ext2fs/rw_bitmaps.c
+++ b/lib/ext2fs/rw_bitmaps.c
@@ -53,8 +53,7 @@ static errcode_t write_bitmaps(ext2_filsys fs, int do_inode, int do_block)
 	inode_nbytes = block_nbytes = 0;
 	if (do_block) {
 		block_nbytes = EXT2_CLUSTERS_PER_GROUP(fs->super) / 8;
-		retval = ext2fs_get_memalign(fs->blocksize, fs->blocksize,
-					     &block_buf);
+		retval = io_channel_alloc_buf(fs->io, 0, &block_buf);
 		if (retval)
 			goto errout;
 		memset(block_buf, 0xff, fs->blocksize);
@@ -62,8 +61,7 @@ static errcode_t write_bitmaps(ext2_filsys fs, int do_inode, int do_block)
 	if (do_inode) {
 		inode_nbytes = (size_t)
 			((EXT2_INODES_PER_GROUP(fs->super)+7) / 8);
-		retval = ext2fs_get_memalign(fs->blocksize, fs->blocksize,
-					     &inode_buf);
+		retval = io_channel_alloc_buf(fs->io, 0, &inode_buf);
 		if (retval)
 			goto errout;
 		memset(inode_buf, 0xff, fs->blocksize);
@@ -186,13 +184,7 @@ static errcode_t read_bitmaps(ext2_filsys fs, int do_inode, int do_block)
 		retval = ext2fs_allocate_block_bitmap(fs, buf, &fs->block_map);
 		if (retval)
 			goto cleanup;
-		if (do_image)
-			retval = ext2fs_get_mem(fs->blocksize, &block_bitmap);
-		else
-			retval = ext2fs_get_memalign(fs->blocksize,
-						     fs->blocksize,
-						     &block_bitmap);
-			
+		retval = io_channel_alloc_buf(fs->io, 0, &block_bitmap);
 		if (retval)
 			goto cleanup;
 	} else
@@ -205,7 +197,7 @@ static errcode_t read_bitmaps(ext2_filsys fs, int do_inode, int do_block)
 		retval = ext2fs_allocate_inode_bitmap(fs, buf, &fs->inode_map);
 		if (retval)
 			goto cleanup;
-		retval = ext2fs_get_mem(fs->blocksize, &inode_bitmap);
+		retval = io_channel_alloc_buf(fs->io, 0, &inode_bitmap);
 		if (retval)
 			goto cleanup;
 	} else
diff --git a/lib/ext2fs/unix_io.c b/lib/ext2fs/unix_io.c
index 8319eba..2ce0563 100644
--- a/lib/ext2fs/unix_io.c
+++ b/lib/ext2fs/unix_io.c
@@ -316,17 +316,14 @@ static errcode_t alloc_cache(io_channel channel,
 		cache->in_use = 0;
 		if (cache->buf)
 			ext2fs_free_mem(&cache->buf);
-		retval = ext2fs_get_memalign(channel->block_size,
-					     channel->align, &cache->buf);
+		retval = io_channel_alloc_buf(channel, 0, &cache->buf);
 		if (retval)
 			return retval;
 	}
 	if (channel->align) {
 		if (data->bounce)
 			ext2fs_free_mem(&data->bounce);
-		retval = ext2fs_get_memalign(channel->block_size,
-					     channel->align,
-					     &data->bounce);
+		retval = io_channel_alloc_buf(channel, 0, &data->bounce);
 	}
 	return retval;
 }
-- 
1.7.10.rc3

[PATCH 5/5] Support systems without posix_memalign() and memalign()

[Theodore Ts'o]

MacOS 10.5 doesn't have posix_memalign() nor memalign(), but it does
have valloc().  The Android SDK would like to be built on MacOS 10.5,
so I've added support for a good-enough emulation of memalign()'s
functionality using valloc(), with an explicit test to make sure
valloc() is returning a pointer which is sufficiently aligned given
the requested alignment.  This won't work if you try to operate on a
file system with a 16k blocksize using an e2fsprogs built on MacOS
10.5 system, but it is good enough for the common case of 4k
blocksize file systems, and we will let the memory allocation fail in
the alignment is not good enough.

I've also added a unit test for ext2fs_get_memalign() so we can be
sure it's working as expected.  I've tested the code paths with
HAVE_POSIX_MEMALIGN defined, HAVE_POSIX_MEMALIGN undefined, and
HAVE_POSIX_MEMALIGN and HAVE_MEMALIGN undefined on an x86 Linux
system, and so I know the valloc() code path works OK.  The simplistic
(and less safe) patch at:

https://trac.macports.org/attachment/ticket/33692/patch-lib-ext2fs-inline.c.diff

Shows that using valloc() apparently works OK for MacOS 10.5 (but if
it doesn't the unit test will catch a problem).

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 lib/ext2fs/Makefile.in |    9 +++++++-
 lib/ext2fs/inline.c    |   56 ++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/lib/ext2fs/Makefile.in b/lib/ext2fs/Makefile.in
index 507a459..f9200fa 100644
--- a/lib/ext2fs/Makefile.in
+++ b/lib/ext2fs/Makefile.in
@@ -369,6 +369,11 @@ tst_extents: $(srcdir)/extent.c extent_dbg.c $(DEBUG_OBJS) $(DEPLIBSS) \
 		$(STATIC_LIBEXT2FS) $(LIBBLKID) $(LIBUUID) $(LIBCOM_ERR) \
 		-I $(top_srcdir)/debugfs
 
+tst_inline: $(srcdir)/inline.c $(STATIC_LIBEXT2FS) $(DEPLIBCOM_ERR)
+	$(E) "	LD $@"
+	$(Q) $(CC) -o tst_inline $(srcdir)/inline.c $(ALL_CFLAGS) -DDEBUG \
+		$(STATIC_LIBEXT2FS) $(LIBCOM_ERR)
+
 tst_csum: csum.c $(STATIC_LIBEXT2FS) $(DEPLIBCOM_ERR) \
 		$(top_srcdir)/lib/e2p/e2p.h
 	$(E) "	LD $@"
@@ -384,7 +389,8 @@ mkjournal: mkjournal.c $(STATIC_LIBEXT2FS) $(DEPLIBCOM_ERR)
 	$(Q) $(CC) -o mkjournal $(srcdir)/mkjournal.c -DDEBUG $(STATIC_LIBEXT2FS) $(LIBCOM_ERR) $(ALL_CFLAGS)
 
 check:: tst_bitops tst_badblocks tst_iscan tst_types tst_icount \
-    tst_super_size tst_types tst_inode_size tst_csum tst_crc32c tst_bitmaps
+    tst_super_size tst_types tst_inode_size tst_csum tst_crc32c tst_bitmaps \
+    tst_inline
 	LD_LIBRARY_PATH=$(LIB) DYLD_LIBRARY_PATH=$(LIB) ./tst_bitops
 	LD_LIBRARY_PATH=$(LIB) DYLD_LIBRARY_PATH=$(LIB) ./tst_badblocks
 	LD_LIBRARY_PATH=$(LIB) DYLD_LIBRARY_PATH=$(LIB) ./tst_iscan
@@ -393,6 +399,7 @@ check:: tst_bitops tst_badblocks tst_iscan tst_types tst_icount \
 	LD_LIBRARY_PATH=$(LIB) DYLD_LIBRARY_PATH=$(LIB) ./tst_super_size
 	LD_LIBRARY_PATH=$(LIB) DYLD_LIBRARY_PATH=$(LIB) ./tst_inode_size
 	LD_LIBRARY_PATH=$(LIB) DYLD_LIBRARY_PATH=$(LIB) ./tst_csum
+	LD_LIBRARY_PATH=$(LIB) DYLD_LIBRARY_PATH=$(LIB) ./tst_inline
 	LD_LIBRARY_PATH=$(LIB) DYLD_LIBRARY_PATH=$(LIB) ./tst_crc32c
 	LD_LIBRARY_PATH=$(LIB) DYLD_LIBRARY_PATH=$(LIB) \
 		./tst_bitmaps -f $(srcdir)/tst_bitmaps_cmds > tst_bitmaps_out
diff --git a/lib/ext2fs/inline.c b/lib/ext2fs/inline.c
index ad0c368..335db55 100644
--- a/lib/ext2fs/inline.c
+++ b/lib/ext2fs/inline.c
@@ -45,7 +45,7 @@ errcode_t ext2fs_get_memalign(unsigned long size,
 	errcode_t retval;
 	void **p = ptr;
 
-	if (align == 0)
+	if (align < 8)
 		align = 8;
 #ifdef HAVE_POSIX_MEMALIGN
 	retval = posix_memalign(p, align, size);
@@ -64,9 +64,61 @@ errcode_t ext2fs_get_memalign(unsigned long size,
 			return EXT2_ET_NO_MEMORY;
 	}
 #else
-#error memalign or posix_memalign must be defined!
+#ifdef HAVE_VALLOC
+	if (align > sizeof(long long))
+		*p = valloc(size);
+	else
+#endif
+		*p = malloc(size);
+	if ((unsigned long) *p & (align - 1)) {
+		free(*p);
+		*p = 0;
+	}
+	if (*p == 0)
+		return EXT2_ET_NO_MEMORY;
 #endif
 #endif
 	return 0;
 }
 
+#ifdef DEBUG
+static int isaligned(void *ptr, unsigned long align)
+{
+	return (((unsigned long) ptr & (align - 1)) == 0);
+}
+
+static errcode_t test_memalign(unsigned long align)
+{
+	void *ptr = 0;
+	errcode_t retval;
+
+	retval = ext2fs_get_memalign(32, align, &ptr);
+	if (!retval && !isaligned(ptr, align))
+		retval = EINVAL;
+	free(ptr);
+	printf("tst_memliagn(%lu): %s\n", align, 
+	       retval ? error_message(retval) : "OK");
+	return retval;
+}
+
+int main(int argc, char **argv)
+{
+	int err = 0;
+
+	if (test_memalign(4))
+		err++;
+	if (test_memalign(32))
+		err++;
+	if (test_memalign(1024))
+		err++;
+	if (test_memalign(4096))
+		err++;
+#if defined(HAVE_POSIX_MEMALIGN) || defined(HAVE_MEMALIGN)
+	if (test_memalign(16384))
+		err++;
+	if (test_memalign(32768))
+		err++;
+#endif
+	return err;
+}
+#endif
-- 
1.7.10.rc3