On Sun, 20 Oct 2013 12:03:01 +0800, Wang Shilong wrote: >> On 10/19/2013 12:32, Shilong Wang wrote: >>> 2013/10/19, Stefan Behrens <[email protected]>: >>>> On 10/19/2013 06:17, Wang Shilong wrote: >>>>> From: Wang Shilong <[email protected]> >>>>> >>>>> Scrubing supers is not in a transaction context, when trying to >>>>> write supers to disk, we should check if we are trying to >>>>> scrub supers.Fix it. >>>>> >>>>> Signed-off-by: Wang Shilong <[email protected]> [...] >>>> What kind of race do you see between writing the 4K superblock and scrub >>>> checking its checksum? Or in other words, what could happen? >> >>> Yeah, it did not hurt. but it may output checksum mismatch. For example: >>> Writing 4k superblock is not totally finished, but we are trying to scrub >>> it. >> >> Have you ever seen this issue?
You replied with "No, just noticing it by accident" in the mail before. I don't believe that this issue can ever happen. I don't believe that somewhere on the path to the flash memory, to the magnetic disc or to the drive's cache memory, someone interrupts a 4KB write in the middle of operation to read from this 4KB area. This is not an issue IMHO. >> If yes, let's find a different solution. You scrub, let's say, once a week. >> Scrubbing the superblock takes, let's say, 100ms, then it's finished. This >> short race doesn't justify to add such code to btrfs_commit_transaction and >> btrfs_commit_super IMHO. And commiting a transaction is synchronized to >> scrub already when the commit root is updated. >> >> If this is really an issue and these 4K disk writes and reads interfere, >> let's find a better solution please. > > How about this approach? > > We let scrub_supers in a transaction context. > > btrfs_join_transaction() > > scrub_supers > > btrfs_commit_transaction(). > > This is not elegant, but we can remove scrub_lock with supers(Notice, there > is another place that have used > this lock). -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
