On Sat, 13 Oct 2012 00:49:06 +0900 Jaegeuk Kim <jaegeuk....@gmail.com> wrote:
> 2012-10-11 (목), 09:24 +1100, NeilBrown: > > On Fri, 05 Oct 2012 20:59:29 +0900 김재극 <jaegeuk....@samsung.com> wrote: > > > > > +static void do_checkpoint(struct f2fs_sb_info *sbi, bool is_umount) > > > +{ > > > + struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi); > > > + nid_t last_nid = 0; > > > + int nat_upd_blkoff[3]; > > > + block_t start_blk; > > > + struct page *cp_page; > > > + unsigned int data_sum_blocks, orphan_blocks; > > > + void *kaddr; > > > + __u32 crc32 = 0; > > > + int i; > > > + > > > + /* Flush all the NAT/SIT pages */ > > > + while (get_pages(sbi, F2FS_DIRTY_META)) > > > + sync_meta_pages(sbi, META, LONG_MAX); > > > + > > > + next_free_nid(sbi, &last_nid); > > > + > > > + /* > > > + * modify checkpoint > > > + * version number is already updated > > > + */ > > > + ckpt->elapsed_time = cpu_to_le64(get_mtime(sbi)); > > > + ckpt->valid_block_count = cpu_to_le64(valid_user_blocks(sbi)); > > > + ckpt->free_segment_count = cpu_to_le32(free_segments(sbi)); > > > + for (i = 0; i < 3; i++) { > > > + ckpt->cur_node_segno[i] = > > > + cpu_to_le32(curseg_segno(sbi, i + CURSEG_HOT_NODE)); > > > + ckpt->cur_node_blkoff[i] = > > > + cpu_to_le16(curseg_blkoff(sbi, i + CURSEG_HOT_NODE)); > > > + nat_upd_blkoff[i] = NM_I(sbi)->nat_upd_blkoff[i]; > > > + ckpt->nat_upd_blkoff[i] = cpu_to_le16(nat_upd_blkoff[i]); > > > + ckpt->alloc_type[i + CURSEG_HOT_NODE] = > > > + curseg_alloc_type(sbi, i + CURSEG_HOT_NODE); > > > + } > > > + for (i = 0; i < 3; i++) { > > > + ckpt->cur_data_segno[i] = > > > + cpu_to_le32(curseg_segno(sbi, i + CURSEG_HOT_DATA)); > > > + ckpt->cur_data_blkoff[i] = > > > + cpu_to_le16(curseg_blkoff(sbi, i + CURSEG_HOT_DATA)); > > > + ckpt->alloc_type[i + CURSEG_HOT_DATA] = > > > + curseg_alloc_type(sbi, i + CURSEG_HOT_DATA); > > > + } > > > + > > > + ckpt->valid_node_count = cpu_to_le32(valid_node_count(sbi)); > > > + ckpt->valid_inode_count = cpu_to_le32(valid_inode_count(sbi)); > > > + ckpt->next_free_nid = cpu_to_le32(last_nid); > > > + > > > + /* 2 cp + n data seg summary + orphan inode blocks */ > > > + data_sum_blocks = npages_for_summary_flush(sbi); > > > + if (data_sum_blocks < 3) > > > + ckpt->ckpt_flags |= CP_COMPACT_SUM_FLAG; > > > + else > > > + ckpt->ckpt_flags &= (~CP_COMPACT_SUM_FLAG); > > > + > > > + orphan_blocks = (sbi->n_orphans + F2FS_ORPHANS_PER_BLOCK - 1) > > > + / F2FS_ORPHANS_PER_BLOCK; > > > + ckpt->cp_pack_start_sum = 1 + orphan_blocks; > > > + ckpt->cp_pack_total_block_count = 2 + data_sum_blocks + orphan_blocks; > > > > This looks a bit weird to me, though I might be misunderstanding something. > > > > data_sum_blocks is either 1, 2, or 3. > > "3" actually means "at least 3". > > > > If it is 3, you choose not to set CP_COMPACT_SUM_FLAG. In that case the NAT > > and SIT journal entries go into SSA blocks, not into the checkpoint at all. > > So in that case, zero blocks of the checkpoint are used for journalling. > > Yet > > you still add data_sum_blocks (==3) to the cp_pack_total_block_count (and > > later to the start block). > > Is that really what you want to do? Leave 3 empty blocks? > > > > I would suggest changing npages_for_summary_flush to return 0 if the number > > of blocks needed would be more than three, and set CP_COMPACT_SUM_FLAG only > > when data_sum_blocks > 0. > > > > I don't know if you would need to make a corresponding change to the > > recovery > > code, I haven't fully examined that yet. > > Ok, let me explain about CP_COMPACT_SUM_FLAG. > Let's assume that there are some journal entries and data summaries. > Note that this scenario is not from the umount procedure. > > Basically f2fs writes three data summary blocks for current active logs > inside the checkpoint pack. > And NAT and SIT journal entries are stored in hot and cold data summary > blocks. > So, if the CP_COMPACT_SUM_FLAG is not set, f2fs writes the checkpoint > pack like this. > > [CP 0] > [Orphan blocks] > [Hot sum block w/ NAT journal] > [Warm sum block] > [Cold sum block w/ SIT journal] > [CP 0'] > > But, if the CP_COMPACT_SUM_FLAG is set, the checkpoint pack consists of > 1 or 2 summary blocks as follows. > > [CP 0] > [Orphan blocks] > [summary entries w/ NAT and SIT journal] > [CP 0'] > > or, > > [CP 0] > [Orphan blocks] > [summary entries] > [summary entries w/ NAT and SIT journal] > [CP 0'] > > So, I think it needs no change. > Any idea? > Thanks, I see. I missed the fact that the current data summary blocks are always written to the checkpoint area - I assumed they were being written back to the SSA. So it makes sense now and you are right - no change needed. Thanks, NeilBrown > > > > > Regards, > > NeilBrown >
signature.asc
Description: PGP signature