* [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).