* [PATCH 5/5] f2fs: move the list_head initialization into the lock protection region @ 2013-11-19 10:03 Gu Zheng 2013-11-20 1:31 ` Jaegeuk Kim 0 siblings, 1 reply; 4+ messages in thread From: Gu Zheng @ 2013-11-19 10:03 UTC (permalink / raw) To: Kim; +Cc: fsdevel, linux-kernel, f2fs Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> --- fs/f2fs/checkpoint.c | 15 ++++++++++----- 1 files changed, 10 insertions(+), 5 deletions(-) diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c index f884589..1de70cc 100644 --- a/fs/f2fs/checkpoint.c +++ b/fs/f2fs/checkpoint.c @@ -511,8 +511,8 @@ void add_dirty_dir_inode(struct inode *inode) void remove_dirty_dir_inode(struct inode *inode) { struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb); - struct list_head *head = &sbi->dir_inode_list; - struct list_head *this; + + struct list_head *this, *head; if (!S_ISDIR(inode->i_mode)) return; @@ -523,6 +523,7 @@ void remove_dirty_dir_inode(struct inode *inode) return; } + head = &sbi->dir_inode_list; list_for_each(this, head) { struct dir_inode_entry *entry; entry = list_entry(this, struct dir_inode_entry, list); @@ -544,11 +545,13 @@ void remove_dirty_dir_inode(struct inode *inode) struct inode *check_dirty_dir_inode(struct f2fs_sb_info *sbi, nid_t ino) { - struct list_head *head = &sbi->dir_inode_list; - struct list_head *this; + + struct list_head *this, *head; struct inode *inode = NULL; spin_lock(&sbi->dir_inode_lock); + + head = &sbi->dir_inode_list; list_for_each(this, head) { struct dir_inode_entry *entry; entry = list_entry(this, struct dir_inode_entry, list); @@ -563,11 +566,13 @@ struct inode *check_dirty_dir_inode(struct f2fs_sb_info *sbi, nid_t ino) void sync_dirty_dir_inodes(struct f2fs_sb_info *sbi) { - struct list_head *head = &sbi->dir_inode_list; + struct list_head *head; struct dir_inode_entry *entry; struct inode *inode; retry: spin_lock(&sbi->dir_inode_lock); + + head = &sbi->dir_inode_list; if (list_empty(head)) { spin_unlock(&sbi->dir_inode_lock); return; -- 1.7.7 ------------------------------------------------------------------------------ Shape the Mobile Experience: Free Subscription Software experts and developers: Be at the forefront of tech innovation. Intel(R) Software Adrenaline delivers strategic insight and game-changing conversations that shape the rapidly evolving mobile landscape. Sign up now. http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 5/5] f2fs: move the list_head initialization into the lock protection region 2013-11-19 10:03 [PATCH 5/5] f2fs: move the list_head initialization into the lock protection region Gu Zheng @ 2013-11-20 1:31 ` Jaegeuk Kim 2013-11-20 9:48 ` Gu Zheng 0 siblings, 1 reply; 4+ messages in thread From: Jaegeuk Kim @ 2013-11-20 1:31 UTC (permalink / raw) To: Gu Zheng; +Cc: fsdevel, linux-kernel, f2fs Hi Gu, IMO, there is no reason to cover the list header by the lock. In any flows, sbi should have the header all the time. What is your opinion? Thanks, 2013-11-19 (화), 18:03 +0800, Gu Zheng: > Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> > --- > fs/f2fs/checkpoint.c | 15 ++++++++++----- > 1 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c > index f884589..1de70cc 100644 > --- a/fs/f2fs/checkpoint.c > +++ b/fs/f2fs/checkpoint.c > @@ -511,8 +511,8 @@ void add_dirty_dir_inode(struct inode *inode) > void remove_dirty_dir_inode(struct inode *inode) > { > struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb); > - struct list_head *head = &sbi->dir_inode_list; > - struct list_head *this; > + > + struct list_head *this, *head; > > if (!S_ISDIR(inode->i_mode)) > return; > @@ -523,6 +523,7 @@ void remove_dirty_dir_inode(struct inode *inode) > return; > } > > + head = &sbi->dir_inode_list; > list_for_each(this, head) { > struct dir_inode_entry *entry; > entry = list_entry(this, struct dir_inode_entry, list); > @@ -544,11 +545,13 @@ void remove_dirty_dir_inode(struct inode *inode) > > struct inode *check_dirty_dir_inode(struct f2fs_sb_info *sbi, nid_t ino) > { > - struct list_head *head = &sbi->dir_inode_list; > - struct list_head *this; > + > + struct list_head *this, *head; > struct inode *inode = NULL; > > spin_lock(&sbi->dir_inode_lock); > + > + head = &sbi->dir_inode_list; > list_for_each(this, head) { > struct dir_inode_entry *entry; > entry = list_entry(this, struct dir_inode_entry, list); > @@ -563,11 +566,13 @@ struct inode *check_dirty_dir_inode(struct f2fs_sb_info *sbi, nid_t ino) > > void sync_dirty_dir_inodes(struct f2fs_sb_info *sbi) > { > - struct list_head *head = &sbi->dir_inode_list; > + struct list_head *head; > struct dir_inode_entry *entry; > struct inode *inode; > retry: > spin_lock(&sbi->dir_inode_lock); > + > + head = &sbi->dir_inode_list; > if (list_empty(head)) { > spin_unlock(&sbi->dir_inode_lock); > return; -- Jaegeuk Kim Samsung ------------------------------------------------------------------------------ Shape the Mobile Experience: Free Subscription Software experts and developers: Be at the forefront of tech innovation. Intel(R) Software Adrenaline delivers strategic insight and game-changing conversations that shape the rapidly evolving mobile landscape. Sign up now. http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 5/5] f2fs: move the list_head initialization into the lock protection region 2013-11-20 1:31 ` Jaegeuk Kim @ 2013-11-20 9:48 ` Gu Zheng 2013-11-21 0:22 ` Jaegeuk Kim 0 siblings, 1 reply; 4+ messages in thread From: Gu Zheng @ 2013-11-20 9:48 UTC (permalink / raw) To: jaegeuk.kim; +Cc: fsdevel, linux-kernel, f2fs Hi Kim, On 11/20/2013 09:31 AM, Jaegeuk Kim wrote: > Hi Gu, > > IMO, there is no reason to cover the list header by the lock. > In any flows, sbi should have the header all the time. Yes, you're right. > What is your opinion? Moving the list_head initialization into the lock protection region, so that we can ignore the list change during the gap(initialization<-->use). And on the other side, it can keep the code style uniform for better maintenance. Regards, Gu > > Thanks, > > 2013-11-19 (화), 18:03 +0800, Gu Zheng: >> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> >> --- >> fs/f2fs/checkpoint.c | 15 ++++++++++----- >> 1 files changed, 10 insertions(+), 5 deletions(-) >> >> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c >> index f884589..1de70cc 100644 >> --- a/fs/f2fs/checkpoint.c >> +++ b/fs/f2fs/checkpoint.c >> @@ -511,8 +511,8 @@ void add_dirty_dir_inode(struct inode *inode) >> void remove_dirty_dir_inode(struct inode *inode) >> { >> struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb); >> - struct list_head *head = &sbi->dir_inode_list; >> - struct list_head *this; >> + >> + struct list_head *this, *head; >> >> if (!S_ISDIR(inode->i_mode)) >> return; >> @@ -523,6 +523,7 @@ void remove_dirty_dir_inode(struct inode *inode) >> return; >> } >> >> + head = &sbi->dir_inode_list; >> list_for_each(this, head) { >> struct dir_inode_entry *entry; >> entry = list_entry(this, struct dir_inode_entry, list); >> @@ -544,11 +545,13 @@ void remove_dirty_dir_inode(struct inode *inode) >> >> struct inode *check_dirty_dir_inode(struct f2fs_sb_info *sbi, nid_t ino) >> { >> - struct list_head *head = &sbi->dir_inode_list; >> - struct list_head *this; >> + >> + struct list_head *this, *head; >> struct inode *inode = NULL; >> >> spin_lock(&sbi->dir_inode_lock); >> + >> + head = &sbi->dir_inode_list; >> list_for_each(this, head) { >> struct dir_inode_entry *entry; >> entry = list_entry(this, struct dir_inode_entry, list); >> @@ -563,11 +566,13 @@ struct inode *check_dirty_dir_inode(struct f2fs_sb_info *sbi, nid_t ino) >> >> void sync_dirty_dir_inodes(struct f2fs_sb_info *sbi) >> { >> - struct list_head *head = &sbi->dir_inode_list; >> + struct list_head *head; >> struct dir_inode_entry *entry; >> struct inode *inode; >> retry: >> spin_lock(&sbi->dir_inode_lock); >> + >> + head = &sbi->dir_inode_list; >> if (list_empty(head)) { >> spin_unlock(&sbi->dir_inode_lock); >> return; > ------------------------------------------------------------------------------ Shape the Mobile Experience: Free Subscription Software experts and developers: Be at the forefront of tech innovation. Intel(R) Software Adrenaline delivers strategic insight and game-changing conversations that shape the rapidly evolving mobile landscape. Sign up now. http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 5/5] f2fs: move the list_head initialization into the lock protection region 2013-11-20 9:48 ` Gu Zheng @ 2013-11-21 0:22 ` Jaegeuk Kim 0 siblings, 0 replies; 4+ messages in thread From: Jaegeuk Kim @ 2013-11-21 0:22 UTC (permalink / raw) To: Gu Zheng; +Cc: fsdevel, linux-kernel, f2fs Ok, got it. Thanks, :) 2013-11-20 (수), 17:48 +0800, Gu Zheng: > Hi Kim, > On 11/20/2013 09:31 AM, Jaegeuk Kim wrote: > > > Hi Gu, > > > > IMO, there is no reason to cover the list header by the lock. > > In any flows, sbi should have the header all the time. > > Yes, you're right. > > > What is your opinion? > > Moving the list_head initialization into the lock protection region, so > that we can ignore the list change during the gap(initialization<-->use). > And on the other side, it can keep the code style uniform for better > maintenance. > > Regards, > Gu > > > > > Thanks, > > > > 2013-11-19 (화), 18:03 +0800, Gu Zheng: > >> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> > >> --- > >> fs/f2fs/checkpoint.c | 15 ++++++++++----- > >> 1 files changed, 10 insertions(+), 5 deletions(-) > >> > >> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c > >> index f884589..1de70cc 100644 > >> --- a/fs/f2fs/checkpoint.c > >> +++ b/fs/f2fs/checkpoint.c > >> @@ -511,8 +511,8 @@ void add_dirty_dir_inode(struct inode *inode) > >> void remove_dirty_dir_inode(struct inode *inode) > >> { > >> struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb); > >> - struct list_head *head = &sbi->dir_inode_list; > >> - struct list_head *this; > >> + > >> + struct list_head *this, *head; > >> > >> if (!S_ISDIR(inode->i_mode)) > >> return; > >> @@ -523,6 +523,7 @@ void remove_dirty_dir_inode(struct inode *inode) > >> return; > >> } > >> > >> + head = &sbi->dir_inode_list; > >> list_for_each(this, head) { > >> struct dir_inode_entry *entry; > >> entry = list_entry(this, struct dir_inode_entry, list); > >> @@ -544,11 +545,13 @@ void remove_dirty_dir_inode(struct inode *inode) > >> > >> struct inode *check_dirty_dir_inode(struct f2fs_sb_info *sbi, nid_t ino) > >> { > >> - struct list_head *head = &sbi->dir_inode_list; > >> - struct list_head *this; > >> + > >> + struct list_head *this, *head; > >> struct inode *inode = NULL; > >> > >> spin_lock(&sbi->dir_inode_lock); > >> + > >> + head = &sbi->dir_inode_list; > >> list_for_each(this, head) { > >> struct dir_inode_entry *entry; > >> entry = list_entry(this, struct dir_inode_entry, list); > >> @@ -563,11 +566,13 @@ struct inode *check_dirty_dir_inode(struct f2fs_sb_info *sbi, nid_t ino) > >> > >> void sync_dirty_dir_inodes(struct f2fs_sb_info *sbi) > >> { > >> - struct list_head *head = &sbi->dir_inode_list; > >> + struct list_head *head; > >> struct dir_inode_entry *entry; > >> struct inode *inode; > >> retry: > >> spin_lock(&sbi->dir_inode_lock); > >> + > >> + head = &sbi->dir_inode_list; > >> if (list_empty(head)) { > >> spin_unlock(&sbi->dir_inode_lock); > >> return; > > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Jaegeuk Kim Samsung ------------------------------------------------------------------------------ Shape the Mobile Experience: Free Subscription Software experts and developers: Be at the forefront of tech innovation. Intel(R) Software Adrenaline delivers strategic insight and game-changing conversations that shape the rapidly evolving mobile landscape. Sign up now. http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-11-21 0:22 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-11-19 10:03 [PATCH 5/5] f2fs: move the list_head initialization into the lock protection region Gu Zheng 2013-11-20 1:31 ` Jaegeuk Kim 2013-11-20 9:48 ` Gu Zheng 2013-11-21 0:22 ` Jaegeuk Kim
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).