[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

[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

[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 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 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 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>

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,