On Sun, 10 May 2015 13:04:18 +0200, Andreas Rohner wrote: > On 2015-05-09 14:17, Ryusuke Konishi wrote: >> On Sun, 3 May 2015 12:05:20 +0200, Andreas Rohner wrote: [...] >> >> Uum. This still looks to have potential for leak of dirty block >> collection between DAT and SUFILE since this retry is limited by >> the fixed retry count. >> >> How about adding function temporarily turning off the live block >> tracking and using it after this propagation loop until log write >> finishes ? >> >> It would reduce the accuracy of live block count, but is it enough ? >> How do you think ? We have to eliminate the possibility of the leak >> because it can cause file system corruption. Every checkpoint must be >> self-contained. > > How exactly could it lead to file system corruption? Maybe I miss > something important here, but it seems to me, that no corruption is > possible. > > The nilfs_sufile_flush_cache_node() function only reads in already > existing blocks. No new blocks are created. If I mark those blocks > dirty, the btree is not changed at all. If I do not call > nilfs_bmap_propagate(), then the btree stays unchanged and there are no > dangling pointers. The resulting checkpoint should be self-contained. Good point. As for btree, it looks like no inconsistency issue arises since nilfs_sufile_flush_cache_node() never inserts new blocks as you pointed out. Even though we also must care inconsistency between sufile header and sufile data blocks, and block count in inode as well, fortunately these look to be ok, too. However, I still think it's not good to carry over dirty blocks to the next segment construction to avoid extra checkpoint creation and to simplify things. >From this viewpoint, I also prefer that nilfs_sufile_flush_cache() and nilfs_sufile_flush_cache_node() are changed a bit so that they will skip adjusting su_nlive_blks and su_nlive_lastmod if the sufile block that includes the segment usage is not marked dirty and only_mark == 0 as well as turing off live block counting temporarily after the sufile/DAT propagation loop. > > The only problem would be, that I could lose some nlive_blks updates. > Regards, Ryusuke Konishi -- To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
