Linux-BTRFS Archive on lore.kernel.org
 help / color / Atom feed
From: Qu Wenruo <quwenruo.btrfs@gmx.com>
To: fdmanana@gmail.com, Qu Wenruo <wqu@suse.com>
Cc: fstests <fstests@vger.kernel.org>,
	linux-btrfs <linux-btrfs@vger.kernel.org>
Subject: Re: [PATCH] fstests: btrfs: Check snapshot creation and deletion with dm-logwrites
Date: Fri, 16 Aug 2019 17:47:33 +0800
Message-ID: <21a7b532-c16d-c014-e714-e280d0a1850a@gmx.com> (raw)
In-Reply-To: <CAL3q7H5v6jrFcKupRBZ9EnaQDKM2UooK3iwOgJ01wTvqMtizcw@mail.gmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 4485 bytes --]



On 2019/8/16 下午5:25, Filipe Manana wrote:
[...]
> 
> That will also make the test fail on systems with a page size > 4Kb.
> So either make it "_notrun" for systems with a page size != 4Kb or,
> preferably make the test independent of the page size.
> If you want to increase the tree height easily, you can set large
> xattrs for files, like I did in btrfs/187, where even if with a 64Kb
> page size, I get a 3 levels fs tree.
> 
>> +
>> +# We need inline extents to quickly bump the tree height
>> +_log_writes_mount -o max_inline=2048
> 
> Again, setting large xatts (3800 bytes so that it works for any page
> size) is even faster then inline extents.

Greater advice for that!

> 
>> +
>> +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/src > /dev/null 2>&1
> 
> We should catch failures here. Just remove the redirection, pass the
> _filter_scratch filter and update the golden output.
> 
>> +mkdir -p $SCRATCH_MNT/snapshots
>> +mkdir -p $SCRATCH_MNT/src/padding
>> +
>> +random_file() {
> 
> The coding style is "{" on the beginning of the next line.
> Like the _cleanup() function, and the _log_writes_fast_replay_check()
> function you added previously.
> 
>> +       local basedir=$1
>> +       echo "$basedir/$(ls $basedir | sort -R | tail -1)"
>> +}
>> +
>> +snapshot_workload() {
> 
> Same comment as above.
> 
>> +       while true; do
>> +               $BTRFS_UTIL_PROG subvolume snapshot \
>> +                       $SCRATCH_MNT/src $SCRATCH_MNT/snapshots/$i \
>> +                       > /dev/null 2>&1
>> +               # Remove two random padding so each snapshot is different
>> +               rm "$(random_file $SCRATCH_MNT/src/padding)"
> 
> I would pass -f to rm. Having an alias "alias rm='rm -i'" is not very
> uncommon (I have such alias for my accounts).
> 
>> +               rm "$(random_file $SCRATCH_MNT/src/padding)"
> 
> Might be interesting to add a few files to each snapshot too (modify
> existing ones, etc).

No problem.

> 
>> +               sleep 1
>> +       done
>> +}
>> +
>> +delete_workload() {
> 
> Same comment as above.
> 
>> +       while true; do
>> +               sleep 2
>> +               $BTRFS_UTIL_PROG subvolume delete \
>> +                       "$(random_file $SCRATCH_MNT/snapshots)" \
>> +                       > /dev/null 2>&1
>> +       done
>> +}
>> +
>> +# Bump the tree height to 2 at least
>> +for ((i = 0; i < 256; i++)); do
>> +       _pwrite_byte 0xcd 0 2k "$SCRATCH_MNT/src/padding/inline_$i" > /dev/null 2>&1
>> +       _pwrite_byte 0xcd 0 4k "$SCRATCH_MNT/src/padding/regular_$i" > /dev/null 2>&1
>> +done
>> +sync
> 
> Why the call to 'sync'?
> Not needed, snapshot creation flushes delalloc and commits a transaction.
> 
>> +
>> +_log_writes_mark prepare
>> +
>> +snapshot_workload &
>> +pid1=$!
>> +delete_workload &
>> +pid2=$!
>> +
>> +$FSSTRESS_PROG $fsstress_args > /dev/null 2>&1 &
>> +sleep $runtime
>> +
>> +$KILLALL_PROG -q $FSSTRESS_PROG &> /dev/null
> 
> You're very inconsistent within the same test :) Using both ">
> /dev/null 2>&1" and "&> /dev/null".

My bad, I mean 2>&1 > /dev/null.
What I mean is output stderr while skip stdout in previous calls.

Thanks,
Qu

> 
>> +kill $pid1 &> /dev/null
>> +kill $pid2 &> /dev/null
>> +wait
>> +_log_writes_unmount
>> +_log_writes_remove
>> +
>> +# Since we have a lot of work to replay, and relay-log will search
>> +# from the first record to the needed entry, we need to use --fsck option
>> +# to reduce unnecessary search, or it will be too slow
>> +$here/src/log-writes/replay-log --log $LOGWRITES_DEV --replay $SCRATCH_DEV \
>> +       --fsck "btrfs check $SCRATCH_DEV" --check fua >> $seqres.full 2>&1
>> +
>> +echo "Silence is golden"
>> +
>> +# success, all done
>> +status=0
>> +exit
>> diff --git a/tests/btrfs/192.out b/tests/btrfs/192.out
>> new file mode 100644
>> index 00000000..6779aa77
>> --- /dev/null
>> +++ b/tests/btrfs/192.out
>> @@ -0,0 +1,2 @@
>> +QA output created by 192
>> +Silence is golden
>> diff --git a/tests/btrfs/group b/tests/btrfs/group
>> index 2474d43e..24723bf1 100644
>> --- a/tests/btrfs/group
>> +++ b/tests/btrfs/group
>> @@ -194,3 +194,4 @@
>>  189 auto quick send clone
>>  190 auto quick replay balance qgroup
>>  191 auto quick send dedupe
>> +192 auto replay
> 
> Missing "snapshot" group.
> 
> Thanks.
> 
>> --
>> 2.22.0
>>
> 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply index

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-14 10:55 Qu Wenruo
2019-08-16  9:25 ` Filipe Manana
2019-08-16  9:47   ` Qu Wenruo [this message]
2019-08-16 11:30     ` Eryu Guan

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=21a7b532-c16d-c014-e714-e280d0a1850a@gmx.com \
    --to=quwenruo.btrfs@gmx.com \
    --cc=fdmanana@gmail.com \
    --cc=fstests@vger.kernel.org \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=wqu@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-BTRFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-btrfs/0 linux-btrfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-btrfs linux-btrfs/ https://lore.kernel.org/linux-btrfs \
		linux-btrfs@vger.kernel.org linux-btrfs@archiver.kernel.org
	public-inbox-index linux-btrfs


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-btrfs


AGPL code for this site: git clone https://public-inbox.org/ public-inbox