[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <28989C0C9F1C0A428470D967B5FCED372B3917@BPXM22GP.gisp.nec.co.jp>
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
Powered by blists - more mailing lists