linux-kernel - Re: [PATCH 5/5] f2fs: move the list_head initialization into the lock protection region

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-id: <1384911101.26319.37.camel@kjgkr>
Date:	Wed, 20 Nov 2013 10:31:41 +0900
From:	Jaegeuk Kim <jaegeuk.kim@...sung.com>
To:	Gu Zheng <guz.fnst@...fujitsu.com>
Cc:	f2fs <linux-f2fs-devel@...ts.sourceforge.net>,
	fsdevel <linux-fsdevel@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 5/5] f2fs: move the list_head initialization into the lock
 protection region

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@...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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/