[PATCH 1/4] f2fs: fix to cover sentry_lock for block allocation

[PATCH 1/4] f2fs: fix to cover sentry_lock for block allocation

[Jaegeuk Kim]

In the following call stack, f2fs changes the bitmap for dirty segments and # of
dirty sentries without grabbing sit_i->sentry_lock.
This can result in mismatch on bitmap and # of dirty sentries, since if there
are some direct_io operations.

In allocate_data_block,
 - __allocate_new_segments
  - mutex_lock(&curseg->curseg_mutex);
  - s_ops->allocate_segment
   - new_curseg/change_curseg
    - reset_curseg
     - __set_sit_entry_type
      - __mark_sit_entry_dirty
       - set_bit(dirty_sentries_bitmap)
       - dirty_sentries++;

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/segment.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 3e80bd6..eafaf72 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1167,6 +1167,7 @@ void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
 	curseg = CURSEG_I(sbi, type);
 
 	mutex_lock(&curseg->curseg_mutex);
+	mutex_lock(&sit_i->sentry_lock);
 
 	/* direct_io'ed data is aligned to the segment for better performance */
 	if (direct_io && curseg->next_blkoff)
@@ -1181,7 +1182,6 @@ void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
 	 */
 	__add_sum_entry(sbi, type, sum);
 
-	mutex_lock(&sit_i->sentry_lock);
 	__refresh_next_blkoff(sbi, curseg);
 
 	stat_inc_block_count(sbi, curseg);
-- 
2.1.1

[PATCH 2/4] f2fs: set buffer_new when new blocks are allocated

[Jaegeuk Kim]

This patch modifies to call set_buffer_new, if new blocks are allocated.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index ba70a78..cefaa67 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -257,7 +257,6 @@ static void f2fs_map_bh(struct super_block *sb, pgoff_t pgofs,
 	unsigned int blkbits = sb->s_blocksize_bits;
 	size_t count;
 
-	set_buffer_new(bh_result);
 	map_bh(bh_result, sb, ei->blk + pgofs - ei->fofs);
 	count = ei->fofs + ei->len - pgofs;
 	if (count < (UINT_MAX >> blkbits))
@@ -1139,7 +1138,7 @@ static int __get_data_block(struct inode *inode, sector_t iblock,
 		goto put_out;
 
 	if (dn.data_blkaddr != NULL_ADDR) {
-		set_buffer_new(bh_result);
+		clear_buffer_new(bh_result);
 		map_bh(bh_result, inode->i_sb, dn.data_blkaddr);
 	} else if (create) {
 		err = __allocate_data_block(&dn);
@@ -1184,6 +1183,7 @@ get_next:
 			if (err)
 				goto sync_out;
 			allocated = true;
+			set_buffer_new(bh_result);
 			blkaddr = dn.data_blkaddr;
 		}
 		/* Give more consecutive addresses for the readahead */
-- 
2.1.1


------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the 
conversation now. http://goparallel.sourceforge.net/

RE: [f2fs-dev] [PATCH 2/4] f2fs: set buffer_new when new blocks are allocated

[Chao Yu]

Hi Jaegeuk,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Wednesday, March 18, 2015 12:58 AM
> To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 2/4] f2fs: set buffer_new when new blocks are allocated
> 
> This patch modifies to call set_buffer_new, if new blocks are allocated.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/data.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index ba70a78..cefaa67 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -257,7 +257,6 @@ static void f2fs_map_bh(struct super_block *sb, pgoff_t pgofs,
>  	unsigned int blkbits = sb->s_blocksize_bits;
>  	size_t count;
> 
> -	set_buffer_new(bh_result);
>  	map_bh(bh_result, sb, ei->blk + pgofs - ei->fofs);
>  	count = ei->fofs + ei->len - pgofs;
>  	if (count < (UINT_MAX >> blkbits))
> @@ -1139,7 +1138,7 @@ static int __get_data_block(struct inode *inode, sector_t iblock,
>  		goto put_out;
> 
>  	if (dn.data_blkaddr != NULL_ADDR) {
> -		set_buffer_new(bh_result);
> +		clear_buffer_new(bh_result);

Add clear_buffer_new in f2fs_map_bh?

Thanks,

>  		map_bh(bh_result, inode->i_sb, dn.data_blkaddr);
>  	} else if (create) {
>  		err = __allocate_data_block(&dn);
> @@ -1184,6 +1183,7 @@ get_next:
>  			if (err)
>  				goto sync_out;
>  			allocated = true;
> +			set_buffer_new(bh_result);
>  			blkaddr = dn.data_blkaddr;
>  		}
>  		/* Give more consecutive addresses for the readahead */
> --
> 2.1.1

Re: [f2fs-dev] [PATCH 2/4 v2] f2fs: set buffer_new when new blocks are allocated

[Jaegeuk Kim]

Hi Chao,

Thanks for the review.

Change log from v1:
 o add clear_buffer_new() in f2fs_map_bh

This patch modifies to call set_buffer_new, if new blocks are allocated.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index ba70a78..4a416e7 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -257,7 +257,7 @@ static void f2fs_map_bh(struct super_block *sb, pgoff_t pgofs,
 	unsigned int blkbits = sb->s_blocksize_bits;
 	size_t count;
 
-	set_buffer_new(bh_result);
+	clear_buffer_new(bh_result);
 	map_bh(bh_result, sb, ei->blk + pgofs - ei->fofs);
 	count = ei->fofs + ei->len - pgofs;
 	if (count < (UINT_MAX >> blkbits))
@@ -1139,7 +1139,7 @@ static int __get_data_block(struct inode *inode, sector_t iblock,
 		goto put_out;
 
 	if (dn.data_blkaddr != NULL_ADDR) {
-		set_buffer_new(bh_result);
+		clear_buffer_new(bh_result);
 		map_bh(bh_result, inode->i_sb, dn.data_blkaddr);
 	} else if (create) {
 		err = __allocate_data_block(&dn);
@@ -1184,6 +1184,7 @@ get_next:
 			if (err)
 				goto sync_out;
 			allocated = true;
+			set_buffer_new(bh_result);
 			blkaddr = dn.data_blkaddr;
 		}
 		/* Give more consecutive addresses for the readahead */
-- 
2.1.1

RE: [f2fs-dev] [PATCH 2/4 v2] f2fs: set buffer_new when new blocks are allocated

[Chao Yu]

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Thursday, March 19, 2015 1:57 AM
> To: Chao Yu
> Cc: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH 2/4 v2] f2fs: set buffer_new when new blocks are allocated
> 
> Hi Chao,
> 
> Thanks for the review.
> 
> Change log from v1:
>  o add clear_buffer_new() in f2fs_map_bh
> 
> This patch modifies to call set_buffer_new, if new blocks are allocated.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <chao2.yu@samsung.com>

[PATCH 3/4] f2fs: enhance multi-threads performance

[Jaegeuk Kim]

Previously, f2fs_write_data_pages has a mutex, sbi->writepages, to serialize
data writes to maximize write bandwidth, while sacrificing multi-threads
performance.
Practically, however, multi-threads environment is much more important for
users. So this patch tries to remove the mutex.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c  | 7 -------
 fs/f2fs/f2fs.h  | 1 -
 fs/f2fs/super.c | 1 -
 3 files changed, 9 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index cefaa67..1a3d86a 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1396,7 +1396,6 @@ static int f2fs_write_data_pages(struct address_space *mapping,
 {
 	struct inode *inode = mapping->host;
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
-	bool locked = false;
 	int ret;
 	long diff;
 
@@ -1417,13 +1416,7 @@ static int f2fs_write_data_pages(struct address_space *mapping,
 
 	diff = nr_pages_to_write(sbi, DATA, wbc);
 
-	if (!S_ISDIR(inode->i_mode)) {
-		mutex_lock(&sbi->writepages);
-		locked = true;
-	}
 	ret = write_cache_pages(mapping, wbc, __f2fs_writepage, mapping);
-	if (locked)
-		mutex_unlock(&sbi->writepages);
 
 	f2fs_submit_merged_bio(sbi, DATA, WRITE);
 
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index c1ad404..f2909c6 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -604,7 +604,6 @@ struct f2fs_sb_info {
 	struct mutex cp_mutex;			/* checkpoint procedure lock */
 	struct rw_semaphore cp_rwsem;		/* blocking FS operations */
 	struct rw_semaphore node_write;		/* locking node writes */
-	struct mutex writepages;		/* mutex for writepages() */
 	wait_queue_head_t cp_wait;
 
 	struct inode_management im[MAX_INO_ENTRY];      /* manage inode cache */
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 0b8a2d8..c9f4aef 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1027,7 +1027,6 @@ try_onemore:
 	sbi->raw_super = raw_super;
 	sbi->raw_super_buf = raw_super_buf;
 	mutex_init(&sbi->gc_mutex);
-	mutex_init(&sbi->writepages);
 	mutex_init(&sbi->cp_mutex);
 	init_rwsem(&sbi->node_write);
 	clear_sbi_flag(sbi, SBI_POR_DOING);
-- 
2.1.1


------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the 
conversation now. http://goparallel.sourceforge.net/

[PATCH 4/4] f2fs: avoid wrong f2fs_bug_on when truncating inline_data

[Jaegeuk Kim]

This patch removes wrong f2fs_bug_on in truncate_inline_inode.

When there is no space, it can happen a corner case where i_isze is over
MAX_INLINE_SIZE while its inode is still inline_data.

The scenario is
 1. write small data into file #A.
 2. fill the whole partition to 100%.
 3. truncate 4096 on file #A.
 4. write data at 8192 offset.
  --> f2fs_write_begin
    -> -ENOSPC = f2fs_convert_inline_page
    -> f2fs_write_failed
      -> truncate_blocks
        -> truncate_inline_inode
	  BUG_ON, since i_size is 4096.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/inline.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 153c5e7..d3e0599 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -54,13 +54,6 @@ bool truncate_inline_inode(struct page *ipage, u64 from)
 {
 	void *addr;
 
-	/*
-	 * we should never truncate inline data past max inline data size,
-	 * because we always convert inline inode to normal one before
-	 * truncating real data if new size is past max inline data size.
-	 */
-	f2fs_bug_on(F2FS_P_SB(ipage), from > MAX_INLINE_DATA);
-
 	if (from >= MAX_INLINE_DATA)
 		return false;
 
-- 
2.1.1

RE: [f2fs-dev] [PATCH 4/4] f2fs: avoid wrong f2fs_bug_on when truncating inline_data

[Chao Yu]

Hi Jaegeuk,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Wednesday, March 18, 2015 12:58 AM
> To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Cc: Jaegeuk Kim
> Subject: [f2fs-dev] [PATCH 4/4] f2fs: avoid wrong f2fs_bug_on when truncating inline_data
> 
> This patch removes wrong f2fs_bug_on in truncate_inline_inode.
> 
> When there is no space, it can happen a corner case where i_isze is over
> MAX_INLINE_SIZE while its inode is still inline_data.
> 
> The scenario is
>  1. write small data into file #A.
>  2. fill the whole partition to 100%.
>  3. truncate 4096 on file #A.

If we truncate size over MAX_INLINE_DATA, we will convert inline data in
f2fs_truncate rather than write_begin below. isn't it?

Thanks,

>  4. write data at 8192 offset.
>   --> f2fs_write_begin
>     -> -ENOSPC = f2fs_convert_inline_page
>     -> f2fs_write_failed
>       -> truncate_blocks
>         -> truncate_inline_inode
> 	  BUG_ON, since i_size is 4096.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/inline.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> index 153c5e7..d3e0599 100644
> --- a/fs/f2fs/inline.c
> +++ b/fs/f2fs/inline.c
> @@ -54,13 +54,6 @@ bool truncate_inline_inode(struct page *ipage, u64 from)
>  {
>  	void *addr;
> 
> -	/*
> -	 * we should never truncate inline data past max inline data size,
> -	 * because we always convert inline inode to normal one before
> -	 * truncating real data if new size is past max inline data size.
> -	 */
> -	f2fs_bug_on(F2FS_P_SB(ipage), from > MAX_INLINE_DATA);
> -
>  	if (from >= MAX_INLINE_DATA)
>  		return false;
> 
> --
> 2.1.1
> 
> 
> ------------------------------------------------------------------------------
> Dive into the World of Parallel Programming The Go Parallel Website, sponsored
> by Intel and developed in partnership with Slashdot Media, is your hub for all
> things parallel software development, from weekly thought leadership blogs to
> news, videos, case studies, tutorials and more. Take a look and join the
> conversation now. http://goparallel.sourceforge.net/
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 4/4] f2fs: avoid wrong f2fs_bug_on when truncating inline_data

[Jaegeuk Kim]

Hi,

On Wed, Mar 18, 2015 at 09:12:00AM +0800, Chao Yu wrote:
> Hi Jaegeuk,
> 
> > -----Original Message-----
> > From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> > Sent: Wednesday, March 18, 2015 12:58 AM
> > To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> > linux-f2fs-devel@lists.sourceforge.net
> > Cc: Jaegeuk Kim
> > Subject: [f2fs-dev] [PATCH 4/4] f2fs: avoid wrong f2fs_bug_on when truncating inline_data
> > 
> > This patch removes wrong f2fs_bug_on in truncate_inline_inode.
> > 
> > When there is no space, it can happen a corner case where i_isze is over
> > MAX_INLINE_SIZE while its inode is still inline_data.
> > 
> > The scenario is
> >  1. write small data into file #A.
> >  2. fill the whole partition to 100%.
> >  3. truncate 4096 on file #A.
> 
> If we truncate size over MAX_INLINE_DATA, we will convert inline data in
> f2fs_truncate rather than write_begin below. isn't it?

But, it fails to convert that due to -ENOSPC. :)

Thanks,

> 
> Thanks,
> 
> >  4. write data at 8192 offset.
> >   --> f2fs_write_begin
> >     -> -ENOSPC = f2fs_convert_inline_page
> >     -> f2fs_write_failed
> >       -> truncate_blocks
> >         -> truncate_inline_inode
> > 	  BUG_ON, since i_size is 4096.
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >  fs/f2fs/inline.c | 7 -------
> >  1 file changed, 7 deletions(-)
> > 
> > diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> > index 153c5e7..d3e0599 100644
> > --- a/fs/f2fs/inline.c
> > +++ b/fs/f2fs/inline.c
> > @@ -54,13 +54,6 @@ bool truncate_inline_inode(struct page *ipage, u64 from)
> >  {
> >  	void *addr;
> > 
> > -	/*
> > -	 * we should never truncate inline data past max inline data size,
> > -	 * because we always convert inline inode to normal one before
> > -	 * truncating real data if new size is past max inline data size.
> > -	 */
> > -	f2fs_bug_on(F2FS_P_SB(ipage), from > MAX_INLINE_DATA);
> > -
> >  	if (from >= MAX_INLINE_DATA)
> >  		return false;
> > 
> > --
> > 2.1.1
> > 
> > 
> > ------------------------------------------------------------------------------
> > Dive into the World of Parallel Programming The Go Parallel Website, sponsored
> > by Intel and developed in partnership with Slashdot Media, is your hub for all
> > things parallel software development, from weekly thought leadership blogs to
> > news, videos, case studies, tutorials and more. Take a look and join the
> > conversation now. http://goparallel.sourceforge.net/
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > Linux-f2fs-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

RE: [f2fs-dev] [PATCH 4/4] f2fs: avoid wrong f2fs_bug_on when truncating inline_data

[Chao Yu]

Hi Jeageuk,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Thursday, March 19, 2015 1:58 AM
> To: Chao Yu
> Cc: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH 4/4] f2fs: avoid wrong f2fs_bug_on when truncating inline_data
> 
> Hi,
> 
> On Wed, Mar 18, 2015 at 09:12:00AM +0800, Chao Yu wrote:
> > Hi Jaegeuk,
> >
> > > -----Original Message-----
> > > From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> > > Sent: Wednesday, March 18, 2015 12:58 AM
> > > To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> > > linux-f2fs-devel@lists.sourceforge.net
> > > Cc: Jaegeuk Kim
> > > Subject: [f2fs-dev] [PATCH 4/4] f2fs: avoid wrong f2fs_bug_on when truncating inline_data
> > >
> > > This patch removes wrong f2fs_bug_on in truncate_inline_inode.
> > >
> > > When there is no space, it can happen a corner case where i_isze is over
> > > MAX_INLINE_SIZE while its inode is still inline_data.
> > >
> > > The scenario is
> > >  1. write small data into file #A.
> > >  2. fill the whole partition to 100%.
> > >  3. truncate 4096 on file #A.
> >
> > If we truncate size over MAX_INLINE_DATA, we will convert inline data in
> > f2fs_truncate rather than write_begin below. isn't it?
> 
> But, it fails to convert that due to -ENOSPC. :)

That makes sense, :)

Reviewed-by: Chao Yu <chao2.yu@samsung.com>

Thank,