From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1jTtSe-0001hH-IA for mharc-grub-devel@gnu.org; Wed, 29 Apr 2020 16:37:40 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:54140) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jTtSR-0001IC-PU for grub-devel@gnu.org; Wed, 29 Apr 2020 16:37:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.90_1) (envelope-from ) id 1jTtPR-0005Tt-7Z for grub-devel@gnu.org; Wed, 29 Apr 2020 16:37:27 -0400 Received: from relay01.mx.bawue.net ([193.7.176.67]:54222 helo=smtp.bawue.net) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jTtPQ-00051P-Mt for grub-devel@gnu.org; Wed, 29 Apr 2020 16:34:20 -0400 Received: from n-dimensional.de (p5B0825D9.dip0.t-ipconnect.de [91.8.37.217]) (Authenticated sender: pdim@bawue.de) by smtp.bawue.net (Postfix) with ESMTPSA id 580B320484 for ; Wed, 29 Apr 2020 22:34:14 +0200 (CEST) Date: Wed, 29 Apr 2020 22:34:13 +0200 From: Hans Ulrich Niedermann To: grub-devel@gnu.org Subject: Re: [PATCH v2] tests: Don't run f2fs test on systems with PAGE_SIZE more than 4096 bytes Message-ID: <20200429223413.7c08dc15@n-dimensional.de> In-Reply-To: <20200429192557.15dd5a4c@n-dimensional.de> References: <20200429140636.GA4270@yogzotot> <20200429192557.15dd5a4c@n-dimensional.de> X-Mailer: Claws Mail 3.17.5 (GTK+ 2.24.32; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Virus-Scanner: SAV Dynamic Interface 2.6.0, Engine: 3.77.1, SAV: 5.74 (3C348D2F) on relay01.mx.bawue.net using milter-sssp 0.1.0 X-Virus-Scan: Found to be clean. Received-SPF: pass client-ip=193.7.176.67; envelope-from=hun@n-dimensional.de; helo=smtp.bawue.net X-detected-operating-system: by eggs.gnu.org: First seen = 2020/04/29 16:34:17 X-ACL-Warn: Detected OS = Linux 3.11 and newer [fuzzy] X-Received-From: 193.7.176.67 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 29 Apr 2020 20:37:38 -0000 On Wed, 29 Apr 2020 19:25:57 +0200 Hans Ulrich Niedermann wrote: > On Wed, 29 Apr 2020 17:06:36 +0300 > Anatoly Pugachev wrote: > > > Don't run f2fs test on systems with PAGE_SIZE > 4096 bytes, since > > f2fs is not supported on these systems and trying to mount a f2fs > > filesystem would fail. > > "Skip the f2fs test on ..." might be better wording, both in this > paragraph and the Subject. > > Exit code 77 is certainly documented with the word "skip", and "exit > 77" will show up in the "make check" output as "SKIP" as well. > > > v2 changes: > > > > - fix compare > > - quotes around variable expansion > > > > Signed-off-by: Anatoly Pugachev > > Reviewed-by: Mike Gilbert > > > > --- > > tests/f2fs_test.in | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/tests/f2fs_test.in b/tests/f2fs_test.in > > index 1ea77c826..3da1dad57 100644 > > --- a/tests/f2fs_test.in > > +++ b/tests/f2fs_test.in > > @@ -15,5 +15,11 @@ if ! which mkfs.f2fs >/dev/null 2>&1; then > > exit 77 > > fi > > > > +PAGE_SIZE=$(getconf PAGE_SIZE) > > +F2FS_BLKSIZE=4096 > > +if [ "$PAGE_SIZE" -gt "$F2FS_BLKSIZE" ]; then > > + printf "f2fs is not supported on PAGE_SIZE(%d) != %d\n" $PAGE_SIZE > > $F2FS_BLKSIZE > > + exit 77 > > +fi > > This confusing to me. You are skipping the test when > > PAGE_SIZE > F2FS_BLKSIZE > > but the corresponding message says > > PAGE_SIZE != F2FS_BLKSIZE > > Now... which condition is it supposed to be? ">" or "!="? > > I know from the Linux kernel's ext2 driver that it is very well > possible that PAGE_SIZE != EXT2_BLOCK_SIZE can work > unless EXT2_BLOCK_SIZE > PAGE_SIZE. > > So ">" and "!=" are not necessarily the same thing, and IMHO the check > and the message use the same condition. The commit title and the commit message are also talking about "PAGE_SIZE > 4096" instead of "PAGE_SIZE != 4096". This adds more confusion over whether F2FS_BLKSIZE just happens to be 4096 somewhere where PAGE_SIZE is 4096, whether F2FS_BLKSIZE is defined to be 4096 everywhere, and whether the whole thing is about the number 4096 or about the value of F2FS_BLKSIZE. (Again, in the ext2 example, filesystems can have many block sizes, but the default is the Linux PAGE_SIZE, so all x86 PCs will create filesystems with block size 4096 but a MIPS system like the Western Digital SOHO external HDD NAS will create a filesystem with block size 65536. The kernel driver just needs PAGE_SIZE >= block size to work, so a MIPS created ext2 filesystem will not mount on a x86 PC.) Mentioning the same condition in commit title, commit message, the actual "if" branch, and then the "printf" message would certainly reduce my confusion. Uli