linux-ext4 - RE: [PATCH 01/24] e4defrag: backwards-allocated files should be defragmented too

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 21 Jul 2014 23:44:59 +0000
From:	Akira Fujita <a-fujita@...jp.nec.com>
To:	"Darrick J. Wong" <darrick.wong@...cle.com>,
	"tytso@....edu" <tytso@....edu>
CC:	Xiaoguang Wang <wangxg.fnst@...fujitsu.com>,
	"linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>
Subject: RE: [PATCH 01/24] e4defrag: backwards-allocated files should be
 defragmented too

>-----Original Message-----
>From: linux-ext4-owner@...r.kernel.org [mailto:linux-ext4-owner@...r.kernel.org] On Behalf Of
>Darrick J. Wong
>Sent: Saturday, July 19, 2014 7:52 AM
>To: tytso@....edu; darrick.wong@...cle.com
>Cc: Xiaoguang Wang; linux-ext4@...r.kernel.org
>Subject: [PATCH 01/24] e4defrag: backwards-allocated files should be defragmented too
>
>Currently, e4defrag avoids increasing file fragmentation by comparing
>the number of runs of physical extents of both the original and the
>donor files.  Unfortunately, there is a bug in the routine that counts
>physical extents, since it doesn't look at the logical block offsets
>of the extents.  Therefore, a file whose blocks were allocated in
>reverse order will be seen as only having one big physical extent, and
>therefore will not be defragmented.
>
>Fix the counting routine to consider logical extent offset so that we
>defragment backwards-allocated files.  This could be problematic if we
>ever gain the ability to lay out logically sparse extents in a
>physically contiguous manner, but presumably one wouldn't call defrag
>on such a file.
>
>Reported-by: Xiaoguang Wang <wangxg.fnst@...fujitsu.com>
>Signed-off-by: Darrick J. Wong <darrick.wong@...cle.com>
>---
> misc/e4defrag.c |    4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>
>diff --git a/misc/e4defrag.c b/misc/e4defrag.c
>index a204793..d0eac60 100644
>--- a/misc/e4defrag.c
>+++ b/misc/e4defrag.c
>@@ -888,7 +888,9 @@ static int get_physical_count(struct fiemap_extent_list *physical_list_head)
>
> 	do {
> 		if ((ext_list_tmp->data.physical + ext_list_tmp->data.len)
>-				!= ext_list_tmp->next->data.physical) {
>+				!= ext_list_tmp->next->data.physical ||
>+		    (ext_list_tmp->data.logical + ext_list_tmp->data.len)
>+				!= ext_list_tmp->next->data.logical) {
> 			/* This extent and next extent are not continuous. */
> 			ret++;
> 		}

It looks good to me.

Reviewed-by: Akira Fujita <a-fujita@...jp.nec.com>

Thanks,
Akira Fujita