Previous we do not truncate inline data in inode page when setattr, so following
case could still read the inline data which has already truncated:
1.write inline data
2.ftruncate size to 0
3.ftruncate size to max inline data size
4.read from offset 0
This patch introduces truncate_inline_data() to fix this problem.
change log from v1:
o fix a bug and do not truncate first page data after truncate inline data.
change log from v2:
o wait to writeback for inode page if it was already submitted.
Signed-off-by: Chao Yu <[email protected]>
---
fs/f2fs/f2fs.h | 1 +
fs/f2fs/file.c | 3 +++
fs/f2fs/inline.c | 19 +++++++++++++++++++
3 files changed, 23 insertions(+)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 2b67679..676a2c6 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1410,5 +1410,6 @@ bool f2fs_may_inline(struct inode *);
int f2fs_read_inline_data(struct inode *, struct page *);
int f2fs_convert_inline_data(struct inode *, pgoff_t);
int f2fs_write_inline_data(struct inode *, struct page *, unsigned int);
+void truncate_inline_data(struct inode *, u64);
int recover_inline_data(struct inode *, struct page *);
#endif
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index b9f4fbf..d97e5c4 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -369,6 +369,9 @@ static void 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 truncate_inline_data(inode, from);
+
if (!offset)
return;
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 3258c7c..6a13997 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -176,6 +176,25 @@ int f2fs_write_inline_data(struct inode *inode,
return 0;
}
+void truncate_inline_data(struct inode *inode, u64 from)
+{
+ struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
+ struct page *ipage;
+
+ if (from >= MAX_INLINE_DATA)
+ return;
+
+ ipage = get_node_page(sbi, inode->i_ino);
+ if (IS_ERR(ipage))
+ return;
+
+ f2fs_wait_on_page_writeback(ipage, NODE);
+ zero_user_segment(ipage, INLINE_DATA_OFFSET + from,
+ INLINE_DATA_OFFSET + MAX_INLINE_DATA);
+ set_page_dirty(ipage);
+ f2fs_put_page(ipage, 1);
+}
+
int recover_inline_data(struct inode *inode, struct page *npage)
{
struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
--
1.7.9.5
Hi,
2014-04-29 (화), 14:03 +0800, Chao Yu:
> Previous we do not truncate inline data in inode page when setattr, so following
> case could still read the inline data which has already truncated:
>
> 1.write inline data
> 2.ftruncate size to 0
> 3.ftruncate size to max inline data size
> 4.read from offset 0
>
> This patch introduces truncate_inline_data() to fix this problem.
>
> change log from v1:
> o fix a bug and do not truncate first page data after truncate inline data.
> change log from v2:
> o wait to writeback for inode page if it was already submitted.
>
> Signed-off-by: Chao Yu <[email protected]>
> ---
> fs/f2fs/f2fs.h | 1 +
> fs/f2fs/file.c | 3 +++
> fs/f2fs/inline.c | 19 +++++++++++++++++++
> 3 files changed, 23 insertions(+)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 2b67679..676a2c6 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1410,5 +1410,6 @@ bool f2fs_may_inline(struct inode *);
> int f2fs_read_inline_data(struct inode *, struct page *);
> int f2fs_convert_inline_data(struct inode *, pgoff_t);
> int f2fs_write_inline_data(struct inode *, struct page *, unsigned int);
> +void truncate_inline_data(struct inode *, u64);
> int recover_inline_data(struct inode *, struct page *);
> #endif
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index b9f4fbf..d97e5c4 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -369,6 +369,9 @@ static void 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 truncate_inline_data(inode, from);
> +
> if (!offset)
> return;
>
> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> index 3258c7c..6a13997 100644
> --- a/fs/f2fs/inline.c
> +++ b/fs/f2fs/inline.c
> @@ -176,6 +176,25 @@ int f2fs_write_inline_data(struct inode *inode,
> return 0;
> }
>
> +void truncate_inline_data(struct inode *inode, u64 from)
> +{
> + struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
> + struct page *ipage;
> +
> + if (from >= MAX_INLINE_DATA)
> + return;
> +
> + ipage = get_node_page(sbi, inode->i_ino);
> + if (IS_ERR(ipage))
> + return;
> +
> + f2fs_wait_on_page_writeback(ipage, NODE);
The get_node_page() triggers,
grab_cache_page_write_begin()
-> wait_for_stable_page()
-> wait_on_page_writeback().
Any bugs in there?
> + zero_user_segment(ipage, INLINE_DATA_OFFSET + from,
> + INLINE_DATA_OFFSET + MAX_INLINE_DATA);
> + set_page_dirty(ipage);
> + f2fs_put_page(ipage, 1);
> +}
> +
> int recover_inline_data(struct inode *inode, struct page *npage)
> {
> struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
--
Jaegeuk Kim
Samsung
Hi Kim,
> -----Original Message-----
> From: Jaegeuk Kim [mailto:[email protected]]
> Sent: Tuesday, April 29, 2014 2:16 PM
> To: Chao Yu
> Cc: [email protected]; [email protected];
> [email protected]
> Subject: Re: [f2fs-dev][PATCH 3/3 v3] f2fs: fix to truncate inline data in inode page when setattr
>
> Hi,
>
> 2014-04-29 (화), 14:03 +0800, Chao Yu:
> > Previous we do not truncate inline data in inode page when setattr, so following
> > case could still read the inline data which has already truncated:
> >
> > 1.write inline data
> > 2.ftruncate size to 0
> > 3.ftruncate size to max inline data size
> > 4.read from offset 0
> >
> > This patch introduces truncate_inline_data() to fix this problem.
> >
> > change log from v1:
> > o fix a bug and do not truncate first page data after truncate inline data.
> > change log from v2:
> > o wait to writeback for inode page if it was already submitted.
> >
> > Signed-off-by: Chao Yu <[email protected]>
> > ---
> > fs/f2fs/f2fs.h | 1 +
> > fs/f2fs/file.c | 3 +++
> > fs/f2fs/inline.c | 19 +++++++++++++++++++
> > 3 files changed, 23 insertions(+)
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 2b67679..676a2c6 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -1410,5 +1410,6 @@ bool f2fs_may_inline(struct inode *);
> > int f2fs_read_inline_data(struct inode *, struct page *);
> > int f2fs_convert_inline_data(struct inode *, pgoff_t);
> > int f2fs_write_inline_data(struct inode *, struct page *, unsigned int);
> > +void truncate_inline_data(struct inode *, u64);
> > int recover_inline_data(struct inode *, struct page *);
> > #endif
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index b9f4fbf..d97e5c4 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -369,6 +369,9 @@ static void 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 truncate_inline_data(inode, from);
> > +
> > if (!offset)
> > return;
> >
> > diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> > index 3258c7c..6a13997 100644
> > --- a/fs/f2fs/inline.c
> > +++ b/fs/f2fs/inline.c
> > @@ -176,6 +176,25 @@ int f2fs_write_inline_data(struct inode *inode,
> > return 0;
> > }
> >
> > +void truncate_inline_data(struct inode *inode, u64 from)
> > +{
> > + struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
> > + struct page *ipage;
> > +
> > + if (from >= MAX_INLINE_DATA)
> > + return;
> > +
> > + ipage = get_node_page(sbi, inode->i_ino);
> > + if (IS_ERR(ipage))
> > + return;
> > +
> > + f2fs_wait_on_page_writeback(ipage, NODE);
>
> The get_node_page() triggers,
> grab_cache_page_write_begin()
> -> wait_for_stable_page()
> -> wait_on_page_writeback().
Oh, it's my mistake for this missing, thanks for the mention.:)
>
> Any bugs in there?
If we wait in grab_cache_page_write_begin(), maybe we will face long time delay
or potential deadlock caused by f2fs bio cache. So how about use
grab_cache_page & f2fs_wait_on_page_writeback to avoid this?
>
> > + zero_user_segment(ipage, INLINE_DATA_OFFSET + from,
> > + INLINE_DATA_OFFSET + MAX_INLINE_DATA);
> > + set_page_dirty(ipage);
> > + f2fs_put_page(ipage, 1);
> > +}
> > +
> > int recover_inline_data(struct inode *inode, struct page *npage)
> > {
> > struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
>
> --
> Jaegeuk Kim
> Samsung
Hi,
2014-04-29 (화), 15:53 +0800, Chao Yu:
> Hi Kim,
>
> > -----Original Message-----
> > From: Jaegeuk Kim [mailto:[email protected]]
> > Sent: Tuesday, April 29, 2014 2:16 PM
> > To: Chao Yu
> > Cc: [email protected]; [email protected];
> > [email protected]
> > Subject: Re: [f2fs-dev][PATCH 3/3 v3] f2fs: fix to truncate inline data in inode page when setattr
> >
> > Hi,
> >
> > 2014-04-29 (화), 14:03 +0800, Chao Yu:
> > > Previous we do not truncate inline data in inode page when setattr, so following
> > > case could still read the inline data which has already truncated:
> > >
> > > 1.write inline data
> > > 2.ftruncate size to 0
> > > 3.ftruncate size to max inline data size
> > > 4.read from offset 0
> > >
> > > This patch introduces truncate_inline_data() to fix this problem.
> > >
> > > change log from v1:
> > > o fix a bug and do not truncate first page data after truncate inline data.
> > > change log from v2:
> > > o wait to writeback for inode page if it was already submitted.
> > >
> > > Signed-off-by: Chao Yu <[email protected]>
> > > ---
> > > fs/f2fs/f2fs.h | 1 +
> > > fs/f2fs/file.c | 3 +++
> > > fs/f2fs/inline.c | 19 +++++++++++++++++++
> > > 3 files changed, 23 insertions(+)
> > >
> > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > index 2b67679..676a2c6 100644
> > > --- a/fs/f2fs/f2fs.h
> > > +++ b/fs/f2fs/f2fs.h
> > > @@ -1410,5 +1410,6 @@ bool f2fs_may_inline(struct inode *);
> > > int f2fs_read_inline_data(struct inode *, struct page *);
> > > int f2fs_convert_inline_data(struct inode *, pgoff_t);
> > > int f2fs_write_inline_data(struct inode *, struct page *, unsigned int);
> > > +void truncate_inline_data(struct inode *, u64);
> > > int recover_inline_data(struct inode *, struct page *);
> > > #endif
> > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > > index b9f4fbf..d97e5c4 100644
> > > --- a/fs/f2fs/file.c
> > > +++ b/fs/f2fs/file.c
> > > @@ -369,6 +369,9 @@ static void 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 truncate_inline_data(inode, from);
> > > +
> > > if (!offset)
> > > return;
> > >
> > > diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> > > index 3258c7c..6a13997 100644
> > > --- a/fs/f2fs/inline.c
> > > +++ b/fs/f2fs/inline.c
> > > @@ -176,6 +176,25 @@ int f2fs_write_inline_data(struct inode *inode,
> > > return 0;
> > > }
> > >
> > > +void truncate_inline_data(struct inode *inode, u64 from)
> > > +{
> > > + struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
> > > + struct page *ipage;
> > > +
> > > + if (from >= MAX_INLINE_DATA)
> > > + return;
> > > +
> > > + ipage = get_node_page(sbi, inode->i_ino);
> > > + if (IS_ERR(ipage))
> > > + return;
> > > +
> > > + f2fs_wait_on_page_writeback(ipage, NODE);
> >
> > The get_node_page() triggers,
> > grab_cache_page_write_begin()
> > -> wait_for_stable_page()
> > -> wait_on_page_writeback().
>
> Oh, it's my mistake for this missing, thanks for the mention.:)
>
> >
> > Any bugs in there?
>
> If we wait in grab_cache_page_write_begin(), maybe we will face long time delay
> or potential deadlock caused by f2fs bio cache. So how about use
> grab_cache_page & f2fs_wait_on_page_writeback to avoid this?
Nice catch. :)
IMO, it'd better merge your v2, and let me investigate all the paths to
avoid that.
Is it okay?
>
> >
> > > + zero_user_segment(ipage, INLINE_DATA_OFFSET + from,
> > > + INLINE_DATA_OFFSET + MAX_INLINE_DATA);
> > > + set_page_dirty(ipage);
> > > + f2fs_put_page(ipage, 1);
> > > +}
> > > +
> > > int recover_inline_data(struct inode *inode, struct page *npage)
> > > {
> > > struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
> >
> > --
> > Jaegeuk Kim
> > Samsung
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Jaegeuk Kim
Samsung
Hi Kim,
> -----Original Message-----
> From: Jaegeuk Kim [mailto:[email protected]]
> Sent: Tuesday, April 29, 2014 4:02 PM
> To: Chao Yu
> Cc: [email protected]; [email protected];
> [email protected]
> Subject: RE: [f2fs-dev][PATCH 3/3 v3] f2fs: fix to truncate inline data in inode page when setattr
>
> Hi,
>
> 2014-04-29 (화), 15:53 +0800, Chao Yu:
> > Hi Kim,
> >
> > > -----Original Message-----
> > > From: Jaegeuk Kim [mailto:[email protected]]
> > > Sent: Tuesday, April 29, 2014 2:16 PM
> > > To: Chao Yu
> > > Cc: [email protected]; [email protected];
> > > [email protected]
> > > Subject: Re: [f2fs-dev][PATCH 3/3 v3] f2fs: fix to truncate inline data in inode page when
> setattr
> > >
> > > Hi,
> > >
> > > 2014-04-29 (화), 14:03 +0800, Chao Yu:
> > > > Previous we do not truncate inline data in inode page when setattr, so following
> > > > case could still read the inline data which has already truncated:
> > > >
> > > > 1.write inline data
> > > > 2.ftruncate size to 0
> > > > 3.ftruncate size to max inline data size
> > > > 4.read from offset 0
> > > >
> > > > This patch introduces truncate_inline_data() to fix this problem.
> > > >
> > > > change log from v1:
> > > > o fix a bug and do not truncate first page data after truncate inline data.
> > > > change log from v2:
> > > > o wait to writeback for inode page if it was already submitted.
> > > >
> > > > Signed-off-by: Chao Yu <[email protected]>
> > > > ---
> > > > fs/f2fs/f2fs.h | 1 +
> > > > fs/f2fs/file.c | 3 +++
> > > > fs/f2fs/inline.c | 19 +++++++++++++++++++
> > > > 3 files changed, 23 insertions(+)
> > > >
> > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > > index 2b67679..676a2c6 100644
> > > > --- a/fs/f2fs/f2fs.h
> > > > +++ b/fs/f2fs/f2fs.h
> > > > @@ -1410,5 +1410,6 @@ bool f2fs_may_inline(struct inode *);
> > > > int f2fs_read_inline_data(struct inode *, struct page *);
> > > > int f2fs_convert_inline_data(struct inode *, pgoff_t);
> > > > int f2fs_write_inline_data(struct inode *, struct page *, unsigned int);
> > > > +void truncate_inline_data(struct inode *, u64);
> > > > int recover_inline_data(struct inode *, struct page *);
> > > > #endif
> > > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > > > index b9f4fbf..d97e5c4 100644
> > > > --- a/fs/f2fs/file.c
> > > > +++ b/fs/f2fs/file.c
> > > > @@ -369,6 +369,9 @@ static void 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 truncate_inline_data(inode, from);
> > > > +
> > > > if (!offset)
> > > > return;
> > > >
> > > > diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> > > > index 3258c7c..6a13997 100644
> > > > --- a/fs/f2fs/inline.c
> > > > +++ b/fs/f2fs/inline.c
> > > > @@ -176,6 +176,25 @@ int f2fs_write_inline_data(struct inode *inode,
> > > > return 0;
> > > > }
> > > >
> > > > +void truncate_inline_data(struct inode *inode, u64 from)
> > > > +{
> > > > + struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
> > > > + struct page *ipage;
> > > > +
> > > > + if (from >= MAX_INLINE_DATA)
> > > > + return;
> > > > +
> > > > + ipage = get_node_page(sbi, inode->i_ino);
> > > > + if (IS_ERR(ipage))
> > > > + return;
> > > > +
> > > > + f2fs_wait_on_page_writeback(ipage, NODE);
> > >
> > > The get_node_page() triggers,
> > > grab_cache_page_write_begin()
> > > -> wait_for_stable_page()
> > > -> wait_on_page_writeback().
> >
> > Oh, it's my mistake for this missing, thanks for the mention.:)
> >
> > >
> > > Any bugs in there?
> >
> > If we wait in grab_cache_page_write_begin(), maybe we will face long time delay
> > or potential deadlock caused by f2fs bio cache. So how about use
> > grab_cache_page & f2fs_wait_on_page_writeback to avoid this?
>
> Nice catch. :)
> IMO, it'd better merge your v2, and let me investigate all the paths to
> avoid that.
> Is it okay?
It's okay, and thanks for your review. :)
Regards.
>
> >
> > >
> > > > + zero_user_segment(ipage, INLINE_DATA_OFFSET + from,
> > > > + INLINE_DATA_OFFSET + MAX_INLINE_DATA);
> > > > + set_page_dirty(ipage);
> > > > + f2fs_put_page(ipage, 1);
> > > > +}
> > > > +
> > > > int recover_inline_data(struct inode *inode, struct page *npage)
> > > > {
> > > > struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
> > >
> > > --
> > > Jaegeuk Kim
> > > Samsung
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
>
> --
> Jaegeuk Kim
> Samsung