[PATCH] f2fs: fix a problem of a truncated inmem page

[PATCH] f2fs: fix a problem of a truncated inmem page

[Yunlei He]

-start atomic write
   -set_page_dirty
      -register_inmem_page
				<--- truncate or punch hole
	  -commit atomic write

Here, may cause memory reclaim painc for page->mapping is null,
but private is -1.

Signed-off-by: Yunlei He <heyunlei@huawei.com>
---
 fs/f2fs/data.c    | 8 --------
 fs/f2fs/segment.c | 5 ++++-
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 1375fef..a91e479 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1948,10 +1948,6 @@ void f2fs_invalidate_page(struct page *page, unsigned int offset,
 		}
 	}
 
-	/* This is atomic written page, keep Private */
-	if (IS_ATOMIC_WRITTEN_PAGE(page))
-		return;
-
 	set_page_private(page, 0);
 	ClearPagePrivate(page);
 }
@@ -1962,10 +1958,6 @@ int f2fs_release_page(struct page *page, gfp_t wait)
 	if (PageDirty(page))
 		return 0;
 
-	/* This is atomic written page, keep Private */
-	if (IS_ATOMIC_WRITTEN_PAGE(page))
-		return 0;
-
 	set_page_private(page, 0);
 	ClearPagePrivate(page);
 	return 1;
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 42727e4..bebac9a 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -207,6 +207,9 @@ static int __revoke_inmem_pages(struct inode *inode,
 
 		lock_page(page);
 
+		if (page->mapping != inode->i_mapping)
+			goto delete;
+
 		if (recover) {
 			struct dnode_of_data dn;
 			struct node_info ni;
@@ -230,7 +233,7 @@ static int __revoke_inmem_pages(struct inode *inode,
 		set_page_private(page, 0);
 		ClearPagePrivate(page);
 		f2fs_put_page(page, 1);
-
+delete:
 		list_del(&cur->list);
 		kmem_cache_free(inmem_entry_slab, cur);
 		dec_page_count(F2FS_I_SB(inode), F2FS_INMEM_PAGES);
-- 
2.10.1


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

Re: [PATCH] f2fs: fix a problem of a truncated inmem page

[Jaegeuk Kim]

On 03/15, Yunlei He wrote:
> -start atomic write
>    -set_page_dirty
>       -register_inmem_page
> 				<--- truncate or punch hole

SQLite bug?

> 	  -commit atomic write
> 
> Here, may cause memory reclaim painc for page->mapping is null,
> but private is -1.
> 
> Signed-off-by: Yunlei He <heyunlei@huawei.com>
> ---
>  fs/f2fs/data.c    | 8 --------
>  fs/f2fs/segment.c | 5 ++++-
>  2 files changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 1375fef..a91e479 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1948,10 +1948,6 @@ void f2fs_invalidate_page(struct page *page, unsigned int offset,
>  		}
>  	}
>  
> -	/* This is atomic written page, keep Private */
> -	if (IS_ATOMIC_WRITTEN_PAGE(page))
> -		return;
> -

We need to give a kernel message like "atomic writes were broken", and
unresitering the page from the inmem page list here to keep consistency
between IS_ATOMIC_WRITTEN_PAGE(page) and its link to the inmem list.

>  	set_page_private(page, 0);
>  	ClearPagePrivate(page);
>  }
> @@ -1962,10 +1958,6 @@ int f2fs_release_page(struct page *page, gfp_t wait)
>  	if (PageDirty(page))
>  		return 0;
>  
> -	/* This is atomic written page, keep Private */
> -	if (IS_ATOMIC_WRITTEN_PAGE(page))
> -		return 0;

We should keep this in order not to be invalidated by someone.

Thanks,

> -
>  	set_page_private(page, 0);
>  	ClearPagePrivate(page);
>  	return 1;
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 42727e4..bebac9a 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -207,6 +207,9 @@ static int __revoke_inmem_pages(struct inode *inode,
>  
>  		lock_page(page);
>  
> +		if (page->mapping != inode->i_mapping)
> +			goto delete;
> +
>  		if (recover) {
>  			struct dnode_of_data dn;
>  			struct node_info ni;
> @@ -230,7 +233,7 @@ static int __revoke_inmem_pages(struct inode *inode,
>  		set_page_private(page, 0);
>  		ClearPagePrivate(page);
>  		f2fs_put_page(page, 1);
> -
> +delete:
>  		list_del(&cur->list);
>  		kmem_cache_free(inmem_entry_slab, cur);
>  		dec_page_count(F2FS_I_SB(inode), F2FS_INMEM_PAGES);
> -- 
> 2.10.1
> 
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

Re: [PATCH] f2fs: fix a problem of a truncated inmem page

[Jaegeuk Kim]

Hi Yunlei,

I could reproduce this. Could you check the patches that I sent just ago?

Thanks,

On 03/16, Jaegeuk Kim wrote:
> On 03/15, Yunlei He wrote:
> > -start atomic write
> >    -set_page_dirty
> >       -register_inmem_page
> > 				<--- truncate or punch hole
> 
> SQLite bug?
> 
> > 	  -commit atomic write
> > 
> > Here, may cause memory reclaim painc for page->mapping is null,
> > but private is -1.
> > 
> > Signed-off-by: Yunlei He <heyunlei@huawei.com>
> > ---
> >  fs/f2fs/data.c    | 8 --------
> >  fs/f2fs/segment.c | 5 ++++-
> >  2 files changed, 4 insertions(+), 9 deletions(-)
> > 
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 1375fef..a91e479 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -1948,10 +1948,6 @@ void f2fs_invalidate_page(struct page *page, unsigned int offset,
> >  		}
> >  	}
> >  
> > -	/* This is atomic written page, keep Private */
> > -	if (IS_ATOMIC_WRITTEN_PAGE(page))
> > -		return;
> > -
> 
> We need to give a kernel message like "atomic writes were broken", and
> unresitering the page from the inmem page list here to keep consistency
> between IS_ATOMIC_WRITTEN_PAGE(page) and its link to the inmem list.
> 
> >  	set_page_private(page, 0);
> >  	ClearPagePrivate(page);
> >  }
> > @@ -1962,10 +1958,6 @@ int f2fs_release_page(struct page *page, gfp_t wait)
> >  	if (PageDirty(page))
> >  		return 0;
> >  
> > -	/* This is atomic written page, keep Private */
> > -	if (IS_ATOMIC_WRITTEN_PAGE(page))
> > -		return 0;
> 
> We should keep this in order not to be invalidated by someone.
> 
> Thanks,
> 
> > -
> >  	set_page_private(page, 0);
> >  	ClearPagePrivate(page);
> >  	return 1;
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 42727e4..bebac9a 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -207,6 +207,9 @@ static int __revoke_inmem_pages(struct inode *inode,
> >  
> >  		lock_page(page);
> >  
> > +		if (page->mapping != inode->i_mapping)
> > +			goto delete;
> > +
> >  		if (recover) {
> >  			struct dnode_of_data dn;
> >  			struct node_info ni;
> > @@ -230,7 +233,7 @@ static int __revoke_inmem_pages(struct inode *inode,
> >  		set_page_private(page, 0);
> >  		ClearPagePrivate(page);
> >  		f2fs_put_page(page, 1);
> > -
> > +delete:
> >  		list_del(&cur->list);
> >  		kmem_cache_free(inmem_entry_slab, cur);
> >  		dec_page_count(F2FS_I_SB(inode), F2FS_INMEM_PAGES);
> > -- 
> > 2.10.1
> > 
> > 
> > ------------------------------------------------------------------------------
> > Check out the vibrant tech community on one of the world's most
> > engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > Linux-f2fs-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

Re: [PATCH] f2fs: fix a problem of a truncated inmem page

[heyunlei]

Hi Jaegeuk,

On 2017/3/17 10:11, Jaegeuk Kim wrote:
> Hi Yunlei,
>
> I could reproduce this. Could you check the patches that I sent just ago?

I have test it with my test case, it's OK.

Thanks,
>
> Thanks,
>
> On 03/16, Jaegeuk Kim wrote:
>> On 03/15, Yunlei He wrote:
>>> -start atomic write
>>>    -set_page_dirty
>>>       -register_inmem_page
>>> 				<--- truncate or punch hole
>>
>> SQLite bug?
>>
>>> 	  -commit atomic write
>>>
>>> Here, may cause memory reclaim painc for page->mapping is null,
>>> but private is -1.
>>>
>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>>> ---
>>>  fs/f2fs/data.c    | 8 --------
>>>  fs/f2fs/segment.c | 5 ++++-
>>>  2 files changed, 4 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>> index 1375fef..a91e479 100644
>>> --- a/fs/f2fs/data.c
>>> +++ b/fs/f2fs/data.c
>>> @@ -1948,10 +1948,6 @@ void f2fs_invalidate_page(struct page *page, unsigned int offset,
>>>  		}
>>>  	}
>>>
>>> -	/* This is atomic written page, keep Private */
>>> -	if (IS_ATOMIC_WRITTEN_PAGE(page))
>>> -		return;
>>> -
>>
>> We need to give a kernel message like "atomic writes were broken", and
>> unresitering the page from the inmem page list here to keep consistency
>> between IS_ATOMIC_WRITTEN_PAGE(page) and its link to the inmem list.
>>
>>>  	set_page_private(page, 0);
>>>  	ClearPagePrivate(page);
>>>  }
>>> @@ -1962,10 +1958,6 @@ int f2fs_release_page(struct page *page, gfp_t wait)
>>>  	if (PageDirty(page))
>>>  		return 0;
>>>
>>> -	/* This is atomic written page, keep Private */
>>> -	if (IS_ATOMIC_WRITTEN_PAGE(page))
>>> -		return 0;
>>
>> We should keep this in order not to be invalidated by someone.
>>
>> Thanks,
>>
>>> -
>>>  	set_page_private(page, 0);
>>>  	ClearPagePrivate(page);
>>>  	return 1;
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index 42727e4..bebac9a 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -207,6 +207,9 @@ static int __revoke_inmem_pages(struct inode *inode,
>>>
>>>  		lock_page(page);
>>>
>>> +		if (page->mapping != inode->i_mapping)
>>> +			goto delete;
>>> +
>>>  		if (recover) {
>>>  			struct dnode_of_data dn;
>>>  			struct node_info ni;
>>> @@ -230,7 +233,7 @@ static int __revoke_inmem_pages(struct inode *inode,
>>>  		set_page_private(page, 0);
>>>  		ClearPagePrivate(page);
>>>  		f2fs_put_page(page, 1);
>>> -
>>> +delete:
>>>  		list_del(&cur->list);
>>>  		kmem_cache_free(inmem_entry_slab, cur);
>>>  		dec_page_count(F2FS_I_SB(inode), F2FS_INMEM_PAGES);
>>> --
>>> 2.10.1
>>>
>>>
>>> ------------------------------------------------------------------------------
>>> Check out the vibrant tech community on one of the world's most
>>> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
>>> _______________________________________________
>>> Linux-f2fs-devel mailing list
>>> Linux-f2fs-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>
>> ------------------------------------------------------------------------------
>> Check out the vibrant tech community on one of the world's most
>> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> Linux-f2fs-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>
> .
>


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