[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.LFD.2.00.1105101620590.18016@dhcp-27-109.brq.redhat.com>
Date: Tue, 10 May 2011 16:35:37 +0200 (CEST)
From: Lukas Czerner <lczerner@...hat.com>
To: Yongqiang Yang <xiaoqiangnk@...il.com>
cc: linux-ext4@...r.kernel.org, tytso@....edu
Subject: Re: [PATCH v2] ext4:Let ext4_ext_fiemap_cb() handle blocks before
request range correctly.
On Tue, 10 May 2011, Yongqiang Yang wrote:
> v1->v2:
> Add more specific description.
>
> To get delayed-extent information, ext4_ext_fiemap_cb() lookups pagecache,
> it thus collects information starting from a page's head block.
> If blockszie < pagesize, the beginning blocks of a page may lie before the
> range. ext4_ext_fiemap_cb() should proceed ignoring them, because they
> has been handled before.
Thanks for the description, but I have one question below.
>
> Reported-by: Amir Goldstein <amir73il@...rs.sf.net>
> Signed-off-by: Yongqiang Yang <xiaoqiangnk@...il.com>
> ---
> fs/ext4/extents.c | 7 ++++++-
> 1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index e363f21..ec37109 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -3718,9 +3718,14 @@ out:
>
> bh = head;
> do {
> + if (end < newex->ec_block)
> + /* The buffer is not in
> + * the request range.
> + */
> + continue;
So if the condition is true, then we might leave the loop, because (bh ==
head) in the first iteration, is that desired behavior? Also if we hit
this condition in other than first iteration we will spin forever. I do
not think this is right. How did you tested this case ?
Thanks!
-Lukas
> if (buffer_mapped(bh)) {
> /* get the 1st mapped buffer. */
> - if (end > newex->ec_block +
> + if (end >= newex->ec_block +
> newex->ec_len)
> /* The buffer is out of
> * the request range.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists