From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751575AbdKFDLU (ORCPT ); Sun, 5 Nov 2017 22:11:20 -0500 Received: from szxga04-in.huawei.com ([45.249.212.190]:9955 "EHLO szxga04-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751073AbdKFDLQ (ORCPT ); Sun, 5 Nov 2017 22:11:16 -0500 Subject: Re: [PATCH 2/3] f2fs: trace checkpoint reason in fsync() To: Jaegeuk Kim , Chao Yu CC: , References: <20171105135330.15070-1-chao@kernel.org> <20171105135330.15070-2-chao@kernel.org> <20171106005541.GB49513@jaegeuk-macbookpro.roam.corp.google.com> From: Chao Yu Message-ID: Date: Mon, 6 Nov 2017 11:10:33 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <20171106005541.GB49513@jaegeuk-macbookpro.roam.corp.google.com> Content-Type: text/plain; charset="windows-1252" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.134.22.195] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020203.59FFD2CA.00EB,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 40061a30e8e7f9890197be07a3ca7866 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2017/11/6 8:55, Jaegeuk Kim wrote: > On 11/05, Chao Yu wrote: >> From: Chao Yu >> >> This patch slightly changes need_do_checkpoint to return the detail >> info that indicates why we need do checkpoint, then caller could print >> it with trace message. >> >> Signed-off-by: Chao Yu >> --- >> fs/f2fs/file.c | 34 ++++++++++++++++++---------------- >> include/trace/events/f2fs.h | 12 ++++++------ >> 2 files changed, 24 insertions(+), 22 deletions(-) >> >> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c >> index cee0f36ba9a1..d9d8b1c372f8 100644 >> --- a/fs/f2fs/file.c >> +++ b/fs/f2fs/file.c >> @@ -141,27 +141,29 @@ static int get_parent_ino(struct inode *inode, nid_t *pino) >> return 1; >> } >> >> -static inline bool need_do_checkpoint(struct inode *inode) >> +static inline int need_do_checkpoint(struct inode *inode) >> { >> struct f2fs_sb_info *sbi = F2FS_I_SB(inode); >> - bool need_cp = false; >> + int cp_reason = 0; >> >> - if (!S_ISREG(inode->i_mode) || inode->i_nlink != 1) >> - need_cp = true; >> + if (!S_ISREG(inode->i_mode)) >> + cp_reason = 1; >> + else if (inode->i_nlink != 1) >> + cp_reason = 2; >> else if (is_sbi_flag_set(sbi, SBI_NEED_CP)) >> - need_cp = true; >> + cp_reason = 3; >> else if (file_wrong_pino(inode)) >> - need_cp = true; >> + cp_reason = 4; >> else if (!space_for_roll_forward(sbi)) >> - need_cp = true; >> + cp_reason = 5; >> else if (!is_checkpointed_node(sbi, F2FS_I(inode)->i_pino)) >> - need_cp = true; >> + cp_reason = 6; >> else if (test_opt(sbi, FASTBOOT)) >> - need_cp = true; >> + cp_reason = 7; >> else if (sbi->active_logs == 2) >> - need_cp = true; >> + cp_reason = 8; > > We need to make the numbers by enum so that we could interpret them in tracepoint > below. Agreed, could you check v2? :) Thanks, > > Thanks, > >> >> - return need_cp; >> + return cp_reason; >> } >> >> static bool need_inode_page_update(struct f2fs_sb_info *sbi, nid_t ino) >> @@ -196,7 +198,7 @@ static int f2fs_do_sync_file(struct file *file, loff_t start, loff_t end, >> struct f2fs_sb_info *sbi = F2FS_I_SB(inode); >> nid_t ino = inode->i_ino; >> int ret = 0; >> - bool need_cp = false; >> + bool cp_reason = 0; >> struct writeback_control wbc = { >> .sync_mode = WB_SYNC_ALL, >> .nr_to_write = LONG_MAX, >> @@ -215,7 +217,7 @@ static int f2fs_do_sync_file(struct file *file, loff_t start, loff_t end, >> clear_inode_flag(inode, FI_NEED_IPU); >> >> if (ret) { >> - trace_f2fs_sync_file_exit(inode, need_cp, datasync, ret); >> + trace_f2fs_sync_file_exit(inode, cp_reason, datasync, ret); >> return ret; >> } >> >> @@ -246,10 +248,10 @@ static int f2fs_do_sync_file(struct file *file, loff_t start, loff_t end, >> * sudden-power-off. >> */ >> down_read(&F2FS_I(inode)->i_sem); >> - need_cp = need_do_checkpoint(inode); >> + cp_reason = need_do_checkpoint(inode); >> up_read(&F2FS_I(inode)->i_sem); >> >> - if (need_cp) { >> + if (cp_reason) { >> /* all the dirty node pages should be flushed for POR */ >> ret = f2fs_sync_fs(inode->i_sb, 1); >> >> @@ -306,7 +308,7 @@ static int f2fs_do_sync_file(struct file *file, loff_t start, loff_t end, >> } >> f2fs_update_time(sbi, REQ_TIME); >> out: >> - trace_f2fs_sync_file_exit(inode, need_cp, datasync, ret); >> + trace_f2fs_sync_file_exit(inode, cp_reason, datasync, ret); >> f2fs_trace_ios(NULL, 1); >> return ret; >> } >> diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h >> index 009745564214..f1a317e6db79 100644 >> --- a/include/trace/events/f2fs.h >> +++ b/include/trace/events/f2fs.h >> @@ -210,14 +210,14 @@ DEFINE_EVENT(f2fs__inode, f2fs_sync_file_enter, >> >> TRACE_EVENT(f2fs_sync_file_exit, >> >> - TP_PROTO(struct inode *inode, int need_cp, int datasync, int ret), >> + TP_PROTO(struct inode *inode, int cp_reason, int datasync, int ret), >> >> - TP_ARGS(inode, need_cp, datasync, ret), >> + TP_ARGS(inode, cp_reason, datasync, ret), >> >> TP_STRUCT__entry( >> __field(dev_t, dev) >> __field(ino_t, ino) >> - __field(int, need_cp) >> + __field(int, cp_reason) >> __field(int, datasync) >> __field(int, ret) >> ), >> @@ -225,15 +225,15 @@ TRACE_EVENT(f2fs_sync_file_exit, >> TP_fast_assign( >> __entry->dev = inode->i_sb->s_dev; >> __entry->ino = inode->i_ino; >> - __entry->need_cp = need_cp; >> + __entry->cp_reason = cp_reason; >> __entry->datasync = datasync; >> __entry->ret = ret; >> ), >> >> - TP_printk("dev = (%d,%d), ino = %lu, checkpoint is %s, " >> + TP_printk("dev = (%d,%d), ino = %lu, cp_reason:%d, " >> "datasync = %d, ret = %d", >> show_dev_ino(__entry), >> - __entry->need_cp ? "needed" : "not needed", >> + __entry->cp_reason, >> __entry->datasync, >> __entry->ret) >> ); >> -- >> 2.14.1.145.gb3622a4ee > > . >