[PATCH] f2fs: avoid migratepage for atomic written page

[PATCH] f2fs: avoid migratepage for atomic written page

[Jaegeuk Kim]

In order to avoid lock contention for atomic written pages, we'd better give
EAGAIN in f2fs_migrate_page. We expect it will be released soon as transaction
commits.

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

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index d58b81213a86..1458e3a6d630 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2197,41 +2197,26 @@ static sector_t f2fs_bmap(struct address_space *mapping, sector_t block)
 int f2fs_migrate_page(struct address_space *mapping,
 		struct page *newpage, struct page *page, enum migrate_mode mode)
 {
-	int rc, extra_count;
-	struct f2fs_inode_info *fi = F2FS_I(mapping->host);
-	bool atomic_written = IS_ATOMIC_WRITTEN_PAGE(page);
+	int rc;
 
-	BUG_ON(PageWriteback(page));
-
-	/* migrating an atomic written page is safe with the inmem_lock hold */
-	if (atomic_written && !mutex_trylock(&fi->inmem_lock))
+	/*
+	 * We'd better return EAGAIN for atomic pages, which will be committed
+	 * sooner or later. Don't botter transactions with inmem_lock.
+	 */
+	if (IS_ATOMIC_WRITTEN_PAGE(page))
 		return -EAGAIN;
 
+	BUG_ON(PageWriteback(page));	/* Writeback must be complete */
+
 	/*
 	 * A reference is expected if PagePrivate set when move mapping,
 	 * however F2FS breaks this for maintaining dirty page counts when
 	 * truncating pages. So here adjusting the 'extra_count' make it work.
 	 */
-	extra_count = (atomic_written ? 1 : 0) - page_has_private(page);
 	rc = migrate_page_move_mapping(mapping, newpage,
-				page, NULL, mode, extra_count);
-	if (rc != MIGRATEPAGE_SUCCESS) {
-		if (atomic_written)
-			mutex_unlock(&fi->inmem_lock);
+			page, NULL, mode, (page_has_private(page) ? -1 : 0));
+	if (rc != MIGRATEPAGE_SUCCESS)
 		return rc;
-	}
-
-	if (atomic_written) {
-		struct inmem_pages *cur;
-		list_for_each_entry(cur, &fi->inmem_pages, list)
-			if (cur->page == page) {
-				cur->page = newpage;
-				break;
-			}
-		mutex_unlock(&fi->inmem_lock);
-		put_page(page);
-		get_page(newpage);
-	}
 
 	if (PagePrivate(page))
 		SetPagePrivate(newpage);
-- 
2.13.0.rc1.294.g07d810a77f-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [f2fs-dev] [PATCH] f2fs: avoid migratepage for atomic written page

[Chao Yu]

Hi Jaegeuk,

On 2017/7/4 7:08, Jaegeuk Kim wrote:
> In order to avoid lock contention for atomic written pages, we'd better give
> EAGAIN in f2fs_migrate_page. We expect it will be released soon as transaction
> commits.

Hmm.. if atomic write is triggered intensively, there is little change to
migrate fragmented page.

How about detecting migrate mode here, for MIGRATE_SYNC case, let it moving
the page; for MIGRATE_ASYNC/MIGRATE_SYNC_LIGHT case, the migration priority
is lower, we can return EAGAIN.

Thanks,

> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/data.c | 35 ++++++++++-------------------------
>  1 file changed, 10 insertions(+), 25 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index d58b81213a86..1458e3a6d630 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -2197,41 +2197,26 @@ static sector_t f2fs_bmap(struct address_space *mapping, sector_t block)
>  int f2fs_migrate_page(struct address_space *mapping,
>  		struct page *newpage, struct page *page, enum migrate_mode mode)
>  {
> -	int rc, extra_count;
> -	struct f2fs_inode_info *fi = F2FS_I(mapping->host);
> -	bool atomic_written = IS_ATOMIC_WRITTEN_PAGE(page);
> +	int rc;
>  
> -	BUG_ON(PageWriteback(page));
> -
> -	/* migrating an atomic written page is safe with the inmem_lock hold */
> -	if (atomic_written && !mutex_trylock(&fi->inmem_lock))
> +	/*
> +	 * We'd better return EAGAIN for atomic pages, which will be committed
> +	 * sooner or later. Don't botter transactions with inmem_lock.
> +	 */
> +	if (IS_ATOMIC_WRITTEN_PAGE(page))
>  		return -EAGAIN;
>  
> +	BUG_ON(PageWriteback(page));	/* Writeback must be complete */
> +
>  	/*
>  	 * A reference is expected if PagePrivate set when move mapping,
>  	 * however F2FS breaks this for maintaining dirty page counts when
>  	 * truncating pages. So here adjusting the 'extra_count' make it work.
>  	 */
> -	extra_count = (atomic_written ? 1 : 0) - page_has_private(page);
>  	rc = migrate_page_move_mapping(mapping, newpage,
> -				page, NULL, mode, extra_count);
> -	if (rc != MIGRATEPAGE_SUCCESS) {
> -		if (atomic_written)
> -			mutex_unlock(&fi->inmem_lock);
> +			page, NULL, mode, (page_has_private(page) ? -1 : 0));
> +	if (rc != MIGRATEPAGE_SUCCESS)
>  		return rc;
> -	}
> -
> -	if (atomic_written) {
> -		struct inmem_pages *cur;
> -		list_for_each_entry(cur, &fi->inmem_pages, list)
> -			if (cur->page == page) {
> -				cur->page = newpage;
> -				break;
> -			}
> -		mutex_unlock(&fi->inmem_lock);
> -		put_page(page);
> -		get_page(newpage);
> -	}
>  
>  	if (PagePrivate(page))
>  		SetPagePrivate(newpage);
> 

Re: [PATCH v2] f2fs: relax migratepage for atomic written page

[Jaegeuk Kim]

In order to avoid lock contention for atomic written pages, we'd better give
EBUSY in f2fs_migrate_page when mode is asynchronous. We expect it will be
released soon as transaction commits.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---

Change log from v1:
 - return -EBUSY when mode is MIGRATE_ASYNC or MIGRATE_SYNC_LIGHT

 fs/f2fs/data.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 85ac3a63cce6..74c31cb203e2 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2206,8 +2206,12 @@ int f2fs_migrate_page(struct address_space *mapping,
 	BUG_ON(PageWriteback(page));
 
 	/* migrating an atomic written page is safe with the inmem_lock hold */
-	if (atomic_written && !mutex_trylock(&fi->inmem_lock))
-		return -EAGAIN;
+	if (atomic_written) {
+		if (mode != MIGRATE_SYNC)
+			return -EBUSY;
+		if (!mutex_trylock(&fi->inmem_lock))
+			return -EAGAIN;
+	}
 
 	/*
 	 * A reference is expected if PagePrivate set when move mapping,
-- 
2.13.0.rc1.294.g07d810a77f-goog

Re: [PATCH v2] f2fs: relax migratepage for atomic written page

[Chao Yu]

On 2017/7/7 5:49, Jaegeuk Kim wrote:
> In order to avoid lock contention for atomic written pages, we'd better give
> EBUSY in f2fs_migrate_page when mode is asynchronous. We expect it will be
> released soon as transaction commits.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

> ---
> 
> Change log from v1:
>  - return -EBUSY when mode is MIGRATE_ASYNC or MIGRATE_SYNC_LIGHT
> 
>  fs/f2fs/data.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 85ac3a63cce6..74c31cb203e2 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -2206,8 +2206,12 @@ int f2fs_migrate_page(struct address_space *mapping,
>  	BUG_ON(PageWriteback(page));
>  
>  	/* migrating an atomic written page is safe with the inmem_lock hold */
> -	if (atomic_written && !mutex_trylock(&fi->inmem_lock))
> -		return -EAGAIN;
> +	if (atomic_written) {
> +		if (mode != MIGRATE_SYNC)
> +			return -EBUSY;
> +		if (!mutex_trylock(&fi->inmem_lock))
> +			return -EAGAIN;
> +	}
>  
>  	/*
>  	 * A reference is expected if PagePrivate set when move mapping,
> 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot