* [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: f2fs, fsdevel, linux-kernel
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
^ 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: f2fs, fsdevel, linux-kernel
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
^ 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: f2fs, fsdevel, linux-kernel
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;
>
^ 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: f2fs, fsdevel, linux-kernel
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-11-21 0:23 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