* [PATCH] common/btrfs: handle dmdust as mounted device in _btrfs_buffered_read_on_mirror()
@ 2023-06-26 6:00 Qu Wenruo
2023-06-26 16:36 ` Filipe Manana
2023-06-26 17:32 ` Zorro Lang
0 siblings, 2 replies; 7+ messages in thread
From: Qu Wenruo @ 2023-06-26 6:00 UTC (permalink / raw)
To: linux-btrfs, fstests; +Cc: Filipe Manana
[BUG]
After commit ab41f0bddb73 ("common/btrfs: use _scratch_cycle_mount to
ensure all page caches are dropped"), the test case btrfs/143 can fail
like below:
btrfs/143 6s ... [failed, exit status 1]- output mismatch (see ~/xfstests/results//btrfs/143.out.bad)
--- tests/btrfs/143.out 2020-06-10 19:29:03.818519162 +0100
+++ ~/xfstests/results//btrfs/143.out.bad 2023-06-19 17:04:00.575033899 +0100
@@ -1,37 +1,6 @@
QA output created by 143
wrote 131072/131072 bytes
XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
................
-XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
................
-XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
................
-XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
................
[CAUSE]
Test case btrfs/143 uses dm-dust device to emulate read errors, this
means we can not use _scratch_cycle_mount to cycle mount $SCRATCH_MNT.
As it would go mount $SCRATCH_DEV, not the dm-dust device to
$SCRATCH_MNT.
This prevents us to trigger read-repair (since no error would be hit)
thus fail the test.
[FIX]
Since we can mount whatever device at $SCRATCH_MNT, we can not use
_scratch_cycle_mount in this case.
Instead implement a small helper to grab the mounted device and its
mount options, and use the same device and mount options to cycle
$SCRATCH_MNT mount.
This would fix btrfs/143 and hopefully future test cases which use dm
devices.
Reported-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
common/btrfs | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/common/btrfs b/common/btrfs
index 175b33ae..4a02b2cc 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -601,8 +601,18 @@ _btrfs_buffered_read_on_mirror()
# The drop_caches doesn't seem to drop every pages on aarch64 with
# 64K page size.
# So here as another workaround, cycle mount the SCRATCH_MNT to ensure
- # the cache are dropped.
- _scratch_cycle_mount
+ # the cache are dropped, but we can not use _scratch_cycle_mount, as
+ # we may mount whatever dm device at SCRATCH_MNT.
+ # So here we grab the mounted block device and its mount options, then
+ # unmount and re-mount with the same device and options.
+ local mount_info=$(_mount | grep "$SCRATCH_MNT")
+ if [ -z "$mount_info" ]; then
+ _fail "failed to grab mount info of $SCRATCH_MNT"
+ fi
+ local dev=$(echo $mount_info | $AWK_PROG '{print $1}')
+ local opts=$(echo $mount_info | $AWK_PROG '{print $6}' | sed 's/[()]//g')
+ _scratch_unmount
+ _mount $dev -o $opts $SCRATCH_MNT
while [[ -z $( (( BASHPID % nr_mirrors == mirror )) &&
exec $XFS_IO_PROG \
-c "pread -b $size $offset $size" $file) ]]; do
--
2.39.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] common/btrfs: handle dmdust as mounted device in _btrfs_buffered_read_on_mirror()
2023-06-26 6:00 [PATCH] common/btrfs: handle dmdust as mounted device in _btrfs_buffered_read_on_mirror() Qu Wenruo
@ 2023-06-26 16:36 ` Filipe Manana
2023-06-26 17:32 ` Zorro Lang
1 sibling, 0 replies; 7+ messages in thread
From: Filipe Manana @ 2023-06-26 16:36 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, fstests
On Mon, Jun 26, 2023 at 7:05 AM Qu Wenruo <wqu@suse.com> wrote:
>
> [BUG]
> After commit ab41f0bddb73 ("common/btrfs: use _scratch_cycle_mount to
> ensure all page caches are dropped"), the test case btrfs/143 can fail
> like below:
>
> btrfs/143 6s ... [failed, exit status 1]- output mismatch (see ~/xfstests/results//btrfs/143.out.bad)
> --- tests/btrfs/143.out 2020-06-10 19:29:03.818519162 +0100
> +++ ~/xfstests/results//btrfs/143.out.bad 2023-06-19 17:04:00.575033899 +0100
> @@ -1,37 +1,6 @@
> QA output created by 143
> wrote 131072/131072 bytes
> XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
> ................
> -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
> ................
> -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
> ................
> -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
> ................
>
> [CAUSE]
> Test case btrfs/143 uses dm-dust device to emulate read errors, this
> means we can not use _scratch_cycle_mount to cycle mount $SCRATCH_MNT.
>
> As it would go mount $SCRATCH_DEV, not the dm-dust device to
> $SCRATCH_MNT.
> This prevents us to trigger read-repair (since no error would be hit)
> thus fail the test.
>
> [FIX]
> Since we can mount whatever device at $SCRATCH_MNT, we can not use
> _scratch_cycle_mount in this case.
>
> Instead implement a small helper to grab the mounted device and its
> mount options, and use the same device and mount options to cycle
> $SCRATCH_MNT mount.
>
> This would fix btrfs/143 and hopefully future test cases which use dm
> devices.
>
> Reported-by: Filipe Manana <fdmanana@suse.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
Looks fine to me, and it works, so:
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Thanks.
> ---
> common/btrfs | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/common/btrfs b/common/btrfs
> index 175b33ae..4a02b2cc 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -601,8 +601,18 @@ _btrfs_buffered_read_on_mirror()
> # The drop_caches doesn't seem to drop every pages on aarch64 with
> # 64K page size.
> # So here as another workaround, cycle mount the SCRATCH_MNT to ensure
> - # the cache are dropped.
> - _scratch_cycle_mount
> + # the cache are dropped, but we can not use _scratch_cycle_mount, as
> + # we may mount whatever dm device at SCRATCH_MNT.
> + # So here we grab the mounted block device and its mount options, then
> + # unmount and re-mount with the same device and options.
> + local mount_info=$(_mount | grep "$SCRATCH_MNT")
> + if [ -z "$mount_info" ]; then
> + _fail "failed to grab mount info of $SCRATCH_MNT"
> + fi
> + local dev=$(echo $mount_info | $AWK_PROG '{print $1}')
> + local opts=$(echo $mount_info | $AWK_PROG '{print $6}' | sed 's/[()]//g')
> + _scratch_unmount
> + _mount $dev -o $opts $SCRATCH_MNT
> while [[ -z $( (( BASHPID % nr_mirrors == mirror )) &&
> exec $XFS_IO_PROG \
> -c "pread -b $size $offset $size" $file) ]]; do
> --
> 2.39.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] common/btrfs: handle dmdust as mounted device in _btrfs_buffered_read_on_mirror()
2023-06-26 6:00 [PATCH] common/btrfs: handle dmdust as mounted device in _btrfs_buffered_read_on_mirror() Qu Wenruo
2023-06-26 16:36 ` Filipe Manana
@ 2023-06-26 17:32 ` Zorro Lang
2023-06-26 21:23 ` Qu Wenruo
1 sibling, 1 reply; 7+ messages in thread
From: Zorro Lang @ 2023-06-26 17:32 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, fstests, Filipe Manana
On Mon, Jun 26, 2023 at 02:00:52PM +0800, Qu Wenruo wrote:
> [BUG]
> After commit ab41f0bddb73 ("common/btrfs: use _scratch_cycle_mount to
> ensure all page caches are dropped"), the test case btrfs/143 can fail
> like below:
>
> btrfs/143 6s ... [failed, exit status 1]- output mismatch (see ~/xfstests/results//btrfs/143.out.bad)
> --- tests/btrfs/143.out 2020-06-10 19:29:03.818519162 +0100
> +++ ~/xfstests/results//btrfs/143.out.bad 2023-06-19 17:04:00.575033899 +0100
> @@ -1,37 +1,6 @@
> QA output created by 143
> wrote 131072/131072 bytes
> XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
> ................
> -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
> ................
> -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
> ................
> -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
> ................
>
> [CAUSE]
> Test case btrfs/143 uses dm-dust device to emulate read errors, this
> means we can not use _scratch_cycle_mount to cycle mount $SCRATCH_MNT.
>
> As it would go mount $SCRATCH_DEV, not the dm-dust device to
> $SCRATCH_MNT.
> This prevents us to trigger read-repair (since no error would be hit)
> thus fail the test.
>
> [FIX]
> Since we can mount whatever device at $SCRATCH_MNT, we can not use
> _scratch_cycle_mount in this case.
>
> Instead implement a small helper to grab the mounted device and its
> mount options, and use the same device and mount options to cycle
> $SCRATCH_MNT mount.
>
> This would fix btrfs/143 and hopefully future test cases which use dm
> devices.
>
> Reported-by: Filipe Manana <fdmanana@suse.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> common/btrfs | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/common/btrfs b/common/btrfs
> index 175b33ae..4a02b2cc 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -601,8 +601,18 @@ _btrfs_buffered_read_on_mirror()
> # The drop_caches doesn't seem to drop every pages on aarch64 with
> # 64K page size.
> # So here as another workaround, cycle mount the SCRATCH_MNT to ensure
> - # the cache are dropped.
> - _scratch_cycle_mount
> + # the cache are dropped, but we can not use _scratch_cycle_mount, as
> + # we may mount whatever dm device at SCRATCH_MNT.
> + # So here we grab the mounted block device and its mount options, then
> + # unmount and re-mount with the same device and options.
> + local mount_info=$(_mount | grep "$SCRATCH_MNT")
> + if [ -z "$mount_info" ]; then
> + _fail "failed to grab mount info of $SCRATCH_MNT"
> + fi
> + local dev=$(echo $mount_info | $AWK_PROG '{print $1}')
> + local opts=$(echo $mount_info | $AWK_PROG '{print $6}' | sed 's/[()]//g')
The `findmnt` can help to get $dev and $opts:
local dev=$(findmnt -n -T $SCRATCH_MNT -o SOURCE)
local opts=$(findmnt -n -T $SCRATCH_MNT -o OPTIONS)
If you hope to check you can keep:
if [ -z "$dev" -o -z "$opts" ];then
_fail "failed to grab mount info of $SCRATCH_MNT"
fi
> + _scratch_unmount
> + _mount $dev -o $opts $SCRATCH_MNT
I'm wondering can this help that, after you get the "real" device name:
SCRATCH_DEV=$dev _scratch_cycle_mount
Thanks,
Zorro
> while [[ -z $( (( BASHPID % nr_mirrors == mirror )) &&
> exec $XFS_IO_PROG \
> -c "pread -b $size $offset $size" $file) ]]; do
> --
> 2.39.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] common/btrfs: handle dmdust as mounted device in _btrfs_buffered_read_on_mirror()
2023-06-26 17:32 ` Zorro Lang
@ 2023-06-26 21:23 ` Qu Wenruo
2023-06-28 11:34 ` Zorro Lang
0 siblings, 1 reply; 7+ messages in thread
From: Qu Wenruo @ 2023-06-26 21:23 UTC (permalink / raw)
To: Zorro Lang, Qu Wenruo; +Cc: linux-btrfs, fstests, Filipe Manana
On 2023/6/27 01:32, Zorro Lang wrote:
> On Mon, Jun 26, 2023 at 02:00:52PM +0800, Qu Wenruo wrote:
>> [BUG]
>> After commit ab41f0bddb73 ("common/btrfs: use _scratch_cycle_mount to
>> ensure all page caches are dropped"), the test case btrfs/143 can fail
>> like below:
>>
>> btrfs/143 6s ... [failed, exit status 1]- output mismatch (see ~/xfstests/results//btrfs/143.out.bad)
>> --- tests/btrfs/143.out 2020-06-10 19:29:03.818519162 +0100
>> +++ ~/xfstests/results//btrfs/143.out.bad 2023-06-19 17:04:00.575033899 +0100
>> @@ -1,37 +1,6 @@
>> QA output created by 143
>> wrote 131072/131072 bytes
>> XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
>> ................
>> -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
>> ................
>> -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
>> ................
>> -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
>> ................
>>
>> [CAUSE]
>> Test case btrfs/143 uses dm-dust device to emulate read errors, this
>> means we can not use _scratch_cycle_mount to cycle mount $SCRATCH_MNT.
>>
>> As it would go mount $SCRATCH_DEV, not the dm-dust device to
>> $SCRATCH_MNT.
>> This prevents us to trigger read-repair (since no error would be hit)
>> thus fail the test.
>>
>> [FIX]
>> Since we can mount whatever device at $SCRATCH_MNT, we can not use
>> _scratch_cycle_mount in this case.
>>
>> Instead implement a small helper to grab the mounted device and its
>> mount options, and use the same device and mount options to cycle
>> $SCRATCH_MNT mount.
>>
>> This would fix btrfs/143 and hopefully future test cases which use dm
>> devices.
>>
>> Reported-by: Filipe Manana <fdmanana@suse.com>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> common/btrfs | 14 ++++++++++++--
>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/common/btrfs b/common/btrfs
>> index 175b33ae..4a02b2cc 100644
>> --- a/common/btrfs
>> +++ b/common/btrfs
>> @@ -601,8 +601,18 @@ _btrfs_buffered_read_on_mirror()
>> # The drop_caches doesn't seem to drop every pages on aarch64 with
>> # 64K page size.
>> # So here as another workaround, cycle mount the SCRATCH_MNT to ensure
>> - # the cache are dropped.
>> - _scratch_cycle_mount
>> + # the cache are dropped, but we can not use _scratch_cycle_mount, as
>> + # we may mount whatever dm device at SCRATCH_MNT.
>> + # So here we grab the mounted block device and its mount options, then
>> + # unmount and re-mount with the same device and options.
>> + local mount_info=$(_mount | grep "$SCRATCH_MNT")
>> + if [ -z "$mount_info" ]; then
>> + _fail "failed to grab mount info of $SCRATCH_MNT"
>> + fi
>> + local dev=$(echo $mount_info | $AWK_PROG '{print $1}')
>> + local opts=$(echo $mount_info | $AWK_PROG '{print $6}' | sed 's/[()]//g')
>
> The `findmnt` can help to get $dev and $opts:
>
> local dev=$(findmnt -n -T $SCRATCH_MNT -o SOURCE)
> local opts=$(findmnt -n -T $SCRATCH_MNT -o OPTIONS)
>
> If you hope to check you can keep:
>
> if [ -z "$dev" -o -z "$opts" ];then
> _fail "failed to grab mount info of $SCRATCH_MNT"
> fi
That's really helpful!
>
>> + _scratch_unmount
>> + _mount $dev -o $opts $SCRATCH_MNT
>
> I'm wondering can this help that, after you get the "real" device name:
>
> SCRATCH_DEV=$dev _scratch_cycle_mount
AFAIK we still need to specify the mount option.
As it's possible previous mount is specifying certain mount option
that's not in MOUNT_OPTIONS environment variables.
E.g. mounting a specific subvolume or a temporary mount option.
Thus I believe we may still need to specific the mount options.
Thanks,
Qu
>
> Thanks,
> Zorro
>
>> while [[ -z $( (( BASHPID % nr_mirrors == mirror )) &&
>> exec $XFS_IO_PROG \
>> -c "pread -b $size $offset $size" $file) ]]; do
>> --
>> 2.39.0
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] common/btrfs: handle dmdust as mounted device in _btrfs_buffered_read_on_mirror()
2023-06-26 21:23 ` Qu Wenruo
@ 2023-06-28 11:34 ` Zorro Lang
2023-06-28 11:40 ` Qu Wenruo
0 siblings, 1 reply; 7+ messages in thread
From: Zorro Lang @ 2023-06-28 11:34 UTC (permalink / raw)
To: Qu Wenruo; +Cc: Qu Wenruo, linux-btrfs, fstests, Filipe Manana
On Tue, Jun 27, 2023 at 05:23:31AM +0800, Qu Wenruo wrote:
>
>
> On 2023/6/27 01:32, Zorro Lang wrote:
> > On Mon, Jun 26, 2023 at 02:00:52PM +0800, Qu Wenruo wrote:
> > > [BUG]
> > > After commit ab41f0bddb73 ("common/btrfs: use _scratch_cycle_mount to
> > > ensure all page caches are dropped"), the test case btrfs/143 can fail
> > > like below:
> > >
> > > btrfs/143 6s ... [failed, exit status 1]- output mismatch (see ~/xfstests/results//btrfs/143.out.bad)
> > > --- tests/btrfs/143.out 2020-06-10 19:29:03.818519162 +0100
> > > +++ ~/xfstests/results//btrfs/143.out.bad 2023-06-19 17:04:00.575033899 +0100
> > > @@ -1,37 +1,6 @@
> > > QA output created by 143
> > > wrote 131072/131072 bytes
> > > XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > > -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
> > > ................
> > > -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
> > > ................
> > > -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
> > > ................
> > > -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
> > > ................
> > >
> > > [CAUSE]
> > > Test case btrfs/143 uses dm-dust device to emulate read errors, this
> > > means we can not use _scratch_cycle_mount to cycle mount $SCRATCH_MNT.
> > >
> > > As it would go mount $SCRATCH_DEV, not the dm-dust device to
> > > $SCRATCH_MNT.
> > > This prevents us to trigger read-repair (since no error would be hit)
> > > thus fail the test.
> > >
> > > [FIX]
> > > Since we can mount whatever device at $SCRATCH_MNT, we can not use
> > > _scratch_cycle_mount in this case.
> > >
> > > Instead implement a small helper to grab the mounted device and its
> > > mount options, and use the same device and mount options to cycle
> > > $SCRATCH_MNT mount.
> > >
> > > This would fix btrfs/143 and hopefully future test cases which use dm
> > > devices.
> > >
> > > Reported-by: Filipe Manana <fdmanana@suse.com>
> > > Signed-off-by: Qu Wenruo <wqu@suse.com>
> > > ---
> > > common/btrfs | 14 ++++++++++++--
> > > 1 file changed, 12 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/common/btrfs b/common/btrfs
> > > index 175b33ae..4a02b2cc 100644
> > > --- a/common/btrfs
> > > +++ b/common/btrfs
> > > @@ -601,8 +601,18 @@ _btrfs_buffered_read_on_mirror()
> > > # The drop_caches doesn't seem to drop every pages on aarch64 with
> > > # 64K page size.
> > > # So here as another workaround, cycle mount the SCRATCH_MNT to ensure
> > > - # the cache are dropped.
> > > - _scratch_cycle_mount
> > > + # the cache are dropped, but we can not use _scratch_cycle_mount, as
> > > + # we may mount whatever dm device at SCRATCH_MNT.
> > > + # So here we grab the mounted block device and its mount options, then
> > > + # unmount and re-mount with the same device and options.
> > > + local mount_info=$(_mount | grep "$SCRATCH_MNT")
> > > + if [ -z "$mount_info" ]; then
> > > + _fail "failed to grab mount info of $SCRATCH_MNT"
> > > + fi
> > > + local dev=$(echo $mount_info | $AWK_PROG '{print $1}')
> > > + local opts=$(echo $mount_info | $AWK_PROG '{print $6}' | sed 's/[()]//g')
> >
> > The `findmnt` can help to get $dev and $opts:
> >
> > local dev=$(findmnt -n -T $SCRATCH_MNT -o SOURCE)
> > local opts=$(findmnt -n -T $SCRATCH_MNT -o OPTIONS)
> >
> > If you hope to check you can keep:
> >
> > if [ -z "$dev" -o -z "$opts" ];then
> > _fail "failed to grab mount info of $SCRATCH_MNT"
> > fi
>
> That's really helpful!
>
> >
> > > + _scratch_unmount
> > > + _mount $dev -o $opts $SCRATCH_MNT
> >
> > I'm wondering can this help that, after you get the "real" device name:
> >
> > SCRATCH_DEV=$dev _scratch_cycle_mount
>
> AFAIK we still need to specify the mount option.
>
> As it's possible previous mount is specifying certain mount option
> that's not in MOUNT_OPTIONS environment variables.
>
> E.g. mounting a specific subvolume or a temporary mount option.
>
> Thus I believe we may still need to specific the mount options.
Hmm... if the _scratch_cycle_mount doesn't support dmdust, others dmxxxx
(e.g. dmdelay, dmthin, dmerror, dmflaky) have similar problem, right?
Thanks,
Zorro
>
> Thanks,
> Qu
>
> >
> > Thanks,
> > Zorro
> >
> > > while [[ -z $( (( BASHPID % nr_mirrors == mirror )) &&
> > > exec $XFS_IO_PROG \
> > > -c "pread -b $size $offset $size" $file) ]]; do
> > > --
> > > 2.39.0
> > >
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] common/btrfs: handle dmdust as mounted device in _btrfs_buffered_read_on_mirror()
2023-06-28 11:34 ` Zorro Lang
@ 2023-06-28 11:40 ` Qu Wenruo
2023-06-28 15:16 ` Zorro Lang
0 siblings, 1 reply; 7+ messages in thread
From: Qu Wenruo @ 2023-06-28 11:40 UTC (permalink / raw)
To: Zorro Lang; +Cc: Qu Wenruo, linux-btrfs, fstests, Filipe Manana
On 2023/6/28 19:34, Zorro Lang wrote:
> On Tue, Jun 27, 2023 at 05:23:31AM +0800, Qu Wenruo wrote:
>>
>>
>> On 2023/6/27 01:32, Zorro Lang wrote:
>>> On Mon, Jun 26, 2023 at 02:00:52PM +0800, Qu Wenruo wrote:
>>>> [BUG]
>>>> After commit ab41f0bddb73 ("common/btrfs: use _scratch_cycle_mount to
>>>> ensure all page caches are dropped"), the test case btrfs/143 can fail
>>>> like below:
>>>>
>>>> btrfs/143 6s ... [failed, exit status 1]- output mismatch (see ~/xfstests/results//btrfs/143.out.bad)
>>>> --- tests/btrfs/143.out 2020-06-10 19:29:03.818519162 +0100
>>>> +++ ~/xfstests/results//btrfs/143.out.bad 2023-06-19 17:04:00.575033899 +0100
>>>> @@ -1,37 +1,6 @@
>>>> QA output created by 143
>>>> wrote 131072/131072 bytes
>>>> XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>>>> -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
>>>> ................
>>>> -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
>>>> ................
>>>> -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
>>>> ................
>>>> -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
>>>> ................
>>>>
>>>> [CAUSE]
>>>> Test case btrfs/143 uses dm-dust device to emulate read errors, this
>>>> means we can not use _scratch_cycle_mount to cycle mount $SCRATCH_MNT.
>>>>
>>>> As it would go mount $SCRATCH_DEV, not the dm-dust device to
>>>> $SCRATCH_MNT.
>>>> This prevents us to trigger read-repair (since no error would be hit)
>>>> thus fail the test.
>>>>
>>>> [FIX]
>>>> Since we can mount whatever device at $SCRATCH_MNT, we can not use
>>>> _scratch_cycle_mount in this case.
>>>>
>>>> Instead implement a small helper to grab the mounted device and its
>>>> mount options, and use the same device and mount options to cycle
>>>> $SCRATCH_MNT mount.
>>>>
>>>> This would fix btrfs/143 and hopefully future test cases which use dm
>>>> devices.
>>>>
>>>> Reported-by: Filipe Manana <fdmanana@suse.com>
>>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>>> ---
>>>> common/btrfs | 14 ++++++++++++--
>>>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/common/btrfs b/common/btrfs
>>>> index 175b33ae..4a02b2cc 100644
>>>> --- a/common/btrfs
>>>> +++ b/common/btrfs
>>>> @@ -601,8 +601,18 @@ _btrfs_buffered_read_on_mirror()
>>>> # The drop_caches doesn't seem to drop every pages on aarch64 with
>>>> # 64K page size.
>>>> # So here as another workaround, cycle mount the SCRATCH_MNT to ensure
>>>> - # the cache are dropped.
>>>> - _scratch_cycle_mount
>>>> + # the cache are dropped, but we can not use _scratch_cycle_mount, as
>>>> + # we may mount whatever dm device at SCRATCH_MNT.
>>>> + # So here we grab the mounted block device and its mount options, then
>>>> + # unmount and re-mount with the same device and options.
>>>> + local mount_info=$(_mount | grep "$SCRATCH_MNT")
>>>> + if [ -z "$mount_info" ]; then
>>>> + _fail "failed to grab mount info of $SCRATCH_MNT"
>>>> + fi
>>>> + local dev=$(echo $mount_info | $AWK_PROG '{print $1}')
>>>> + local opts=$(echo $mount_info | $AWK_PROG '{print $6}' | sed 's/[()]//g')
>>>
>>> The `findmnt` can help to get $dev and $opts:
>>>
>>> local dev=$(findmnt -n -T $SCRATCH_MNT -o SOURCE)
>>> local opts=$(findmnt -n -T $SCRATCH_MNT -o OPTIONS)
>>>
>>> If you hope to check you can keep:
>>>
>>> if [ -z "$dev" -o -z "$opts" ];then
>>> _fail "failed to grab mount info of $SCRATCH_MNT"
>>> fi
>>
>> That's really helpful!
>>
>>>
>>>> + _scratch_unmount
>>>> + _mount $dev -o $opts $SCRATCH_MNT
>>>
>>> I'm wondering can this help that, after you get the "real" device name:
>>>
>>> SCRATCH_DEV=$dev _scratch_cycle_mount
>>
>> AFAIK we still need to specify the mount option.
>>
>> As it's possible previous mount is specifying certain mount option
>> that's not in MOUNT_OPTIONS environment variables.
>>
>> E.g. mounting a specific subvolume or a temporary mount option.
>>
>> Thus I believe we may still need to specific the mount options.
>
> Hmm... if the _scratch_cycle_mount doesn't support dmdust, others dmxxxx
> (e.g. dmdelay, dmthin, dmerror, dmflaky) have similar problem, right?
Yes, but my point here is, although "SCRATCH_DEV=$dev
_scratch_cycle_mount" can work for most cases, it can still miss the
specific mount option of the current mount.
Thus we still need to go "_mount $dev -o $opts $SCRATCH_MNT", just for
the extra mount options.
Thanks,
Qu
>
> Thanks,
> Zorro
>
>>
>> Thanks,
>> Qu
>>
>>>
>>> Thanks,
>>> Zorro
>>>
>>>> while [[ -z $( (( BASHPID % nr_mirrors == mirror )) &&
>>>> exec $XFS_IO_PROG \
>>>> -c "pread -b $size $offset $size" $file) ]]; do
>>>> --
>>>> 2.39.0
>>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] common/btrfs: handle dmdust as mounted device in _btrfs_buffered_read_on_mirror()
2023-06-28 11:40 ` Qu Wenruo
@ 2023-06-28 15:16 ` Zorro Lang
0 siblings, 0 replies; 7+ messages in thread
From: Zorro Lang @ 2023-06-28 15:16 UTC (permalink / raw)
To: Qu Wenruo; +Cc: Qu Wenruo, linux-btrfs, fstests, Filipe Manana
On Wed, Jun 28, 2023 at 07:40:14PM +0800, Qu Wenruo wrote:
>
>
> On 2023/6/28 19:34, Zorro Lang wrote:
> > On Tue, Jun 27, 2023 at 05:23:31AM +0800, Qu Wenruo wrote:
> > >
> > >
> > > On 2023/6/27 01:32, Zorro Lang wrote:
> > > > On Mon, Jun 26, 2023 at 02:00:52PM +0800, Qu Wenruo wrote:
> > > > > [BUG]
> > > > > After commit ab41f0bddb73 ("common/btrfs: use _scratch_cycle_mount to
> > > > > ensure all page caches are dropped"), the test case btrfs/143 can fail
> > > > > like below:
> > > > >
> > > > > btrfs/143 6s ... [failed, exit status 1]- output mismatch (see ~/xfstests/results//btrfs/143.out.bad)
> > > > > --- tests/btrfs/143.out 2020-06-10 19:29:03.818519162 +0100
> > > > > +++ ~/xfstests/results//btrfs/143.out.bad 2023-06-19 17:04:00.575033899 +0100
> > > > > @@ -1,37 +1,6 @@
> > > > > QA output created by 143
> > > > > wrote 131072/131072 bytes
> > > > > XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > > > > -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
> > > > > ................
> > > > > -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
> > > > > ................
> > > > > -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
> > > > > ................
> > > > > -XXXXXXXX: aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
> > > > > ................
> > > > >
> > > > > [CAUSE]
> > > > > Test case btrfs/143 uses dm-dust device to emulate read errors, this
> > > > > means we can not use _scratch_cycle_mount to cycle mount $SCRATCH_MNT.
> > > > >
> > > > > As it would go mount $SCRATCH_DEV, not the dm-dust device to
> > > > > $SCRATCH_MNT.
> > > > > This prevents us to trigger read-repair (since no error would be hit)
> > > > > thus fail the test.
> > > > >
> > > > > [FIX]
> > > > > Since we can mount whatever device at $SCRATCH_MNT, we can not use
> > > > > _scratch_cycle_mount in this case.
> > > > >
> > > > > Instead implement a small helper to grab the mounted device and its
> > > > > mount options, and use the same device and mount options to cycle
> > > > > $SCRATCH_MNT mount.
> > > > >
> > > > > This would fix btrfs/143 and hopefully future test cases which use dm
> > > > > devices.
> > > > >
> > > > > Reported-by: Filipe Manana <fdmanana@suse.com>
> > > > > Signed-off-by: Qu Wenruo <wqu@suse.com>
> > > > > ---
> > > > > common/btrfs | 14 ++++++++++++--
> > > > > 1 file changed, 12 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/common/btrfs b/common/btrfs
> > > > > index 175b33ae..4a02b2cc 100644
> > > > > --- a/common/btrfs
> > > > > +++ b/common/btrfs
> > > > > @@ -601,8 +601,18 @@ _btrfs_buffered_read_on_mirror()
> > > > > # The drop_caches doesn't seem to drop every pages on aarch64 with
> > > > > # 64K page size.
> > > > > # So here as another workaround, cycle mount the SCRATCH_MNT to ensure
> > > > > - # the cache are dropped.
> > > > > - _scratch_cycle_mount
> > > > > + # the cache are dropped, but we can not use _scratch_cycle_mount, as
> > > > > + # we may mount whatever dm device at SCRATCH_MNT.
> > > > > + # So here we grab the mounted block device and its mount options, then
> > > > > + # unmount and re-mount with the same device and options.
> > > > > + local mount_info=$(_mount | grep "$SCRATCH_MNT")
> > > > > + if [ -z "$mount_info" ]; then
> > > > > + _fail "failed to grab mount info of $SCRATCH_MNT"
> > > > > + fi
> > > > > + local dev=$(echo $mount_info | $AWK_PROG '{print $1}')
> > > > > + local opts=$(echo $mount_info | $AWK_PROG '{print $6}' | sed 's/[()]//g')
> > > >
> > > > The `findmnt` can help to get $dev and $opts:
> > > >
> > > > local dev=$(findmnt -n -T $SCRATCH_MNT -o SOURCE)
> > > > local opts=$(findmnt -n -T $SCRATCH_MNT -o OPTIONS)
> > > >
> > > > If you hope to check you can keep:
> > > >
> > > > if [ -z "$dev" -o -z "$opts" ];then
> > > > _fail "failed to grab mount info of $SCRATCH_MNT"
> > > > fi
> > >
> > > That's really helpful!
> > >
> > > >
> > > > > + _scratch_unmount
> > > > > + _mount $dev -o $opts $SCRATCH_MNT
> > > >
> > > > I'm wondering can this help that, after you get the "real" device name:
> > > >
> > > > SCRATCH_DEV=$dev _scratch_cycle_mount
> > >
> > > AFAIK we still need to specify the mount option.
> > >
> > > As it's possible previous mount is specifying certain mount option
> > > that's not in MOUNT_OPTIONS environment variables.
> > >
> > > E.g. mounting a specific subvolume or a temporary mount option.
> > >
> > > Thus I believe we may still need to specific the mount options.
> >
> > Hmm... if the _scratch_cycle_mount doesn't support dmdust, others dmxxxx
> > (e.g. dmdelay, dmthin, dmerror, dmflaky) have similar problem, right?
>
> Yes, but my point here is, although "SCRATCH_DEV=$dev
> _scratch_cycle_mount" can work for most cases, it can still miss the
> specific mount option of the current mount.
>
> Thus we still need to go "_mount $dev -o $opts $SCRATCH_MNT", just for
> the extra mount options.
OK, let's merge this patch to fix this small regression issue at first. Then
I'll think about how to make _scratch_cycle_mount work with dmXXXX in another
patch.
Please send a V2 to use findmnt, I'll merge it in next fstests release.
Thanks,
Zorro
>
> Thanks,
> Qu
> >
> > Thanks,
> > Zorro
> >
> > >
> > > Thanks,
> > > Qu
> > >
> > > >
> > > > Thanks,
> > > > Zorro
> > > >
> > > > > while [[ -z $( (( BASHPID % nr_mirrors == mirror )) &&
> > > > > exec $XFS_IO_PROG \
> > > > > -c "pread -b $size $offset $size" $file) ]]; do
> > > > > --
> > > > > 2.39.0
> > > > >
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-06-28 15:18 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-26 6:00 [PATCH] common/btrfs: handle dmdust as mounted device in _btrfs_buffered_read_on_mirror() Qu Wenruo
2023-06-26 16:36 ` Filipe Manana
2023-06-26 17:32 ` Zorro Lang
2023-06-26 21:23 ` Qu Wenruo
2023-06-28 11:34 ` Zorro Lang
2023-06-28 11:40 ` Qu Wenruo
2023-06-28 15:16 ` Zorro Lang
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).