RE: [f2fs-dev][PATCH 3/3 v3] f2fs: fix to truncate inline data in inode page when setattr
Hi Kim, > -Original Message- > From: Jaegeuk Kim [mailto:jaegeuk@samsung.com] > Sent: Tuesday, April 29, 2014 4:02 PM > To: Chao Yu > Cc: linux-f2fs-de...@lists.sourceforge.net; linux-fsde...@vger.kernel.org; > linux-kernel@vger.kernel.org > 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:jaegeuk@samsung.com] > > > Sent: Tuesday, April 29, 2014 2:16 PM > > > To: Chao Yu > > > Cc: linux-f2fs-de...@lists.sourceforge.net; linux-fsde...@vger.kernel.org; > > > linux-kernel@vger.kernel.org > > > 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 > > > > --- > > > > 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?
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:jaegeuk@samsung.com] > > Sent: Tuesday, April 29, 2014 2:16 PM > > To: Chao Yu > > Cc: linux-f2fs-de...@lists.sourceforge.net; linux-fsde...@vger.kernel.org; > > linux-kernel@vger.kernel.org > > 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 > > > --- > > > 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 majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- Jaegeuk Kim Samsung -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [f2fs-dev][PATCH 3/3 v3] f2fs: fix to truncate inline data in inode page when setattr
Hi Kim, > -Original Message- > From: Jaegeuk Kim [mailto:jaegeuk@samsung.com] > Sent: Tuesday, April 29, 2014 2:16 PM > To: Chao Yu > Cc: linux-f2fs-de...@lists.sourceforge.net; linux-fsde...@vger.kernel.org; > linux-kernel@vger.kernel.org > 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 > > --- > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 > --- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 chao2...@samsung.com --- 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [f2fs-dev][PATCH 3/3 v3] f2fs: fix to truncate inline data in inode page when setattr
Hi Kim, -Original Message- From: Jaegeuk Kim [mailto:jaegeuk@samsung.com] Sent: Tuesday, April 29, 2014 2:16 PM To: Chao Yu Cc: linux-f2fs-de...@lists.sourceforge.net; linux-fsde...@vger.kernel.org; linux-kernel@vger.kernel.org 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 chao2...@samsung.com --- 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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:jaegeuk@samsung.com] Sent: Tuesday, April 29, 2014 2:16 PM To: Chao Yu Cc: linux-f2fs-de...@lists.sourceforge.net; linux-fsde...@vger.kernel.org; linux-kernel@vger.kernel.org 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 chao2...@samsung.com --- 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- Jaegeuk Kim Samsung -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [f2fs-dev][PATCH 3/3 v3] f2fs: fix to truncate inline data in inode page when setattr
Hi Kim, -Original Message- From: Jaegeuk Kim [mailto:jaegeuk@samsung.com] Sent: Tuesday, April 29, 2014 4:02 PM To: Chao Yu Cc: linux-f2fs-de...@lists.sourceforge.net; linux-fsde...@vger.kernel.org; linux-kernel@vger.kernel.org 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:jaegeuk@samsung.com] Sent: Tuesday, April 29, 2014 2:16 PM To: Chao Yu Cc: linux-f2fs-de...@lists.sourceforge.net; linux-fsde...@vger.kernel.org; linux-kernel@vger.kernel.org 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 chao2...@samsung.com --- 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- Jaegeuk Kim Samsung -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/