linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] common/defrag: rework _require_defrag for ext4
@ 2016-08-04 20:02 Eric Whitney
  2016-08-11 17:48 ` Christoph Hellwig
  0 siblings, 1 reply; 2+ messages in thread
From: Eric Whitney @ 2016-08-04 20:02 UTC (permalink / raw)
  To: fstests; +Cc: linux-ext4, tytso

The existing _require_defrag function rejects ext4 file systems
mounted with the dax option or constructed without extents.  However,
there are also a number of other ext4 file system configuration cases
that do not currently support online defrag, including encryption, data
journaling, and bigalloc.  In the future, online defrag functionality
may be implemented for some of these, and new configuration cases
could be added that don't support it.

Rather than add a laundry list of mount and file system configuration
options to the existing _require_defrag function that will need ongoing
maintenance, use the available src/e4compact program to directly
determine whether the kernel supports ext4's move extent ioctl on the
test file system.

Signed-off-by: Eric Whitney <enwlinux@gmail.com>
---
 common/defrag | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/common/defrag b/common/defrag
index 7db0cb4..986b4bf 100644
--- a/common/defrag
+++ b/common/defrag
@@ -27,14 +27,19 @@ _require_defrag()
         DEFRAG_PROG="$XFS_FSR_PROG"
 	;;
     ext4|ext4dev)
-	echo $MOUNT_OPTIONS | grep -q dax
+	testfile="$TEST_DIR/$$-test.defrag"
+	donorfile="$TEST_DIR/$$-donor.defrag"
+	bsize=`get_block_size $TEST_DIR`
+	$XFS_IO_PROG -f -c "pwrite -b $bsize 0 $bsize" $testfile > /dev/null
+	cp $testfile $donorfile
+	echo $testfile | $here/src/e4compact -v -f $donorfile | \
+		grep -q "err:95"
 	if [ $? -eq 0 ]; then
-		_notrun "$FSTYP defragmentation not supported with DAX"
-	else
-		DEFRAG_PROG="$E4DEFRAG_PROG"
+		rm -f $testfile $donorfile 2>&1 > /dev/null
+		_notrun "$FSTYP test filesystem doesn't support online defrag"
 	fi
-	dumpe2fs -h $TEST_DEV 2> /dev/null | grep -wq extent || \
-		_notrun "file system does not have extents, needed for defrag"
+	rm -f $testfile $donorfile 2>&1 > /dev/null
+	DEFRAG_PROG="$E4DEFRAG_PROG"
 	;;
     btrfs)
 	DEFRAG_PROG="$BTRFS_UTIL_PROG filesystem defragment"
-- 
2.1.4

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

* Re: [PATCH] common/defrag: rework _require_defrag for ext4
  2016-08-04 20:02 [PATCH] common/defrag: rework _require_defrag for ext4 Eric Whitney
@ 2016-08-11 17:48 ` Christoph Hellwig
  0 siblings, 0 replies; 2+ messages in thread
From: Christoph Hellwig @ 2016-08-11 17:48 UTC (permalink / raw)
  To: Eric Whitney; +Cc: fstests, linux-ext4, tytso

On Thu, Aug 04, 2016 at 04:02:46PM -0400, Eric Whitney wrote:
> The existing _require_defrag function rejects ext4 file systems
> mounted with the dax option or constructed without extents.  However,
> there are also a number of other ext4 file system configuration cases
> that do not currently support online defrag, including encryption, data
> journaling, and bigalloc.  In the future, online defrag functionality
> may be implemented for some of these, and new configuration cases
> could be added that don't support it.
> 
> Rather than add a laundry list of mount and file system configuration
> options to the existing _require_defrag function that will need ongoing
> maintenance, use the available src/e4compact program to directly
> determine whether the kernel supports ext4's move extent ioctl on the
> test file system.

This look reasonable - but I wonder if we could just come up with an
even more generic version that uses your approach on all file systems,
so that the only per-fs logic left would be to apply the
DEFRAG_PROG variable.

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

end of thread, other threads:[~2016-08-11 17:48 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-04 20:02 [PATCH] common/defrag: rework _require_defrag for ext4 Eric Whitney
2016-08-11 17:48 ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).