On Fri, Feb 27, 2015 at 9:54 AM, Lukáš Czerner <lczerner@xxxxxxxxxx> wrote: > On Thu, 26 Feb 2015, Eric Sandeen wrote: > >> Date: Thu, 26 Feb 2015 20:45:59 -0600 >> From: Eric Sandeen <sandeen@xxxxxxxxxxx> >> To: Jaegeuk Kim <jaegeuk@xxxxxxxxxx>, Dave Chinner <david@xxxxxxxxxxxxx> >> Cc: fstests@xxxxxxxxxxxxxxx, linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxx, >> Filipe Manana <fdmanana@xxxxxxxx> >> Subject: Re: [PATCH 1/2] generic/065: f2fs serves 64KB size with zero data >> >> On 2/26/15 7:23 PM, Jaegeuk Kim wrote: >> > The f2fs provides 64KB size with 0 data after fsync was done to directory file. >> > >> > Cc: Filipe Manana <fdmanana@xxxxxxxx> >> > Signed-off-by: Jaegeuk Kim <jaegeuk@xxxxxxxxxx> >> > --- >> > tests/generic/065 | 4 ++++ >> > 1 file changed, 4 insertions(+) >> > >> > diff --git a/tests/generic/065 b/tests/generic/065 >> > index b5a296d..3d2b437 100755 >> > --- a/tests/generic/065 >> > +++ b/tests/generic/065 >> > @@ -139,6 +139,10 @@ ext3) >> > # a 64Kb file, with all bytes having the value 0xff >> > [ $hello_digest == "ecb99e6ffea7be1e5419350f725da86b" ] && digest_ok=yes >> > ;; >> > +f2fs) >> > + # a 64Kb file, with all bytes having the value 0 >> > + [ $hello_digest == "fcd6bcb56c1689fcef28b57c22475bad" ] && digest_ok=yes >> > + ;; >> >> whoa... I will admit to having poorly reviewed this test. Given that this file was >> never fsynced, I don't think the test should be looking at file contents *at all* >> I'll do an ex post facto review, I think, and really, I think all of the above should >> just be removed from the test. Without fsync, we don't know what's in the file. >> (ext3 could be mounted with writeback mode, for example). > > wow :D > > Different mdsums for different file systems and > > "file 'hello' has expected size and content" > > You have to have some fun from time to time ;) My bad. > > I can not tell whether the content of the file is important, or > whether the explicit fsync on the file defeats the purpose of the > test, but the easier would be to just fsync the file and then print > out md5sum since it should be always the same no matter what file > system you use. I added this check because while doing the btrfs fix I came into a scenario where the file's metadata was inconsistent (i.e. the dir fsync would cause metadata inconsistency for non-empty child files). This was detected by fsck alone, but then I added this check anyway. > > Or we can just not test the content of the file at all. But > regardless of the decision we do not need this patch. > > Felipe what would be the best solution ? (Felipe -> Filipe :)) I think explicitly fsyncing the file, after fsyncing the directory, and then check its md5sum/content is a good thing to test. Like this we know there's only one expected result for all filesystems. I'll send a patch for that soon. thanks > > Thanks! > >> >> -Eric >> >> > *) >> > # an empty file >> > [ $hello_digest == "d41d8cd98f00b204e9800998ecf8427e" ] && digest_ok=yes >> > >> >> -- >> To unsubscribe from this list: send the line "unsubscribe fstests" in >> the body of a message to majordomo@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > > ------------------------------------------------------------------------------ > Dive into the World of Parallel Programming The Go Parallel Website, sponsored > by Intel and developed in partnership with Slashdot Media, is your hub for all > things parallel software development, from weekly thought leadership blogs to > news, videos, case studies, tutorials and more. Take a look and join the > conversation now. http://goparallel.sourceforge.net/ > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel -- Filipe David Manana, "Reasonable men adapt themselves to the world. Unreasonable men adapt the world to themselves. That's why all progress depends on unreasonable men." -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
