RE: [PATCH] f2fs: fix to truncate inline data past EOF

2015-03-09 Thread Chao Yu
Hi Jaegeuk,

> -Original Message-
> From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
> Sent: Tuesday, March 10, 2015 11:00 AM
> To: Chao Yu
> Cc: 'Changman Lee'; linux-f2fs-de...@lists.sourceforge.net; 
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] f2fs: fix to truncate inline data past EOF
> 
> Hi Chao,
> 
> On Tue, Mar 10, 2015 at 10:02:46AM +0800, Chao Yu wrote:
> > Hi Jaegeuk,
> >
> 
> [snip]
> 
> > > > > +static int truncate_partial_data_page(struct inode *inode, u64 from, 
> > > > > bool force)
> > > > >  {
> > > > >   unsigned offset = from & (PAGE_CACHE_SIZE - 1);
> > > > >   struct page *page;
> > > > >
> > > > > - if (!offset)
> > > > > + if (!offset && !force)
> > > >
> > > > We truncate on-disk inode page and #0 page separately, so IMO there is 
> > > > a very
> > > > small chance the following issue will occur.
> > > >
> > > > (0 < truncated_size < MAX_INLINE_SIZE)
> > > > ->f2fs_setattr
> > > >   ->truncate_setsize #0 page has been truncated partially
> > > >   ->f2fs_truncate
> > > > ->truncate_blocks
> > > > invalidate #0 page by reclaimer
> > > > update #0 page with original 
> > > > data in inode page by reader
> > >
> > >   truncate_setsize called 
> > > truncate_pagesize.
> > >   so #0 page was truncated successfully.
> >
> > Yeah, but it's partially truncating as 0 < truncated_size < MAX_INLINE_SIZE,
> > After truncating, we still keep [0, truncated_size) valid data in #0 page, 
> > and
> > our #0 page status is uptodate | !dirty.
> >
> > So what I mean is that mm can reclaim this #0 page, then if someone else 
> > read
> > the #0 page again, we will update the #0 page with original data in inode 
> > page
> > since inode page haven't truncated yet.
> 
> The truncate_blocks dropped inline_data in inode page, which is modified by
> this patch.
> And, the cached #0 page was truncated by truncate_setsize.
> Even if this page was evicted and reloaded again, the data would be filled 
> with
> the inode page having truncated inline_data.
> So, it seems there is no problem.
> 
> BTW, do you mean like this scenario?

Yeah, actually it is. ;-)

> 
> -> f2fs_setattr
>  ->truncate_setsize: #0 page has been truncated partially
>  ->f2fs_truncate
>   invalidate #0 page by reclaimer
>   update #0 page with original data in 
> inode page by reader
> 
> ->truncate_blocks
>   (*)-> truncate_inline_inode
> (*)-> truncate_partial_data_page(,, force)
>find_data_page(,, force)  <- we can use *force* here.
> 
> Then, I agree that two functions as noted (*) above would be necessary.
> 
> >
> > If this happened, if we don't truncate #0 page in following code of 
> > truncate_blocks,
> > we will remain the original data for user, it's wrong.
> >
> > IMO, it's better to truncate inode page and #0 page together, or truncate 
> > #0 page
> > in truncate_partial_data_page.
> >
> > How do you think?
> >
> > >
> > > >   ->truncate_inline_inode
> > > >   ->truncate_partial_data_page
> > > > we will fail to truncate #0 page because we can't find valid 
> > > > blkaddr for
> > > > #0 page in find_data_page(,,false)
> > > >
> > > > How about using find_data_page(,,true) to check weather this page is 
> > > > update or not
> > > > for this condition.
> > >
> > > Oh, I realized that we don't need to call truncate_partial_data_page, 
> > > since the
> > > cached #0 page was truncated already. We don't care about that.
> >
> > IMO, we should care about this #0 page if above example can happen.
> >
> > > So, do we need to add just truncate_inline_inode() like below?
> > >
> > > >
> > > > >   return 0;
> > > > >
> > > > >   page = find_data_page(inode, from >> PAGE_CACHE_SHIFT, false);
> > > > > @@ -489,6 +489,7 @@ int truncate_blocks(struct inode *inode, u64 
> > > > > from, bool lock)
> > > > >  

Re: [PATCH] f2fs: fix to truncate inline data past EOF

2015-03-09 Thread Jaegeuk Kim
Hi Chao,

On Tue, Mar 10, 2015 at 10:02:46AM +0800, Chao Yu wrote:
> Hi Jaegeuk,
> 

[snip]

> > > > +static int truncate_partial_data_page(struct inode *inode, u64 from, 
> > > > bool force)
> > > >  {
> > > > unsigned offset = from & (PAGE_CACHE_SIZE - 1);
> > > > struct page *page;
> > > >
> > > > -   if (!offset)
> > > > +   if (!offset && !force)
> > >
> > > We truncate on-disk inode page and #0 page separately, so IMO there is a 
> > > very
> > > small chance the following issue will occur.
> > >
> > > (0 < truncated_size < MAX_INLINE_SIZE)
> > > ->f2fs_setattr
> > >   ->truncate_setsize #0 page has been truncated partially
> > >   ->f2fs_truncate
> > > ->truncate_blocks
> > >   invalidate #0 page by reclaimer
> > >   update #0 page with original data in 
> > > inode page by reader
> > 
> > truncate_setsize called 
> > truncate_pagesize.
> > so #0 page was truncated successfully.
> 
> Yeah, but it's partially truncating as 0 < truncated_size < MAX_INLINE_SIZE,
> After truncating, we still keep [0, truncated_size) valid data in #0 page, and
> our #0 page status is uptodate | !dirty.
> 
> So what I mean is that mm can reclaim this #0 page, then if someone else read
> the #0 page again, we will update the #0 page with original data in inode page
> since inode page haven't truncated yet.

The truncate_blocks dropped inline_data in inode page, which is modified by
this patch.
And, the cached #0 page was truncated by truncate_setsize.
Even if this page was evicted and reloaded again, the data would be filled with
the inode page having truncated inline_data.
So, it seems there is no problem.

BTW, do you mean like this scenario?

-> f2fs_setattr
 ->truncate_setsize: #0 page has been truncated partially
 ->f2fs_truncate
invalidate #0 page by reclaimer
update #0 page with original data in 
inode page by reader

->truncate_blocks
  (*)-> truncate_inline_inode
(*)-> truncate_partial_data_page(,, force)
   find_data_page(,, force)  <- we can use *force* here.

Then, I agree that two functions as noted (*) above would be necessary.

> 
> If this happened, if we don't truncate #0 page in following code of 
> truncate_blocks,
> we will remain the original data for user, it's wrong.
> 
> IMO, it's better to truncate inode page and #0 page together, or truncate #0 
> page
> in truncate_partial_data_page.
> 
> How do you think?
> 
> > 
> > >   ->truncate_inline_inode
> > >   ->truncate_partial_data_page
> > > we will fail to truncate #0 page because we can't find valid 
> > > blkaddr for
> > > #0 page in find_data_page(,,false)
> > >
> > > How about using find_data_page(,,true) to check weather this page is 
> > > update or not
> > > for this condition.
> > 
> > Oh, I realized that we don't need to call truncate_partial_data_page, since 
> > the
> > cached #0 page was truncated already. We don't care about that.
> 
> IMO, we should care about this #0 page if above example can happen.
> 
> > So, do we need to add just truncate_inline_inode() like below?
> > 
> > >
> > > > return 0;
> > > >
> > > > page = find_data_page(inode, from >> PAGE_CACHE_SHIFT, false);
> > > > @@ -489,6 +489,7 @@ int truncate_blocks(struct inode *inode, u64 from, 
> > > > bool lock)
> > > > pgoff_t free_from;
> > > > int count = 0, err = 0;
> > > > struct page *ipage;
> > > > +   bool truncate_page = false;
> > > >
> > > > trace_f2fs_truncate_blocks_enter(inode, from);
> > > >
> > > > @@ -504,6 +505,9 @@ int truncate_blocks(struct inode *inode, u64 from, 
> > > > bool lock)
> > > > }
> > > >
> > > > if (f2fs_has_inline_data(inode)) {
> > > > +   truncate_inline_inode(ipage, from);
> > > > +   set_page_dirty(ipage);
> > >
> > > If @from is the same as MAX_INLINE_DATA, we will change nothing in inode 
> > > page,
> > > How about skipping set_page_dirty for inode page in this condition?
> > 
> > Agreed.
> > How about adding this in truncate_inline_inode?
> > 
> > if (from >= MAX_INLINE_DATA)
> > return;
> > ...
> > set_page_dirty(ipage);
> 
> Yeah, that's good.
>  
> And What I suggest is:
> 
> bool truncate_inline_inode(struct page *ipage, u64 from)
> {
>   /*
>* 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)

if (from >= MAX_INLINE_DATA)  to handle when f2fs_bug_on is bypassed.

>   return false;
>   ...
>   return true;
> }
> 
> in 

RE: [PATCH] f2fs: fix to truncate inline data past EOF

2015-03-09 Thread Chao Yu
Hi Jaegeuk,

> -Original Message-
> From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
> Sent: Tuesday, March 10, 2015 12:16 AM
> To: Chao Yu
> Cc: 'Changman Lee'; linux-f2fs-de...@lists.sourceforge.net; 
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] f2fs: fix to truncate inline data past EOF
> 
> Hi Chao,
> 
> On Mon, Mar 09, 2015 at 04:30:32PM +0800, Chao Yu wrote:
> > Hi Jaegeuk,
> >
> > > -Original Message-
> > > From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
> > > Sent: Monday, March 09, 2015 11:49 AM
> > > To: Chao Yu
> > > Cc: 'Changman Lee'; linux-f2fs-de...@lists.sourceforge.net; 
> > > linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH] f2fs: fix to truncate inline data past EOF
> > >
> > > Hi Chao,
> > >
> > >
> > > On Mon, Mar 09, 2015 at 10:24:35AM +0800, Chao Yu wrote:
> > > > Hi Jaegeuk,
> > > >
> > > > > -Original Message-
> > > > > From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
> > > > > Sent: Friday, March 06, 2015 5:34 AM
> > > > > To: Chao Yu
> > > > > Cc: Changman Lee; linux-f2fs-de...@lists.sourceforge.net; 
> > > > > linux-kernel@vger.kernel.org
> > > > > Subject: Re: [PATCH] f2fs: fix to truncate inline data past EOF
> > > > >
> > > > > Hi Chao,
> > > > >
> > > > > On Thu, Mar 05, 2015 at 05:41:32PM +0800, Chao Yu wrote:
> > > > > > Previously if inode is with inline data, we will try to invalid 
> > > > > > partial inline
> > > > > > data in page #0 when we truncate size of inode in 
> > > > > > truncate_partial_data_page().
> > > > > > And then we set page #0 to dirty, after this we can synchronize 
> > > > > > inode page with
> > > > > > page #0 at ->writepage().
> > > > > >
> > > > > > But sometimes we will fail to operate page #0 in 
> > > > > > truncate_partial_data_page()
> > > > > > due to below reason:
> > > > > > a) if offset is zero, we will skip setting page #0 to dirty.
> > > > > > b) if page #0 is not uptodate, we will fail to update it as it has 
> > > > > > no mapping
> > > > > > data.
> > > > > >
> > > > > > So with following operations, we will meet recent data which should 
> > > > > > be
> > > > > > truncated.
> > > > > >
> > > > > > 1.write inline data to file
> > > > > > 2.sync first data page to inode page
> > > > > > 3.truncate file size to 0
> > > > > > 4.truncate file size to max_inline_size
> > > > > > 5.echo 1 > /proc/sys/vm/drop_caches
> > > > > > 6.read file --> meet original inline data which is remained in 
> > > > > > inode page.
> > > > > >
> > > > > > This patch renames truncate_inline_data() to 
> > > > > > truncate_inline_inode() for code
> > > > > > readability, then intruduces new truncate_inline_data() and use it 
> > > > > > to truncate
> > > > > > inline data in page #0 and inode page in 
> > > > > > truncate_partial_data_page() for
> > > > > > fixing.
> > > > > >
> > > > > > Signed-off-by: Chao Yu 
> > > > > > ---
> > > > > >  fs/f2fs/f2fs.h   |  1 +
> > > > > >  fs/f2fs/file.c   | 13 -
> > > > > >  fs/f2fs/inline.c | 57 
> > > > > > +++-
> > > > > >  3 files changed, 61 insertions(+), 10 deletions(-)
> > > > > >
> > > > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > > > > index 511d6cd..55dbbe7 100644
> > > > > > --- a/fs/f2fs/f2fs.h
> > > > > > +++ b/fs/f2fs/f2fs.h
> > > > > > @@ -1754,6 +1754,7 @@ extern struct kmem_cache *inode_entry_slab;
> > > > > >   */
> > > > > >  bool f2fs_may_inline(struct inode *);
> > > > > >  void read_inline_data(struct page *, struct page *);
> > > > > > +int truncate_inline_data(struct inode *, u64 from);
> > > > > >  int f2fs_read_inline_data(struct inode *, struct page *);
> > > > > >

Re: [PATCH] f2fs: fix to truncate inline data past EOF

2015-03-09 Thread Jaegeuk Kim
Hi Chao,

On Mon, Mar 09, 2015 at 04:30:32PM +0800, Chao Yu wrote:
> Hi Jaegeuk,
> 
> > -Original Message-
> > From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
> > Sent: Monday, March 09, 2015 11:49 AM
> > To: Chao Yu
> > Cc: 'Changman Lee'; linux-f2fs-de...@lists.sourceforge.net; 
> > linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH] f2fs: fix to truncate inline data past EOF
> > 
> > Hi Chao,
> > 
> > 
> > On Mon, Mar 09, 2015 at 10:24:35AM +0800, Chao Yu wrote:
> > > Hi Jaegeuk,
> > >
> > > > -Original Message-
> > > > From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
> > > > Sent: Friday, March 06, 2015 5:34 AM
> > > > To: Chao Yu
> > > > Cc: Changman Lee; linux-f2fs-de...@lists.sourceforge.net; 
> > > > linux-kernel@vger.kernel.org
> > > > Subject: Re: [PATCH] f2fs: fix to truncate inline data past EOF
> > > >
> > > > Hi Chao,
> > > >
> > > > On Thu, Mar 05, 2015 at 05:41:32PM +0800, Chao Yu wrote:
> > > > > Previously if inode is with inline data, we will try to invalid 
> > > > > partial inline
> > > > > data in page #0 when we truncate size of inode in 
> > > > > truncate_partial_data_page().
> > > > > And then we set page #0 to dirty, after this we can synchronize inode 
> > > > > page with
> > > > > page #0 at ->writepage().
> > > > >
> > > > > But sometimes we will fail to operate page #0 in 
> > > > > truncate_partial_data_page()
> > > > > due to below reason:
> > > > > a) if offset is zero, we will skip setting page #0 to dirty.
> > > > > b) if page #0 is not uptodate, we will fail to update it as it has no 
> > > > > mapping
> > > > > data.
> > > > >
> > > > > So with following operations, we will meet recent data which should be
> > > > > truncated.
> > > > >
> > > > > 1.write inline data to file
> > > > > 2.sync first data page to inode page
> > > > > 3.truncate file size to 0
> > > > > 4.truncate file size to max_inline_size
> > > > > 5.echo 1 > /proc/sys/vm/drop_caches
> > > > > 6.read file --> meet original inline data which is remained in inode 
> > > > > page.
> > > > >
> > > > > This patch renames truncate_inline_data() to truncate_inline_inode() 
> > > > > for code
> > > > > readability, then intruduces new truncate_inline_data() and use it to 
> > > > > truncate
> > > > > inline data in page #0 and inode page in truncate_partial_data_page() 
> > > > > for
> > > > > fixing.
> > > > >
> > > > > Signed-off-by: Chao Yu 
> > > > > ---
> > > > >  fs/f2fs/f2fs.h   |  1 +
> > > > >  fs/f2fs/file.c   | 13 -
> > > > >  fs/f2fs/inline.c | 57 
> > > > > +++-
> > > > >  3 files changed, 61 insertions(+), 10 deletions(-)
> > > > >
> > > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > > > index 511d6cd..55dbbe7 100644
> > > > > --- a/fs/f2fs/f2fs.h
> > > > > +++ b/fs/f2fs/f2fs.h
> > > > > @@ -1754,6 +1754,7 @@ extern struct kmem_cache *inode_entry_slab;
> > > > >   */
> > > > >  bool f2fs_may_inline(struct inode *);
> > > > >  void read_inline_data(struct page *, struct page *);
> > > > > +int truncate_inline_data(struct inode *, u64 from);
> > > > >  int f2fs_read_inline_data(struct inode *, struct page *);
> > > > >  int f2fs_convert_inline_page(struct dnode_of_data *, struct page *);
> > > > >  int f2fs_convert_inline_inode(struct inode *);
> > > > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > > > > index f1341c7..6c4443e 100644
> > > > > --- a/fs/f2fs/file.c
> > > > > +++ b/fs/f2fs/file.c
> > > > > @@ -461,6 +461,9 @@ static int truncate_partial_data_page(struct 
> > > > > inode *inode, u64 from)
> > > > >   unsigned offset = from & (PAGE_CACHE_SIZE - 1);
> > > > >   struct page *page;
> > > > >
> > > > > + if (f2fs_has_inline_data(inode))
> > > > > + return 0;
> > &g

RE: [PATCH] f2fs: fix to truncate inline data past EOF

2015-03-09 Thread Chao Yu
Hi Jaegeuk,

> -Original Message-
> From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
> Sent: Monday, March 09, 2015 11:49 AM
> To: Chao Yu
> Cc: 'Changman Lee'; linux-f2fs-de...@lists.sourceforge.net; 
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] f2fs: fix to truncate inline data past EOF
> 
> Hi Chao,
> 
> 
> On Mon, Mar 09, 2015 at 10:24:35AM +0800, Chao Yu wrote:
> > Hi Jaegeuk,
> >
> > > -Original Message-
> > > From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
> > > Sent: Friday, March 06, 2015 5:34 AM
> > > To: Chao Yu
> > > Cc: Changman Lee; linux-f2fs-de...@lists.sourceforge.net; 
> > > linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH] f2fs: fix to truncate inline data past EOF
> > >
> > > Hi Chao,
> > >
> > > On Thu, Mar 05, 2015 at 05:41:32PM +0800, Chao Yu wrote:
> > > > Previously if inode is with inline data, we will try to invalid partial 
> > > > inline
> > > > data in page #0 when we truncate size of inode in 
> > > > truncate_partial_data_page().
> > > > And then we set page #0 to dirty, after this we can synchronize inode 
> > > > page with
> > > > page #0 at ->writepage().
> > > >
> > > > But sometimes we will fail to operate page #0 in 
> > > > truncate_partial_data_page()
> > > > due to below reason:
> > > > a) if offset is zero, we will skip setting page #0 to dirty.
> > > > b) if page #0 is not uptodate, we will fail to update it as it has no 
> > > > mapping
> > > > data.
> > > >
> > > > So with following operations, we will meet recent data which should be
> > > > truncated.
> > > >
> > > > 1.write inline data to file
> > > > 2.sync first data page to inode page
> > > > 3.truncate file size to 0
> > > > 4.truncate file size to max_inline_size
> > > > 5.echo 1 > /proc/sys/vm/drop_caches
> > > > 6.read file --> meet original inline data which is remained in inode 
> > > > page.
> > > >
> > > > This patch renames truncate_inline_data() to truncate_inline_inode() 
> > > > for code
> > > > readability, then intruduces new truncate_inline_data() and use it to 
> > > > truncate
> > > > inline data in page #0 and inode page in truncate_partial_data_page() 
> > > > for
> > > > fixing.
> > > >
> > > > Signed-off-by: Chao Yu 
> > > > ---
> > > >  fs/f2fs/f2fs.h   |  1 +
> > > >  fs/f2fs/file.c   | 13 -
> > > >  fs/f2fs/inline.c | 57 
> > > > +++-
> > > >  3 files changed, 61 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > > index 511d6cd..55dbbe7 100644
> > > > --- a/fs/f2fs/f2fs.h
> > > > +++ b/fs/f2fs/f2fs.h
> > > > @@ -1754,6 +1754,7 @@ extern struct kmem_cache *inode_entry_slab;
> > > >   */
> > > >  bool f2fs_may_inline(struct inode *);
> > > >  void read_inline_data(struct page *, struct page *);
> > > > +int truncate_inline_data(struct inode *, u64 from);
> > > >  int f2fs_read_inline_data(struct inode *, struct page *);
> > > >  int f2fs_convert_inline_page(struct dnode_of_data *, struct page *);
> > > >  int f2fs_convert_inline_inode(struct inode *);
> > > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > > > index f1341c7..6c4443e 100644
> > > > --- a/fs/f2fs/file.c
> > > > +++ b/fs/f2fs/file.c
> > > > @@ -461,6 +461,9 @@ static int truncate_partial_data_page(struct inode 
> > > > *inode, u64 from)
> > > > unsigned offset = from & (PAGE_CACHE_SIZE - 1);
> > > > struct page *page;
> > > >
> > > > +   if (f2fs_has_inline_data(inode))
> > > > +   return 0;
> > > > +
> > > > if (!offset)
> > > > return 0;
> > > >
> > > > @@ -497,14 +500,14 @@ int truncate_blocks(struct inode *inode, u64 
> > > > from, bool lock)
> > > > if (lock)
> > > > f2fs_lock_op(sbi);
> > > >
> > > > -   ipage = get_node_page(sbi, inode->i_ino);
> > > > -   if (IS_ERR(ipage)) {
> > > > -   err 

Re: [PATCH] f2fs: fix to truncate inline data past EOF

2015-03-09 Thread Jaegeuk Kim
Hi Chao,

On Mon, Mar 09, 2015 at 04:30:32PM +0800, Chao Yu wrote:
 Hi Jaegeuk,
 
  -Original Message-
  From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
  Sent: Monday, March 09, 2015 11:49 AM
  To: Chao Yu
  Cc: 'Changman Lee'; linux-f2fs-de...@lists.sourceforge.net; 
  linux-kernel@vger.kernel.org
  Subject: Re: [PATCH] f2fs: fix to truncate inline data past EOF
  
  Hi Chao,
  
  
  On Mon, Mar 09, 2015 at 10:24:35AM +0800, Chao Yu wrote:
   Hi Jaegeuk,
  
-Original Message-
From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
Sent: Friday, March 06, 2015 5:34 AM
To: Chao Yu
Cc: Changman Lee; linux-f2fs-de...@lists.sourceforge.net; 
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] f2fs: fix to truncate inline data past EOF
   
Hi Chao,
   
On Thu, Mar 05, 2015 at 05:41:32PM +0800, Chao Yu wrote:
 Previously if inode is with inline data, we will try to invalid 
 partial inline
 data in page #0 when we truncate size of inode in 
 truncate_partial_data_page().
 And then we set page #0 to dirty, after this we can synchronize inode 
 page with
 page #0 at -writepage().

 But sometimes we will fail to operate page #0 in 
 truncate_partial_data_page()
 due to below reason:
 a) if offset is zero, we will skip setting page #0 to dirty.
 b) if page #0 is not uptodate, we will fail to update it as it has no 
 mapping
 data.

 So with following operations, we will meet recent data which should be
 truncated.

 1.write inline data to file
 2.sync first data page to inode page
 3.truncate file size to 0
 4.truncate file size to max_inline_size
 5.echo 1  /proc/sys/vm/drop_caches
 6.read file -- meet original inline data which is remained in inode 
 page.

 This patch renames truncate_inline_data() to truncate_inline_inode() 
 for code
 readability, then intruduces new truncate_inline_data() and use it to 
 truncate
 inline data in page #0 and inode page in truncate_partial_data_page() 
 for
 fixing.

 Signed-off-by: Chao Yu chao2...@samsung.com
 ---
  fs/f2fs/f2fs.h   |  1 +
  fs/f2fs/file.c   | 13 -
  fs/f2fs/inline.c | 57 
 +++-
  3 files changed, 61 insertions(+), 10 deletions(-)

 diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
 index 511d6cd..55dbbe7 100644
 --- a/fs/f2fs/f2fs.h
 +++ b/fs/f2fs/f2fs.h
 @@ -1754,6 +1754,7 @@ extern struct kmem_cache *inode_entry_slab;
   */
  bool f2fs_may_inline(struct inode *);
  void read_inline_data(struct page *, struct page *);
 +int truncate_inline_data(struct inode *, u64 from);
  int f2fs_read_inline_data(struct inode *, struct page *);
  int f2fs_convert_inline_page(struct dnode_of_data *, struct page *);
  int f2fs_convert_inline_inode(struct inode *);
 diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
 index f1341c7..6c4443e 100644
 --- a/fs/f2fs/file.c
 +++ b/fs/f2fs/file.c
 @@ -461,6 +461,9 @@ static int truncate_partial_data_page(struct 
 inode *inode, u64 from)
   unsigned offset = from  (PAGE_CACHE_SIZE - 1);
   struct page *page;

 + if (f2fs_has_inline_data(inode))
 + return 0;
 +
   if (!offset)
   return 0;

 @@ -497,14 +500,14 @@ int truncate_blocks(struct inode *inode, u64 
 from, bool lock)
   if (lock)
   f2fs_lock_op(sbi);

 - ipage = get_node_page(sbi, inode-i_ino);
 - if (IS_ERR(ipage)) {
 - err = PTR_ERR(ipage);
 + if (f2fs_has_inline_data(inode)) {
 + err = truncate_inline_data(inode, from);
   goto out;
   }

 - if (f2fs_has_inline_data(inode)) {
 - f2fs_put_page(ipage, 1);
 + ipage = get_node_page(sbi, inode-i_ino);
 + if (IS_ERR(ipage)) {
 + err = PTR_ERR(ipage);
   goto out;
   }

 diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
 index 4ba9732..a8189c92 100644
 --- a/fs/f2fs/inline.c
 +++ b/fs/f2fs/inline.c
 @@ -50,10 +50,57 @@ void read_inline_data(struct page *page, struct 
 page *ipage)
   SetPageUptodate(page);
  }

 -static void truncate_inline_data(struct page *ipage)
 +static void truncate_inline_inode(struct page *ipage, u64 from)
  {
 + void *addr = inline_data_addr(ipage);
 +
   f2fs_wait_on_page_writeback(ipage, NODE);
 - memset(inline_data_addr(ipage), 0, MAX_INLINE_DATA);
 + memset(addr + from, 0, MAX_INLINE_DATA - from);
 +}
 +
 +int truncate_inline_data(struct inode *inode, u64 from)
 +{
 + struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 + struct page *page, *ipage;
 + void *addr;
 + int

RE: [PATCH] f2fs: fix to truncate inline data past EOF

2015-03-09 Thread Chao Yu
Hi Jaegeuk,

 -Original Message-
 From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
 Sent: Tuesday, March 10, 2015 11:00 AM
 To: Chao Yu
 Cc: 'Changman Lee'; linux-f2fs-de...@lists.sourceforge.net; 
 linux-kernel@vger.kernel.org
 Subject: Re: [PATCH] f2fs: fix to truncate inline data past EOF
 
 Hi Chao,
 
 On Tue, Mar 10, 2015 at 10:02:46AM +0800, Chao Yu wrote:
  Hi Jaegeuk,
 
 
 [snip]
 
 +static int truncate_partial_data_page(struct inode *inode, u64 from, 
 bool force)
  {
   unsigned offset = from  (PAGE_CACHE_SIZE - 1);
   struct page *page;

 - if (!offset)
 + if (!offset  !force)
   
We truncate on-disk inode page and #0 page separately, so IMO there is 
a very
small chance the following issue will occur.
   
(0  truncated_size  MAX_INLINE_SIZE)
-f2fs_setattr
  -truncate_setsize #0 page has been truncated partially
  -f2fs_truncate
-truncate_blocks
invalidate #0 page by reclaimer
update #0 page with original 
data in inode page by reader
  
 truncate_setsize called 
   truncate_pagesize.
 so #0 page was truncated successfully.
 
  Yeah, but it's partially truncating as 0  truncated_size  MAX_INLINE_SIZE,
  After truncating, we still keep [0, truncated_size) valid data in #0 page, 
  and
  our #0 page status is uptodate | !dirty.
 
  So what I mean is that mm can reclaim this #0 page, then if someone else 
  read
  the #0 page again, we will update the #0 page with original data in inode 
  page
  since inode page haven't truncated yet.
 
 The truncate_blocks dropped inline_data in inode page, which is modified by
 this patch.
 And, the cached #0 page was truncated by truncate_setsize.
 Even if this page was evicted and reloaded again, the data would be filled 
 with
 the inode page having truncated inline_data.
 So, it seems there is no problem.
 
 BTW, do you mean like this scenario?

Yeah, actually it is. ;-)

 
 - f2fs_setattr
  -truncate_setsize: #0 page has been truncated partially
  -f2fs_truncate
   invalidate #0 page by reclaimer
   update #0 page with original data in 
 inode page by reader
 
 -truncate_blocks
   (*)- truncate_inline_inode
 (*)- truncate_partial_data_page(,, force)
find_data_page(,, force)  - we can use *force* here.
 
 Then, I agree that two functions as noted (*) above would be necessary.
 
 
  If this happened, if we don't truncate #0 page in following code of 
  truncate_blocks,
  we will remain the original data for user, it's wrong.
 
  IMO, it's better to truncate inode page and #0 page together, or truncate 
  #0 page
  in truncate_partial_data_page.
 
  How do you think?
 
  
  -truncate_inline_inode
  -truncate_partial_data_page
we will fail to truncate #0 page because we can't find valid 
blkaddr for
#0 page in find_data_page(,,false)
   
How about using find_data_page(,,true) to check weather this page is 
update or not
for this condition.
  
   Oh, I realized that we don't need to call truncate_partial_data_page, 
   since the
   cached #0 page was truncated already. We don't care about that.
 
  IMO, we should care about this #0 page if above example can happen.
 
   So, do we need to add just truncate_inline_inode() like below?
  
   
   return 0;

   page = find_data_page(inode, from  PAGE_CACHE_SHIFT, false);
 @@ -489,6 +489,7 @@ int truncate_blocks(struct inode *inode, u64 
 from, bool lock)
   pgoff_t free_from;
   int count = 0, err = 0;
   struct page *ipage;
 + bool truncate_page = false;

   trace_f2fs_truncate_blocks_enter(inode, from);

 @@ -504,6 +505,9 @@ int truncate_blocks(struct inode *inode, u64 
 from, bool lock)
   }

   if (f2fs_has_inline_data(inode)) {
 + truncate_inline_inode(ipage, from);
 + set_page_dirty(ipage);
   
If @from is the same as MAX_INLINE_DATA, we will change nothing in 
inode page,
How about skipping set_page_dirty for inode page in this condition?
  
   Agreed.
   How about adding this in truncate_inline_inode?
  
 if (from = MAX_INLINE_DATA)
 return;
 ...
 set_page_dirty(ipage);
 
  Yeah, that's good.
 
  And What I suggest is:
 
  bool truncate_inline_inode(struct page *ipage, u64 from)
  {
  /*
   * 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)
 
   if (from

RE: [PATCH] f2fs: fix to truncate inline data past EOF

2015-03-09 Thread Chao Yu
Hi Jaegeuk,

 -Original Message-
 From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
 Sent: Tuesday, March 10, 2015 12:16 AM
 To: Chao Yu
 Cc: 'Changman Lee'; linux-f2fs-de...@lists.sourceforge.net; 
 linux-kernel@vger.kernel.org
 Subject: Re: [PATCH] f2fs: fix to truncate inline data past EOF
 
 Hi Chao,
 
 On Mon, Mar 09, 2015 at 04:30:32PM +0800, Chao Yu wrote:
  Hi Jaegeuk,
 
   -Original Message-
   From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
   Sent: Monday, March 09, 2015 11:49 AM
   To: Chao Yu
   Cc: 'Changman Lee'; linux-f2fs-de...@lists.sourceforge.net; 
   linux-kernel@vger.kernel.org
   Subject: Re: [PATCH] f2fs: fix to truncate inline data past EOF
  
   Hi Chao,
  
  
   On Mon, Mar 09, 2015 at 10:24:35AM +0800, Chao Yu wrote:
Hi Jaegeuk,
   
 -Original Message-
 From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
 Sent: Friday, March 06, 2015 5:34 AM
 To: Chao Yu
 Cc: Changman Lee; linux-f2fs-de...@lists.sourceforge.net; 
 linux-kernel@vger.kernel.org
 Subject: Re: [PATCH] f2fs: fix to truncate inline data past EOF

 Hi Chao,

 On Thu, Mar 05, 2015 at 05:41:32PM +0800, Chao Yu wrote:
  Previously if inode is with inline data, we will try to invalid 
  partial inline
  data in page #0 when we truncate size of inode in 
  truncate_partial_data_page().
  And then we set page #0 to dirty, after this we can synchronize 
  inode page with
  page #0 at -writepage().
 
  But sometimes we will fail to operate page #0 in 
  truncate_partial_data_page()
  due to below reason:
  a) if offset is zero, we will skip setting page #0 to dirty.
  b) if page #0 is not uptodate, we will fail to update it as it has 
  no mapping
  data.
 
  So with following operations, we will meet recent data which should 
  be
  truncated.
 
  1.write inline data to file
  2.sync first data page to inode page
  3.truncate file size to 0
  4.truncate file size to max_inline_size
  5.echo 1  /proc/sys/vm/drop_caches
  6.read file -- meet original inline data which is remained in 
  inode page.
 
  This patch renames truncate_inline_data() to 
  truncate_inline_inode() for code
  readability, then intruduces new truncate_inline_data() and use it 
  to truncate
  inline data in page #0 and inode page in 
  truncate_partial_data_page() for
  fixing.
 
  Signed-off-by: Chao Yu chao2...@samsung.com
  ---
   fs/f2fs/f2fs.h   |  1 +
   fs/f2fs/file.c   | 13 -
   fs/f2fs/inline.c | 57 
  +++-
   3 files changed, 61 insertions(+), 10 deletions(-)
 
  diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
  index 511d6cd..55dbbe7 100644
  --- a/fs/f2fs/f2fs.h
  +++ b/fs/f2fs/f2fs.h
  @@ -1754,6 +1754,7 @@ extern struct kmem_cache *inode_entry_slab;
*/
   bool f2fs_may_inline(struct inode *);
   void read_inline_data(struct page *, struct page *);
  +int truncate_inline_data(struct inode *, u64 from);
   int f2fs_read_inline_data(struct inode *, struct page *);
   int f2fs_convert_inline_page(struct dnode_of_data *, struct page 
  *);
   int f2fs_convert_inline_inode(struct inode *);
  diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
  index f1341c7..6c4443e 100644
  --- a/fs/f2fs/file.c
  +++ b/fs/f2fs/file.c
  @@ -461,6 +461,9 @@ static int truncate_partial_data_page(struct 
  inode *inode, u64
 from)
  unsigned offset = from  (PAGE_CACHE_SIZE - 1);
  struct page *page;
 
  +   if (f2fs_has_inline_data(inode))
  +   return 0;
  +
  if (!offset)
  return 0;
 
  @@ -497,14 +500,14 @@ int truncate_blocks(struct inode *inode, u64 
  from, bool lock)
  if (lock)
  f2fs_lock_op(sbi);
 
  -   ipage = get_node_page(sbi, inode-i_ino);
  -   if (IS_ERR(ipage)) {
  -   err = PTR_ERR(ipage);
  +   if (f2fs_has_inline_data(inode)) {
  +   err = truncate_inline_data(inode, from);
  goto out;
  }
 
  -   if (f2fs_has_inline_data(inode)) {
  -   f2fs_put_page(ipage, 1);
  +   ipage = get_node_page(sbi, inode-i_ino);
  +   if (IS_ERR(ipage)) {
  +   err = PTR_ERR(ipage);
  goto out;
  }
 
  diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
  index 4ba9732..a8189c92 100644
  --- a/fs/f2fs/inline.c
  +++ b/fs/f2fs/inline.c
  @@ -50,10 +50,57 @@ void read_inline_data(struct page *page, struct 
  page *ipage)
  SetPageUptodate(page);
   }
 
  -static void truncate_inline_data(struct page *ipage)
  +static void truncate_inline_inode(struct page *ipage, u64 from)
   {
  +   void *addr = inline_data_addr(ipage

Re: [PATCH] f2fs: fix to truncate inline data past EOF

2015-03-09 Thread Jaegeuk Kim
Hi Chao,

On Tue, Mar 10, 2015 at 10:02:46AM +0800, Chao Yu wrote:
 Hi Jaegeuk,
 

[snip]

+static int truncate_partial_data_page(struct inode *inode, u64 from, 
bool force)
 {
unsigned offset = from  (PAGE_CACHE_SIZE - 1);
struct page *page;
   
-   if (!offset)
+   if (!offset  !force)
  
   We truncate on-disk inode page and #0 page separately, so IMO there is a 
   very
   small chance the following issue will occur.
  
   (0  truncated_size  MAX_INLINE_SIZE)
   -f2fs_setattr
 -truncate_setsize #0 page has been truncated partially
 -f2fs_truncate
   -truncate_blocks
 invalidate #0 page by reclaimer
 update #0 page with original data in 
   inode page by reader
  
  truncate_setsize called 
  truncate_pagesize.
  so #0 page was truncated successfully.
 
 Yeah, but it's partially truncating as 0  truncated_size  MAX_INLINE_SIZE,
 After truncating, we still keep [0, truncated_size) valid data in #0 page, and
 our #0 page status is uptodate | !dirty.
 
 So what I mean is that mm can reclaim this #0 page, then if someone else read
 the #0 page again, we will update the #0 page with original data in inode page
 since inode page haven't truncated yet.

The truncate_blocks dropped inline_data in inode page, which is modified by
this patch.
And, the cached #0 page was truncated by truncate_setsize.
Even if this page was evicted and reloaded again, the data would be filled with
the inode page having truncated inline_data.
So, it seems there is no problem.

BTW, do you mean like this scenario?

- f2fs_setattr
 -truncate_setsize: #0 page has been truncated partially
 -f2fs_truncate
invalidate #0 page by reclaimer
update #0 page with original data in 
inode page by reader

-truncate_blocks
  (*)- truncate_inline_inode
(*)- truncate_partial_data_page(,, force)
   find_data_page(,, force)  - we can use *force* here.

Then, I agree that two functions as noted (*) above would be necessary.

 
 If this happened, if we don't truncate #0 page in following code of 
 truncate_blocks,
 we will remain the original data for user, it's wrong.
 
 IMO, it's better to truncate inode page and #0 page together, or truncate #0 
 page
 in truncate_partial_data_page.
 
 How do you think?
 
  
 -truncate_inline_inode
 -truncate_partial_data_page
   we will fail to truncate #0 page because we can't find valid 
   blkaddr for
   #0 page in find_data_page(,,false)
  
   How about using find_data_page(,,true) to check weather this page is 
   update or not
   for this condition.
  
  Oh, I realized that we don't need to call truncate_partial_data_page, since 
  the
  cached #0 page was truncated already. We don't care about that.
 
 IMO, we should care about this #0 page if above example can happen.
 
  So, do we need to add just truncate_inline_inode() like below?
  
  
return 0;
   
page = find_data_page(inode, from  PAGE_CACHE_SHIFT, false);
@@ -489,6 +489,7 @@ int truncate_blocks(struct inode *inode, u64 from, 
bool lock)
pgoff_t free_from;
int count = 0, err = 0;
struct page *ipage;
+   bool truncate_page = false;
   
trace_f2fs_truncate_blocks_enter(inode, from);
   
@@ -504,6 +505,9 @@ int truncate_blocks(struct inode *inode, u64 from, 
bool lock)
}
   
if (f2fs_has_inline_data(inode)) {
+   truncate_inline_inode(ipage, from);
+   set_page_dirty(ipage);
  
   If @from is the same as MAX_INLINE_DATA, we will change nothing in inode 
   page,
   How about skipping set_page_dirty for inode page in this condition?
  
  Agreed.
  How about adding this in truncate_inline_inode?
  
  if (from = MAX_INLINE_DATA)
  return;
  ...
  set_page_dirty(ipage);
 
 Yeah, that's good.
  
 And What I suggest is:
 
 bool truncate_inline_inode(struct page *ipage, u64 from)
 {
   /*
* 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)

if (from = MAX_INLINE_DATA)  to handle when f2fs_bug_on is bypassed.

   return false;
   ...
   return true;
 }
 
 in truncate_blocks()
 
   if (f2fs_has_inline_data(inode)) {
   if (truncate_inline_inode(ipage, from))
   set_page_dirty(ipage);
 
 So by this way, we can distinguish between a bug and invalid truncating in
 truncate_inline_inode, and also we can avoid unneeded 

RE: [PATCH] f2fs: fix to truncate inline data past EOF

2015-03-09 Thread Chao Yu
Hi Jaegeuk,

 -Original Message-
 From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
 Sent: Monday, March 09, 2015 11:49 AM
 To: Chao Yu
 Cc: 'Changman Lee'; linux-f2fs-de...@lists.sourceforge.net; 
 linux-kernel@vger.kernel.org
 Subject: Re: [PATCH] f2fs: fix to truncate inline data past EOF
 
 Hi Chao,
 
 
 On Mon, Mar 09, 2015 at 10:24:35AM +0800, Chao Yu wrote:
  Hi Jaegeuk,
 
   -Original Message-
   From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
   Sent: Friday, March 06, 2015 5:34 AM
   To: Chao Yu
   Cc: Changman Lee; linux-f2fs-de...@lists.sourceforge.net; 
   linux-kernel@vger.kernel.org
   Subject: Re: [PATCH] f2fs: fix to truncate inline data past EOF
  
   Hi Chao,
  
   On Thu, Mar 05, 2015 at 05:41:32PM +0800, Chao Yu wrote:
Previously if inode is with inline data, we will try to invalid partial 
inline
data in page #0 when we truncate size of inode in 
truncate_partial_data_page().
And then we set page #0 to dirty, after this we can synchronize inode 
page with
page #0 at -writepage().
   
But sometimes we will fail to operate page #0 in 
truncate_partial_data_page()
due to below reason:
a) if offset is zero, we will skip setting page #0 to dirty.
b) if page #0 is not uptodate, we will fail to update it as it has no 
mapping
data.
   
So with following operations, we will meet recent data which should be
truncated.
   
1.write inline data to file
2.sync first data page to inode page
3.truncate file size to 0
4.truncate file size to max_inline_size
5.echo 1  /proc/sys/vm/drop_caches
6.read file -- meet original inline data which is remained in inode 
page.
   
This patch renames truncate_inline_data() to truncate_inline_inode() 
for code
readability, then intruduces new truncate_inline_data() and use it to 
truncate
inline data in page #0 and inode page in truncate_partial_data_page() 
for
fixing.
   
Signed-off-by: Chao Yu chao2...@samsung.com
---
 fs/f2fs/f2fs.h   |  1 +
 fs/f2fs/file.c   | 13 -
 fs/f2fs/inline.c | 57 
+++-
 3 files changed, 61 insertions(+), 10 deletions(-)
   
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 511d6cd..55dbbe7 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1754,6 +1754,7 @@ extern struct kmem_cache *inode_entry_slab;
  */
 bool f2fs_may_inline(struct inode *);
 void read_inline_data(struct page *, struct page *);
+int truncate_inline_data(struct inode *, u64 from);
 int f2fs_read_inline_data(struct inode *, struct page *);
 int f2fs_convert_inline_page(struct dnode_of_data *, struct page *);
 int f2fs_convert_inline_inode(struct inode *);
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index f1341c7..6c4443e 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -461,6 +461,9 @@ static int truncate_partial_data_page(struct inode 
*inode, u64 from)
unsigned offset = from  (PAGE_CACHE_SIZE - 1);
struct page *page;
   
+   if (f2fs_has_inline_data(inode))
+   return 0;
+
if (!offset)
return 0;
   
@@ -497,14 +500,14 @@ int truncate_blocks(struct inode *inode, u64 
from, bool lock)
if (lock)
f2fs_lock_op(sbi);
   
-   ipage = get_node_page(sbi, inode-i_ino);
-   if (IS_ERR(ipage)) {
-   err = PTR_ERR(ipage);
+   if (f2fs_has_inline_data(inode)) {
+   err = truncate_inline_data(inode, from);
goto out;
}
   
-   if (f2fs_has_inline_data(inode)) {
-   f2fs_put_page(ipage, 1);
+   ipage = get_node_page(sbi, inode-i_ino);
+   if (IS_ERR(ipage)) {
+   err = PTR_ERR(ipage);
goto out;
}
   
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 4ba9732..a8189c92 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -50,10 +50,57 @@ void read_inline_data(struct page *page, struct 
page *ipage)
SetPageUptodate(page);
 }
   
-static void truncate_inline_data(struct page *ipage)
+static void truncate_inline_inode(struct page *ipage, u64 from)
 {
+   void *addr = inline_data_addr(ipage);
+
f2fs_wait_on_page_writeback(ipage, NODE);
-   memset(inline_data_addr(ipage), 0, MAX_INLINE_DATA);
+   memset(addr + from, 0, MAX_INLINE_DATA - from);
+}
+
+int truncate_inline_data(struct inode *inode, u64 from)
+{
+   struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
+   struct page *page, *ipage;
+   void *addr;
+   int err = 0;
+
+   /*
+* we should never truncate inline data past max inline data 
size,
+* because we always

Re: [PATCH] f2fs: fix to truncate inline data past EOF

2015-03-08 Thread Jaegeuk Kim
Hi Chao,


On Mon, Mar 09, 2015 at 10:24:35AM +0800, Chao Yu wrote:
> Hi Jaegeuk,
> 
> > -Original Message-
> > From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
> > Sent: Friday, March 06, 2015 5:34 AM
> > To: Chao Yu
> > Cc: Changman Lee; linux-f2fs-de...@lists.sourceforge.net; 
> > linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH] f2fs: fix to truncate inline data past EOF
> > 
> > Hi Chao,
> > 
> > On Thu, Mar 05, 2015 at 05:41:32PM +0800, Chao Yu wrote:
> > > Previously if inode is with inline data, we will try to invalid partial 
> > > inline
> > > data in page #0 when we truncate size of inode in 
> > > truncate_partial_data_page().
> > > And then we set page #0 to dirty, after this we can synchronize inode 
> > > page with
> > > page #0 at ->writepage().
> > >
> > > But sometimes we will fail to operate page #0 in 
> > > truncate_partial_data_page()
> > > due to below reason:
> > > a) if offset is zero, we will skip setting page #0 to dirty.
> > > b) if page #0 is not uptodate, we will fail to update it as it has no 
> > > mapping
> > > data.
> > >
> > > So with following operations, we will meet recent data which should be
> > > truncated.
> > >
> > > 1.write inline data to file
> > > 2.sync first data page to inode page
> > > 3.truncate file size to 0
> > > 4.truncate file size to max_inline_size
> > > 5.echo 1 > /proc/sys/vm/drop_caches
> > > 6.read file --> meet original inline data which is remained in inode page.
> > >
> > > This patch renames truncate_inline_data() to truncate_inline_inode() for 
> > > code
> > > readability, then intruduces new truncate_inline_data() and use it to 
> > > truncate
> > > inline data in page #0 and inode page in truncate_partial_data_page() for
> > > fixing.
> > >
> > > Signed-off-by: Chao Yu 
> > > ---
> > >  fs/f2fs/f2fs.h   |  1 +
> > >  fs/f2fs/file.c   | 13 -
> > >  fs/f2fs/inline.c | 57 
> > > +++-
> > >  3 files changed, 61 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > index 511d6cd..55dbbe7 100644
> > > --- a/fs/f2fs/f2fs.h
> > > +++ b/fs/f2fs/f2fs.h
> > > @@ -1754,6 +1754,7 @@ extern struct kmem_cache *inode_entry_slab;
> > >   */
> > >  bool f2fs_may_inline(struct inode *);
> > >  void read_inline_data(struct page *, struct page *);
> > > +int truncate_inline_data(struct inode *, u64 from);
> > >  int f2fs_read_inline_data(struct inode *, struct page *);
> > >  int f2fs_convert_inline_page(struct dnode_of_data *, struct page *);
> > >  int f2fs_convert_inline_inode(struct inode *);
> > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > > index f1341c7..6c4443e 100644
> > > --- a/fs/f2fs/file.c
> > > +++ b/fs/f2fs/file.c
> > > @@ -461,6 +461,9 @@ static int truncate_partial_data_page(struct inode 
> > > *inode, u64 from)
> > >   unsigned offset = from & (PAGE_CACHE_SIZE - 1);
> > >   struct page *page;
> > >
> > > + if (f2fs_has_inline_data(inode))
> > > + return 0;
> > > +
> > >   if (!offset)
> > >   return 0;
> > >
> > > @@ -497,14 +500,14 @@ int truncate_blocks(struct inode *inode, u64 from, 
> > > bool lock)
> > >   if (lock)
> > >   f2fs_lock_op(sbi);
> > >
> > > - ipage = get_node_page(sbi, inode->i_ino);
> > > - if (IS_ERR(ipage)) {
> > > - err = PTR_ERR(ipage);
> > > + if (f2fs_has_inline_data(inode)) {
> > > + err = truncate_inline_data(inode, from);
> > >   goto out;
> > >   }
> > >
> > > - if (f2fs_has_inline_data(inode)) {
> > > - f2fs_put_page(ipage, 1);
> > > + ipage = get_node_page(sbi, inode->i_ino);
> > > + if (IS_ERR(ipage)) {
> > > + err = PTR_ERR(ipage);
> > >   goto out;
> > >   }
> > >
> > > diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> > > index 4ba9732..a8189c92 100644
> > > --- a/fs/f2fs/inline.c
> > > +++ b/fs/f2fs/inline.c
> > > @@ -50,10 +50,57 @@ void read_inline_data(struct page *page, struct page 
> > > *ipage)
> > >   SetPageUptodate(page);
> > >  }
> > >

RE: [PATCH] f2fs: fix to truncate inline data past EOF

2015-03-08 Thread Chao Yu
Hi Jaegeuk,

> -Original Message-
> From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
> Sent: Friday, March 06, 2015 5:34 AM
> To: Chao Yu
> Cc: Changman Lee; linux-f2fs-de...@lists.sourceforge.net; 
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] f2fs: fix to truncate inline data past EOF
> 
> Hi Chao,
> 
> On Thu, Mar 05, 2015 at 05:41:32PM +0800, Chao Yu wrote:
> > Previously if inode is with inline data, we will try to invalid partial 
> > inline
> > data in page #0 when we truncate size of inode in 
> > truncate_partial_data_page().
> > And then we set page #0 to dirty, after this we can synchronize inode page 
> > with
> > page #0 at ->writepage().
> >
> > But sometimes we will fail to operate page #0 in 
> > truncate_partial_data_page()
> > due to below reason:
> > a) if offset is zero, we will skip setting page #0 to dirty.
> > b) if page #0 is not uptodate, we will fail to update it as it has no 
> > mapping
> > data.
> >
> > So with following operations, we will meet recent data which should be
> > truncated.
> >
> > 1.write inline data to file
> > 2.sync first data page to inode page
> > 3.truncate file size to 0
> > 4.truncate file size to max_inline_size
> > 5.echo 1 > /proc/sys/vm/drop_caches
> > 6.read file --> meet original inline data which is remained in inode page.
> >
> > This patch renames truncate_inline_data() to truncate_inline_inode() for 
> > code
> > readability, then intruduces new truncate_inline_data() and use it to 
> > truncate
> > inline data in page #0 and inode page in truncate_partial_data_page() for
> > fixing.
> >
> > Signed-off-by: Chao Yu 
> > ---
> >  fs/f2fs/f2fs.h   |  1 +
> >  fs/f2fs/file.c   | 13 -
> >  fs/f2fs/inline.c | 57 
> > +++-
> >  3 files changed, 61 insertions(+), 10 deletions(-)
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 511d6cd..55dbbe7 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -1754,6 +1754,7 @@ extern struct kmem_cache *inode_entry_slab;
> >   */
> >  bool f2fs_may_inline(struct inode *);
> >  void read_inline_data(struct page *, struct page *);
> > +int truncate_inline_data(struct inode *, u64 from);
> >  int f2fs_read_inline_data(struct inode *, struct page *);
> >  int f2fs_convert_inline_page(struct dnode_of_data *, struct page *);
> >  int f2fs_convert_inline_inode(struct inode *);
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index f1341c7..6c4443e 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -461,6 +461,9 @@ static int truncate_partial_data_page(struct inode 
> > *inode, u64 from)
> > unsigned offset = from & (PAGE_CACHE_SIZE - 1);
> > struct page *page;
> >
> > +   if (f2fs_has_inline_data(inode))
> > +   return 0;
> > +
> > if (!offset)
> > return 0;
> >
> > @@ -497,14 +500,14 @@ int truncate_blocks(struct inode *inode, u64 from, 
> > bool lock)
> > if (lock)
> > f2fs_lock_op(sbi);
> >
> > -   ipage = get_node_page(sbi, inode->i_ino);
> > -   if (IS_ERR(ipage)) {
> > -   err = PTR_ERR(ipage);
> > +   if (f2fs_has_inline_data(inode)) {
> > +   err = truncate_inline_data(inode, from);
> > goto out;
> > }
> >
> > -   if (f2fs_has_inline_data(inode)) {
> > -   f2fs_put_page(ipage, 1);
> > +   ipage = get_node_page(sbi, inode->i_ino);
> > +   if (IS_ERR(ipage)) {
> > +   err = PTR_ERR(ipage);
> > goto out;
> > }
> >
> > diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> > index 4ba9732..a8189c92 100644
> > --- a/fs/f2fs/inline.c
> > +++ b/fs/f2fs/inline.c
> > @@ -50,10 +50,57 @@ void read_inline_data(struct page *page, struct page 
> > *ipage)
> > SetPageUptodate(page);
> >  }
> >
> > -static void truncate_inline_data(struct page *ipage)
> > +static void truncate_inline_inode(struct page *ipage, u64 from)
> >  {
> > +   void *addr = inline_data_addr(ipage);
> > +
> > f2fs_wait_on_page_writeback(ipage, NODE);
> > -   memset(inline_data_addr(ipage), 0, MAX_INLINE_DATA);
> > +   memset(addr + from, 0, MAX_INLINE_DATA - from);
> > +}
> > +
> > +int truncate_inline_data(struct inode *inode, u64 from)
> > +{
> > +   struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> > +   struct page

RE: [PATCH] f2fs: fix to truncate inline data past EOF

2015-03-08 Thread Chao Yu
Hi Jaegeuk,

 -Original Message-
 From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
 Sent: Friday, March 06, 2015 5:34 AM
 To: Chao Yu
 Cc: Changman Lee; linux-f2fs-de...@lists.sourceforge.net; 
 linux-kernel@vger.kernel.org
 Subject: Re: [PATCH] f2fs: fix to truncate inline data past EOF
 
 Hi Chao,
 
 On Thu, Mar 05, 2015 at 05:41:32PM +0800, Chao Yu wrote:
  Previously if inode is with inline data, we will try to invalid partial 
  inline
  data in page #0 when we truncate size of inode in 
  truncate_partial_data_page().
  And then we set page #0 to dirty, after this we can synchronize inode page 
  with
  page #0 at -writepage().
 
  But sometimes we will fail to operate page #0 in 
  truncate_partial_data_page()
  due to below reason:
  a) if offset is zero, we will skip setting page #0 to dirty.
  b) if page #0 is not uptodate, we will fail to update it as it has no 
  mapping
  data.
 
  So with following operations, we will meet recent data which should be
  truncated.
 
  1.write inline data to file
  2.sync first data page to inode page
  3.truncate file size to 0
  4.truncate file size to max_inline_size
  5.echo 1  /proc/sys/vm/drop_caches
  6.read file -- meet original inline data which is remained in inode page.
 
  This patch renames truncate_inline_data() to truncate_inline_inode() for 
  code
  readability, then intruduces new truncate_inline_data() and use it to 
  truncate
  inline data in page #0 and inode page in truncate_partial_data_page() for
  fixing.
 
  Signed-off-by: Chao Yu chao2...@samsung.com
  ---
   fs/f2fs/f2fs.h   |  1 +
   fs/f2fs/file.c   | 13 -
   fs/f2fs/inline.c | 57 
  +++-
   3 files changed, 61 insertions(+), 10 deletions(-)
 
  diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
  index 511d6cd..55dbbe7 100644
  --- a/fs/f2fs/f2fs.h
  +++ b/fs/f2fs/f2fs.h
  @@ -1754,6 +1754,7 @@ extern struct kmem_cache *inode_entry_slab;
*/
   bool f2fs_may_inline(struct inode *);
   void read_inline_data(struct page *, struct page *);
  +int truncate_inline_data(struct inode *, u64 from);
   int f2fs_read_inline_data(struct inode *, struct page *);
   int f2fs_convert_inline_page(struct dnode_of_data *, struct page *);
   int f2fs_convert_inline_inode(struct inode *);
  diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
  index f1341c7..6c4443e 100644
  --- a/fs/f2fs/file.c
  +++ b/fs/f2fs/file.c
  @@ -461,6 +461,9 @@ static int truncate_partial_data_page(struct inode 
  *inode, u64 from)
  unsigned offset = from  (PAGE_CACHE_SIZE - 1);
  struct page *page;
 
  +   if (f2fs_has_inline_data(inode))
  +   return 0;
  +
  if (!offset)
  return 0;
 
  @@ -497,14 +500,14 @@ int truncate_blocks(struct inode *inode, u64 from, 
  bool lock)
  if (lock)
  f2fs_lock_op(sbi);
 
  -   ipage = get_node_page(sbi, inode-i_ino);
  -   if (IS_ERR(ipage)) {
  -   err = PTR_ERR(ipage);
  +   if (f2fs_has_inline_data(inode)) {
  +   err = truncate_inline_data(inode, from);
  goto out;
  }
 
  -   if (f2fs_has_inline_data(inode)) {
  -   f2fs_put_page(ipage, 1);
  +   ipage = get_node_page(sbi, inode-i_ino);
  +   if (IS_ERR(ipage)) {
  +   err = PTR_ERR(ipage);
  goto out;
  }
 
  diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
  index 4ba9732..a8189c92 100644
  --- a/fs/f2fs/inline.c
  +++ b/fs/f2fs/inline.c
  @@ -50,10 +50,57 @@ void read_inline_data(struct page *page, struct page 
  *ipage)
  SetPageUptodate(page);
   }
 
  -static void truncate_inline_data(struct page *ipage)
  +static void truncate_inline_inode(struct page *ipage, u64 from)
   {
  +   void *addr = inline_data_addr(ipage);
  +
  f2fs_wait_on_page_writeback(ipage, NODE);
  -   memset(inline_data_addr(ipage), 0, MAX_INLINE_DATA);
  +   memset(addr + from, 0, MAX_INLINE_DATA - from);
  +}
  +
  +int truncate_inline_data(struct inode *inode, u64 from)
  +{
  +   struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
  +   struct page *page, *ipage;
  +   void *addr;
  +   int err = 0;
  +
  +   /*
  +* 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(sbi, from  MAX_INLINE_DATA);
  +
  +   if (from == MAX_INLINE_DATA)
  +   return 0;
  +
  +   page = grab_cache_page(inode-i_mapping, 0);
  +   if (!page)
  +   return -ENOMEM;
 
 Do we need to grab and read #0 page?

No need to read #0 page, sorry for the mistake.

 How about checking its cached page after on-disk truncation?
 
 1. page = get_node_page()
 2. check_inline
 3. truncate_inline_inode
 4. f2fs_put_page(ipage)
 
 5. truncate_partial_data_page(inode, from, force)
   if (!offset  !force)
   return 0;
 
  +
  +   ipage = get_node_page(sbi, inode-i_ino);
  +   if (IS_ERR(ipage

Re: [PATCH] f2fs: fix to truncate inline data past EOF

2015-03-08 Thread Jaegeuk Kim
Hi Chao,


On Mon, Mar 09, 2015 at 10:24:35AM +0800, Chao Yu wrote:
 Hi Jaegeuk,
 
  -Original Message-
  From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
  Sent: Friday, March 06, 2015 5:34 AM
  To: Chao Yu
  Cc: Changman Lee; linux-f2fs-de...@lists.sourceforge.net; 
  linux-kernel@vger.kernel.org
  Subject: Re: [PATCH] f2fs: fix to truncate inline data past EOF
  
  Hi Chao,
  
  On Thu, Mar 05, 2015 at 05:41:32PM +0800, Chao Yu wrote:
   Previously if inode is with inline data, we will try to invalid partial 
   inline
   data in page #0 when we truncate size of inode in 
   truncate_partial_data_page().
   And then we set page #0 to dirty, after this we can synchronize inode 
   page with
   page #0 at -writepage().
  
   But sometimes we will fail to operate page #0 in 
   truncate_partial_data_page()
   due to below reason:
   a) if offset is zero, we will skip setting page #0 to dirty.
   b) if page #0 is not uptodate, we will fail to update it as it has no 
   mapping
   data.
  
   So with following operations, we will meet recent data which should be
   truncated.
  
   1.write inline data to file
   2.sync first data page to inode page
   3.truncate file size to 0
   4.truncate file size to max_inline_size
   5.echo 1  /proc/sys/vm/drop_caches
   6.read file -- meet original inline data which is remained in inode page.
  
   This patch renames truncate_inline_data() to truncate_inline_inode() for 
   code
   readability, then intruduces new truncate_inline_data() and use it to 
   truncate
   inline data in page #0 and inode page in truncate_partial_data_page() for
   fixing.
  
   Signed-off-by: Chao Yu chao2...@samsung.com
   ---
fs/f2fs/f2fs.h   |  1 +
fs/f2fs/file.c   | 13 -
fs/f2fs/inline.c | 57 
   +++-
3 files changed, 61 insertions(+), 10 deletions(-)
  
   diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
   index 511d6cd..55dbbe7 100644
   --- a/fs/f2fs/f2fs.h
   +++ b/fs/f2fs/f2fs.h
   @@ -1754,6 +1754,7 @@ extern struct kmem_cache *inode_entry_slab;
 */
bool f2fs_may_inline(struct inode *);
void read_inline_data(struct page *, struct page *);
   +int truncate_inline_data(struct inode *, u64 from);
int f2fs_read_inline_data(struct inode *, struct page *);
int f2fs_convert_inline_page(struct dnode_of_data *, struct page *);
int f2fs_convert_inline_inode(struct inode *);
   diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
   index f1341c7..6c4443e 100644
   --- a/fs/f2fs/file.c
   +++ b/fs/f2fs/file.c
   @@ -461,6 +461,9 @@ static int truncate_partial_data_page(struct inode 
   *inode, u64 from)
 unsigned offset = from  (PAGE_CACHE_SIZE - 1);
 struct page *page;
  
   + if (f2fs_has_inline_data(inode))
   + return 0;
   +
 if (!offset)
 return 0;
  
   @@ -497,14 +500,14 @@ int truncate_blocks(struct inode *inode, u64 from, 
   bool lock)
 if (lock)
 f2fs_lock_op(sbi);
  
   - ipage = get_node_page(sbi, inode-i_ino);
   - if (IS_ERR(ipage)) {
   - err = PTR_ERR(ipage);
   + if (f2fs_has_inline_data(inode)) {
   + err = truncate_inline_data(inode, from);
 goto out;
 }
  
   - if (f2fs_has_inline_data(inode)) {
   - f2fs_put_page(ipage, 1);
   + ipage = get_node_page(sbi, inode-i_ino);
   + if (IS_ERR(ipage)) {
   + err = PTR_ERR(ipage);
 goto out;
 }
  
   diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
   index 4ba9732..a8189c92 100644
   --- a/fs/f2fs/inline.c
   +++ b/fs/f2fs/inline.c
   @@ -50,10 +50,57 @@ void read_inline_data(struct page *page, struct page 
   *ipage)
 SetPageUptodate(page);
}
  
   -static void truncate_inline_data(struct page *ipage)
   +static void truncate_inline_inode(struct page *ipage, u64 from)
{
   + void *addr = inline_data_addr(ipage);
   +
 f2fs_wait_on_page_writeback(ipage, NODE);
   - memset(inline_data_addr(ipage), 0, MAX_INLINE_DATA);
   + memset(addr + from, 0, MAX_INLINE_DATA - from);
   +}
   +
   +int truncate_inline_data(struct inode *inode, u64 from)
   +{
   + struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
   + struct page *page, *ipage;
   + void *addr;
   + int err = 0;
   +
   + /*
   +  * 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(sbi, from  MAX_INLINE_DATA);
   +
   + if (from == MAX_INLINE_DATA)
   + return 0;
   +
   + page = grab_cache_page(inode-i_mapping, 0);
   + if (!page)
   + return -ENOMEM;
  
  Do we need to grab and read #0 page?
 
 No need to read #0 page, sorry for the mistake.
 
  How about checking its cached page after on-disk truncation?
  
  1. page = get_node_page()
  2. check_inline
  3. truncate_inline_inode
  4. f2fs_put_page(ipage)
  
  5. truncate_partial_data_page(inode, from, force

Re: [PATCH] f2fs: fix to truncate inline data past EOF

2015-03-05 Thread Jaegeuk Kim
Hi Chao,

On Thu, Mar 05, 2015 at 05:41:32PM +0800, Chao Yu wrote:
> Previously if inode is with inline data, we will try to invalid partial inline
> data in page #0 when we truncate size of inode in 
> truncate_partial_data_page().
> And then we set page #0 to dirty, after this we can synchronize inode page 
> with
> page #0 at ->writepage().
> 
> But sometimes we will fail to operate page #0 in truncate_partial_data_page()
> due to below reason:
> a) if offset is zero, we will skip setting page #0 to dirty.
> b) if page #0 is not uptodate, we will fail to update it as it has no mapping
> data.
> 
> So with following operations, we will meet recent data which should be
> truncated.
> 
> 1.write inline data to file
> 2.sync first data page to inode page
> 3.truncate file size to 0
> 4.truncate file size to max_inline_size
> 5.echo 1 > /proc/sys/vm/drop_caches
> 6.read file --> meet original inline data which is remained in inode page.
> 
> This patch renames truncate_inline_data() to truncate_inline_inode() for code
> readability, then intruduces new truncate_inline_data() and use it to truncate
> inline data in page #0 and inode page in truncate_partial_data_page() for
> fixing.
> 
> Signed-off-by: Chao Yu 
> ---
>  fs/f2fs/f2fs.h   |  1 +
>  fs/f2fs/file.c   | 13 -
>  fs/f2fs/inline.c | 57 
> +++-
>  3 files changed, 61 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 511d6cd..55dbbe7 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1754,6 +1754,7 @@ extern struct kmem_cache *inode_entry_slab;
>   */
>  bool f2fs_may_inline(struct inode *);
>  void read_inline_data(struct page *, struct page *);
> +int truncate_inline_data(struct inode *, u64 from);
>  int f2fs_read_inline_data(struct inode *, struct page *);
>  int f2fs_convert_inline_page(struct dnode_of_data *, struct page *);
>  int f2fs_convert_inline_inode(struct inode *);
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index f1341c7..6c4443e 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -461,6 +461,9 @@ static int truncate_partial_data_page(struct inode 
> *inode, u64 from)
>   unsigned offset = from & (PAGE_CACHE_SIZE - 1);
>   struct page *page;
>  
> + if (f2fs_has_inline_data(inode))
> + return 0;
> +
>   if (!offset)
>   return 0;
>  
> @@ -497,14 +500,14 @@ int truncate_blocks(struct inode *inode, u64 from, bool 
> lock)
>   if (lock)
>   f2fs_lock_op(sbi);
>  
> - ipage = get_node_page(sbi, inode->i_ino);
> - if (IS_ERR(ipage)) {
> - err = PTR_ERR(ipage);
> + if (f2fs_has_inline_data(inode)) {
> + err = truncate_inline_data(inode, from);
>   goto out;
>   }
>  
> - if (f2fs_has_inline_data(inode)) {
> - f2fs_put_page(ipage, 1);
> + ipage = get_node_page(sbi, inode->i_ino);
> + if (IS_ERR(ipage)) {
> + err = PTR_ERR(ipage);
>   goto out;
>   }
>  
> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> index 4ba9732..a8189c92 100644
> --- a/fs/f2fs/inline.c
> +++ b/fs/f2fs/inline.c
> @@ -50,10 +50,57 @@ void read_inline_data(struct page *page, struct page 
> *ipage)
>   SetPageUptodate(page);
>  }
>  
> -static void truncate_inline_data(struct page *ipage)
> +static void truncate_inline_inode(struct page *ipage, u64 from)
>  {
> + void *addr = inline_data_addr(ipage);
> +
>   f2fs_wait_on_page_writeback(ipage, NODE);
> - memset(inline_data_addr(ipage), 0, MAX_INLINE_DATA);
> + memset(addr + from, 0, MAX_INLINE_DATA - from);
> +}
> +
> +int truncate_inline_data(struct inode *inode, u64 from)
> +{
> + struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> + struct page *page, *ipage;
> + void *addr;
> + int err = 0;
> +
> + /*
> +  * 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(sbi, from > MAX_INLINE_DATA);
> +
> + if (from == MAX_INLINE_DATA)
> + return 0;
> +
> + page = grab_cache_page(inode->i_mapping, 0);
> + if (!page)
> + return -ENOMEM;

Do we need to grab and read #0 page?
How about checking its cached page after on-disk truncation?

1. page = get_node_page()
2. check_inline
3. truncate_inline_inode
4. f2fs_put_page(ipage)

5. truncate_partial_data_page(inode, from, force)
if (!offset && !force)
return 0;

> +
> + ipage = get_node_page(sbi, inode->i_ino);
> + if (IS_ERR(ipage)) {
> + err = PTR_ERR(ipage);
> + goto out;
> + }
> +
> + if (!f2fs_has_inline_data(inode)) {
> + f2fs_put_page(ipage, 1);
> + goto out;
> + }
> +
> + truncate_inline_inode(ipage, from);
> + set_page_dirty(ipage);
> +
> +   

[PATCH] f2fs: fix to truncate inline data past EOF

2015-03-05 Thread Chao Yu
Previously if inode is with inline data, we will try to invalid partial inline
data in page #0 when we truncate size of inode in truncate_partial_data_page().
And then we set page #0 to dirty, after this we can synchronize inode page with
page #0 at ->writepage().

But sometimes we will fail to operate page #0 in truncate_partial_data_page()
due to below reason:
a) if offset is zero, we will skip setting page #0 to dirty.
b) if page #0 is not uptodate, we will fail to update it as it has no mapping
data.

So with following operations, we will meet recent data which should be
truncated.

1.write inline data to file
2.sync first data page to inode page
3.truncate file size to 0
4.truncate file size to max_inline_size
5.echo 1 > /proc/sys/vm/drop_caches
6.read file --> meet original inline data which is remained in inode page.

This patch renames truncate_inline_data() to truncate_inline_inode() for code
readability, then intruduces new truncate_inline_data() and use it to truncate
inline data in page #0 and inode page in truncate_partial_data_page() for
fixing.

Signed-off-by: Chao Yu 
---
 fs/f2fs/f2fs.h   |  1 +
 fs/f2fs/file.c   | 13 -
 fs/f2fs/inline.c | 57 +++-
 3 files changed, 61 insertions(+), 10 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 511d6cd..55dbbe7 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1754,6 +1754,7 @@ extern struct kmem_cache *inode_entry_slab;
  */
 bool f2fs_may_inline(struct inode *);
 void read_inline_data(struct page *, struct page *);
+int truncate_inline_data(struct inode *, u64 from);
 int f2fs_read_inline_data(struct inode *, struct page *);
 int f2fs_convert_inline_page(struct dnode_of_data *, struct page *);
 int f2fs_convert_inline_inode(struct inode *);
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index f1341c7..6c4443e 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -461,6 +461,9 @@ static int truncate_partial_data_page(struct inode *inode, 
u64 from)
unsigned offset = from & (PAGE_CACHE_SIZE - 1);
struct page *page;
 
+   if (f2fs_has_inline_data(inode))
+   return 0;
+
if (!offset)
return 0;
 
@@ -497,14 +500,14 @@ int truncate_blocks(struct inode *inode, u64 from, bool 
lock)
if (lock)
f2fs_lock_op(sbi);
 
-   ipage = get_node_page(sbi, inode->i_ino);
-   if (IS_ERR(ipage)) {
-   err = PTR_ERR(ipage);
+   if (f2fs_has_inline_data(inode)) {
+   err = truncate_inline_data(inode, from);
goto out;
}
 
-   if (f2fs_has_inline_data(inode)) {
-   f2fs_put_page(ipage, 1);
+   ipage = get_node_page(sbi, inode->i_ino);
+   if (IS_ERR(ipage)) {
+   err = PTR_ERR(ipage);
goto out;
}
 
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 4ba9732..a8189c92 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -50,10 +50,57 @@ void read_inline_data(struct page *page, struct page *ipage)
SetPageUptodate(page);
 }
 
-static void truncate_inline_data(struct page *ipage)
+static void truncate_inline_inode(struct page *ipage, u64 from)
 {
+   void *addr = inline_data_addr(ipage);
+
f2fs_wait_on_page_writeback(ipage, NODE);
-   memset(inline_data_addr(ipage), 0, MAX_INLINE_DATA);
+   memset(addr + from, 0, MAX_INLINE_DATA - from);
+}
+
+int truncate_inline_data(struct inode *inode, u64 from)
+{
+   struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
+   struct page *page, *ipage;
+   void *addr;
+   int err = 0;
+
+   /*
+* 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(sbi, from > MAX_INLINE_DATA);
+
+   if (from == MAX_INLINE_DATA)
+   return 0;
+
+   page = grab_cache_page(inode->i_mapping, 0);
+   if (!page)
+   return -ENOMEM;
+
+   ipage = get_node_page(sbi, inode->i_ino);
+   if (IS_ERR(ipage)) {
+   err = PTR_ERR(ipage);
+   goto out;
+   }
+
+   if (!f2fs_has_inline_data(inode)) {
+   f2fs_put_page(ipage, 1);
+   goto out;
+   }
+
+   truncate_inline_inode(ipage, from);
+   set_page_dirty(ipage);
+
+   read_inline_data(page, ipage);
+   f2fs_put_page(ipage, 1);
+
+   addr = inline_data_addr(page);
+   memset(addr + from, 0, MAX_INLINE_DATA - from);
+out:
+   f2fs_put_page(page, 1);
+   return err;
 }
 
 int f2fs_read_inline_data(struct inode *inode, struct page *page)
@@ -131,7 +178,7 @@ no_update:
set_inode_flag(F2FS_I(dn->inode), FI_APPEND_WRITE);
 
/* clear inline data and flag after data writeback */
-   truncate_inline_data(dn->inode_page);
+   truncate_inline_inode(dn->inode_page, 

Re: [PATCH] f2fs: fix to truncate inline data past EOF

2015-03-05 Thread Jaegeuk Kim
Hi Chao,

On Thu, Mar 05, 2015 at 05:41:32PM +0800, Chao Yu wrote:
 Previously if inode is with inline data, we will try to invalid partial inline
 data in page #0 when we truncate size of inode in 
 truncate_partial_data_page().
 And then we set page #0 to dirty, after this we can synchronize inode page 
 with
 page #0 at -writepage().
 
 But sometimes we will fail to operate page #0 in truncate_partial_data_page()
 due to below reason:
 a) if offset is zero, we will skip setting page #0 to dirty.
 b) if page #0 is not uptodate, we will fail to update it as it has no mapping
 data.
 
 So with following operations, we will meet recent data which should be
 truncated.
 
 1.write inline data to file
 2.sync first data page to inode page
 3.truncate file size to 0
 4.truncate file size to max_inline_size
 5.echo 1  /proc/sys/vm/drop_caches
 6.read file -- meet original inline data which is remained in inode page.
 
 This patch renames truncate_inline_data() to truncate_inline_inode() for code
 readability, then intruduces new truncate_inline_data() and use it to truncate
 inline data in page #0 and inode page in truncate_partial_data_page() for
 fixing.
 
 Signed-off-by: Chao Yu chao2...@samsung.com
 ---
  fs/f2fs/f2fs.h   |  1 +
  fs/f2fs/file.c   | 13 -
  fs/f2fs/inline.c | 57 
 +++-
  3 files changed, 61 insertions(+), 10 deletions(-)
 
 diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
 index 511d6cd..55dbbe7 100644
 --- a/fs/f2fs/f2fs.h
 +++ b/fs/f2fs/f2fs.h
 @@ -1754,6 +1754,7 @@ extern struct kmem_cache *inode_entry_slab;
   */
  bool f2fs_may_inline(struct inode *);
  void read_inline_data(struct page *, struct page *);
 +int truncate_inline_data(struct inode *, u64 from);
  int f2fs_read_inline_data(struct inode *, struct page *);
  int f2fs_convert_inline_page(struct dnode_of_data *, struct page *);
  int f2fs_convert_inline_inode(struct inode *);
 diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
 index f1341c7..6c4443e 100644
 --- a/fs/f2fs/file.c
 +++ b/fs/f2fs/file.c
 @@ -461,6 +461,9 @@ static int truncate_partial_data_page(struct inode 
 *inode, u64 from)
   unsigned offset = from  (PAGE_CACHE_SIZE - 1);
   struct page *page;
  
 + if (f2fs_has_inline_data(inode))
 + return 0;
 +
   if (!offset)
   return 0;
  
 @@ -497,14 +500,14 @@ int truncate_blocks(struct inode *inode, u64 from, bool 
 lock)
   if (lock)
   f2fs_lock_op(sbi);
  
 - ipage = get_node_page(sbi, inode-i_ino);
 - if (IS_ERR(ipage)) {
 - err = PTR_ERR(ipage);
 + if (f2fs_has_inline_data(inode)) {
 + err = truncate_inline_data(inode, from);
   goto out;
   }
  
 - if (f2fs_has_inline_data(inode)) {
 - f2fs_put_page(ipage, 1);
 + ipage = get_node_page(sbi, inode-i_ino);
 + if (IS_ERR(ipage)) {
 + err = PTR_ERR(ipage);
   goto out;
   }
  
 diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
 index 4ba9732..a8189c92 100644
 --- a/fs/f2fs/inline.c
 +++ b/fs/f2fs/inline.c
 @@ -50,10 +50,57 @@ void read_inline_data(struct page *page, struct page 
 *ipage)
   SetPageUptodate(page);
  }
  
 -static void truncate_inline_data(struct page *ipage)
 +static void truncate_inline_inode(struct page *ipage, u64 from)
  {
 + void *addr = inline_data_addr(ipage);
 +
   f2fs_wait_on_page_writeback(ipage, NODE);
 - memset(inline_data_addr(ipage), 0, MAX_INLINE_DATA);
 + memset(addr + from, 0, MAX_INLINE_DATA - from);
 +}
 +
 +int truncate_inline_data(struct inode *inode, u64 from)
 +{
 + struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 + struct page *page, *ipage;
 + void *addr;
 + int err = 0;
 +
 + /*
 +  * 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(sbi, from  MAX_INLINE_DATA);
 +
 + if (from == MAX_INLINE_DATA)
 + return 0;
 +
 + page = grab_cache_page(inode-i_mapping, 0);
 + if (!page)
 + return -ENOMEM;

Do we need to grab and read #0 page?
How about checking its cached page after on-disk truncation?

1. page = get_node_page()
2. check_inline
3. truncate_inline_inode
4. f2fs_put_page(ipage)

5. truncate_partial_data_page(inode, from, force)
if (!offset  !force)
return 0;

 +
 + ipage = get_node_page(sbi, inode-i_ino);
 + if (IS_ERR(ipage)) {
 + err = PTR_ERR(ipage);
 + goto out;
 + }
 +
 + if (!f2fs_has_inline_data(inode)) {
 + f2fs_put_page(ipage, 1);
 + goto out;
 + }
 +
 + truncate_inline_inode(ipage, from);
 + set_page_dirty(ipage);
 +
 + read_inline_data(page, ipage);
 + f2fs_put_page(ipage, 1);
 +
 + addr = inline_data_addr(page);
 + memset(addr + 

[PATCH] f2fs: fix to truncate inline data past EOF

2015-03-05 Thread Chao Yu
Previously if inode is with inline data, we will try to invalid partial inline
data in page #0 when we truncate size of inode in truncate_partial_data_page().
And then we set page #0 to dirty, after this we can synchronize inode page with
page #0 at -writepage().

But sometimes we will fail to operate page #0 in truncate_partial_data_page()
due to below reason:
a) if offset is zero, we will skip setting page #0 to dirty.
b) if page #0 is not uptodate, we will fail to update it as it has no mapping
data.

So with following operations, we will meet recent data which should be
truncated.

1.write inline data to file
2.sync first data page to inode page
3.truncate file size to 0
4.truncate file size to max_inline_size
5.echo 1  /proc/sys/vm/drop_caches
6.read file -- meet original inline data which is remained in inode page.

This patch renames truncate_inline_data() to truncate_inline_inode() for code
readability, then intruduces new truncate_inline_data() and use it to truncate
inline data in page #0 and inode page in truncate_partial_data_page() for
fixing.

Signed-off-by: Chao Yu chao2...@samsung.com
---
 fs/f2fs/f2fs.h   |  1 +
 fs/f2fs/file.c   | 13 -
 fs/f2fs/inline.c | 57 +++-
 3 files changed, 61 insertions(+), 10 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 511d6cd..55dbbe7 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1754,6 +1754,7 @@ extern struct kmem_cache *inode_entry_slab;
  */
 bool f2fs_may_inline(struct inode *);
 void read_inline_data(struct page *, struct page *);
+int truncate_inline_data(struct inode *, u64 from);
 int f2fs_read_inline_data(struct inode *, struct page *);
 int f2fs_convert_inline_page(struct dnode_of_data *, struct page *);
 int f2fs_convert_inline_inode(struct inode *);
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index f1341c7..6c4443e 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -461,6 +461,9 @@ static int truncate_partial_data_page(struct inode *inode, 
u64 from)
unsigned offset = from  (PAGE_CACHE_SIZE - 1);
struct page *page;
 
+   if (f2fs_has_inline_data(inode))
+   return 0;
+
if (!offset)
return 0;
 
@@ -497,14 +500,14 @@ int truncate_blocks(struct inode *inode, u64 from, bool 
lock)
if (lock)
f2fs_lock_op(sbi);
 
-   ipage = get_node_page(sbi, inode-i_ino);
-   if (IS_ERR(ipage)) {
-   err = PTR_ERR(ipage);
+   if (f2fs_has_inline_data(inode)) {
+   err = truncate_inline_data(inode, from);
goto out;
}
 
-   if (f2fs_has_inline_data(inode)) {
-   f2fs_put_page(ipage, 1);
+   ipage = get_node_page(sbi, inode-i_ino);
+   if (IS_ERR(ipage)) {
+   err = PTR_ERR(ipage);
goto out;
}
 
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 4ba9732..a8189c92 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -50,10 +50,57 @@ void read_inline_data(struct page *page, struct page *ipage)
SetPageUptodate(page);
 }
 
-static void truncate_inline_data(struct page *ipage)
+static void truncate_inline_inode(struct page *ipage, u64 from)
 {
+   void *addr = inline_data_addr(ipage);
+
f2fs_wait_on_page_writeback(ipage, NODE);
-   memset(inline_data_addr(ipage), 0, MAX_INLINE_DATA);
+   memset(addr + from, 0, MAX_INLINE_DATA - from);
+}
+
+int truncate_inline_data(struct inode *inode, u64 from)
+{
+   struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
+   struct page *page, *ipage;
+   void *addr;
+   int err = 0;
+
+   /*
+* 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(sbi, from  MAX_INLINE_DATA);
+
+   if (from == MAX_INLINE_DATA)
+   return 0;
+
+   page = grab_cache_page(inode-i_mapping, 0);
+   if (!page)
+   return -ENOMEM;
+
+   ipage = get_node_page(sbi, inode-i_ino);
+   if (IS_ERR(ipage)) {
+   err = PTR_ERR(ipage);
+   goto out;
+   }
+
+   if (!f2fs_has_inline_data(inode)) {
+   f2fs_put_page(ipage, 1);
+   goto out;
+   }
+
+   truncate_inline_inode(ipage, from);
+   set_page_dirty(ipage);
+
+   read_inline_data(page, ipage);
+   f2fs_put_page(ipage, 1);
+
+   addr = inline_data_addr(page);
+   memset(addr + from, 0, MAX_INLINE_DATA - from);
+out:
+   f2fs_put_page(page, 1);
+   return err;
 }
 
 int f2fs_read_inline_data(struct inode *inode, struct page *page)
@@ -131,7 +178,7 @@ no_update:
set_inode_flag(F2FS_I(dn-inode), FI_APPEND_WRITE);
 
/* clear inline data and flag after data writeback */
-   truncate_inline_data(dn-inode_page);
+