[1/2] btrfs-progs: Fix false alert about EXTENT_DATA shouldn't be hole
diff mbox

Message ID 20170616014323.13170-1-quwenruo@cn.fujitsu.com
State New
Headers show

Commit Message

Qu Wenruo June 16, 2017, 1:43 a.m. UTC
Since incompat feature NO_HOLES still allow us to have explicit hole
file extent, current check is too restrict and will cause false alert
like:

root 5 EXTENT_DATA[257, 0] shouldn't be hole

Fix it by removing the restrict hole file extent check.

Reported-by: Henk Slager <eye1tm@gmail.com>
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 cmds-check.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Comments

Henk Slager June 19, 2017, 11:26 a.m. UTC | #1
On 16-06-17 03:43, Qu Wenruo wrote:
> Since incompat feature NO_HOLES still allow us to have explicit hole
> file extent, current check is too restrict and will cause false alert
> like:
>
> root 5 EXTENT_DATA[257, 0] shouldn't be hole
>
> Fix it by removing the restrict hole file extent check.
>
> Reported-by: Henk Slager <eye1tm@gmail.com>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> ---
>  cmds-check.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/cmds-check.c b/cmds-check.c
> index c052f66e..7bd57677 100644
> --- a/cmds-check.c
> +++ b/cmds-check.c
> @@ -4841,11 +4841,7 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
>  	}
>  
>  	/* Check EXTENT_DATA hole */
> -	if (no_holes && is_hole) {
> -		err |= FILE_EXTENT_ERROR;
> -		error("root %llu EXTENT_DATA[%llu %llu] shouldn't be hole",
> -		      root->objectid, fkey->objectid, fkey->offset);
> -	} else if (!no_holes && *end != fkey->offset) {
> +	if (!no_holes && *end != fkey->offset) {
>  		err |= FILE_EXTENT_ERROR;
>  		error("root %llu EXTENT_DATA[%llu %llu] interrupt",
>  		      root->objectid, fkey->objectid, fkey->offset);


Thanks for the patch, I applied it on v4.11 btrfs-progs and re-ran the check:
# btrfs check -p --readonly /dev/mapper/smr

on filesystem mentioned in:
https://www.spinics.net/lists/linux-btrfs/msg66374.html

and now the "shouldn't be hole" errors don't show up anymore.

Tested-by: Henk Slager <eye1tm@gmail.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Sterba June 26, 2017, 2:36 p.m. UTC | #2
On Mon, Jun 19, 2017 at 01:26:20PM +0200, Henk Slager wrote:
> On 16-06-17 03:43, Qu Wenruo wrote:
> > Since incompat feature NO_HOLES still allow us to have explicit hole
> > file extent, current check is too restrict and will cause false alert
> > like:
> >
> > root 5 EXTENT_DATA[257, 0] shouldn't be hole
> >
> > Fix it by removing the restrict hole file extent check.
> >
> > Reported-by: Henk Slager <eye1tm@gmail.com>
> > Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> > ---
> >  cmds-check.c | 6 +-----
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/cmds-check.c b/cmds-check.c
> > index c052f66e..7bd57677 100644
> > --- a/cmds-check.c
> > +++ b/cmds-check.c
> > @@ -4841,11 +4841,7 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
> >  	}
> >  
> >  	/* Check EXTENT_DATA hole */
> > -	if (no_holes && is_hole) {
> > -		err |= FILE_EXTENT_ERROR;
> > -		error("root %llu EXTENT_DATA[%llu %llu] shouldn't be hole",
> > -		      root->objectid, fkey->objectid, fkey->offset);
> > -	} else if (!no_holes && *end != fkey->offset) {
> > +	if (!no_holes && *end != fkey->offset) {
> >  		err |= FILE_EXTENT_ERROR;
> >  		error("root %llu EXTENT_DATA[%llu %llu] interrupt",
> >  		      root->objectid, fkey->objectid, fkey->offset);
> 
> 
> Thanks for the patch, I applied it on v4.11 btrfs-progs and re-ran the check:
> # btrfs check -p --readonly /dev/mapper/smr
> 
> on filesystem mentioned in:
> https://www.spinics.net/lists/linux-btrfs/msg66374.html
> 
> and now the "shouldn't be hole" errors don't show up anymore.
> 
> Tested-by: Henk Slager <eye1tm@gmail.com>

Thank you both, patch applied. I might also release a 4.11.x release
with this fix included.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Henk Slager July 3, 2017, 9:50 a.m. UTC | #3
On Mon, Jun 19, 2017 at 1:26 PM, Henk Slager <eye1tm@gmail.com> wrote:
> On 16-06-17 03:43, Qu Wenruo wrote:
>> Since incompat feature NO_HOLES still allow us to have explicit hole
>> file extent, current check is too restrict and will cause false alert
>> like:
>>
>> root 5 EXTENT_DATA[257, 0] shouldn't be hole
>>
>> Fix it by removing the restrict hole file extent check.
>>
>> Reported-by: Henk Slager <eye1tm@gmail.com>
>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>> ---
>>  cmds-check.c | 6 +-----
>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/cmds-check.c b/cmds-check.c
>> index c052f66e..7bd57677 100644
>> --- a/cmds-check.c
>> +++ b/cmds-check.c
>> @@ -4841,11 +4841,7 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
>>       }
>>
>>       /* Check EXTENT_DATA hole */
>> -     if (no_holes && is_hole) {
>> -             err |= FILE_EXTENT_ERROR;
>> -             error("root %llu EXTENT_DATA[%llu %llu] shouldn't be hole",
>> -                   root->objectid, fkey->objectid, fkey->offset);
>> -     } else if (!no_holes && *end != fkey->offset) {
>> +     if (!no_holes && *end != fkey->offset) {
>>               err |= FILE_EXTENT_ERROR;
>>               error("root %llu EXTENT_DATA[%llu %llu] interrupt",
>>                     root->objectid, fkey->objectid, fkey->offset);
>
>
> Thanks for the patch, I applied it on v4.11 btrfs-progs and re-ran the check:
> # btrfs check -p --readonly /dev/mapper/smr
While looking at new kernel+progs releases  and my script logs, I see
I made copy-paste error; of course it is the lowmem mode:
# btrfs check -p --mode lowmem --readonly /dev/mapper/smr
> on filesystem mentioned in:
> https://www.spinics.net/lists/linux-btrfs/msg66374.html
>
> and now the "shouldn't be hole" errors don't show up anymore.
>
> Tested-by: Henk Slager <eye1tm@gmail.com>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/cmds-check.c b/cmds-check.c
index c052f66e..7bd57677 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -4841,11 +4841,7 @@  static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
 	}
 
 	/* Check EXTENT_DATA hole */
-	if (no_holes && is_hole) {
-		err |= FILE_EXTENT_ERROR;
-		error("root %llu EXTENT_DATA[%llu %llu] shouldn't be hole",
-		      root->objectid, fkey->objectid, fkey->offset);
-	} else if (!no_holes && *end != fkey->offset) {
+	if (!no_holes && *end != fkey->offset) {
 		err |= FILE_EXTENT_ERROR;
 		error("root %llu EXTENT_DATA[%llu %llu] interrupt",
 		      root->objectid, fkey->objectid, fkey->offset);