Otherwise, if a large amount of direct IO writes were done, the
segment allocation may be failed because no enough segments are gced.
Signed-off-by: Huang, Ying <[email protected]>
---
fs/f2fs/data.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1038,6 +1038,7 @@ static ssize_t f2fs_direct_IO(int rw, st
{
struct file *file = iocb->ki_filp;
struct inode *inode = file->f_mapping->host;
+ struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
/* Let buffer I/O handle the inline data case. */
if (f2fs_has_inline_data(inode))
@@ -1046,8 +1047,11 @@ static ssize_t f2fs_direct_IO(int rw, st
if (check_direct_IO(inode, rw, iter, offset))
return 0;
+ if (rw == WRITE)
+ f2fs_balance_fs(sbi);
+
/* clear fsync mark to recover these blocks */
- fsync_mark_clear(F2FS_SB(inode->i_sb), inode->i_ino);
+ fsync_mark_clear(sbi, inode->i_ino);
return blockdev_direct_IO(rw, iocb, inode, iter, offset,
get_data_block);
Hi Huang,
It occurs a merge conflict.
Please check the patch.
Anyway, IMO, it needs to place f2fs_balance_fs somewhere in get_data_block.
Thanks,
On Mon, Jul 07, 2014 at 01:46:28PM +0800, Huang Ying wrote:
> Otherwise, if a large amount of direct IO writes were done, the
> segment allocation may be failed because no enough segments are gced.
>
> Signed-off-by: Huang, Ying <[email protected]>
> ---
> fs/f2fs/data.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1038,6 +1038,7 @@ static ssize_t f2fs_direct_IO(int rw, st
> {
> struct file *file = iocb->ki_filp;
> struct inode *inode = file->f_mapping->host;
> + struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
>
> /* Let buffer I/O handle the inline data case. */
> if (f2fs_has_inline_data(inode))
> @@ -1046,8 +1047,11 @@ static ssize_t f2fs_direct_IO(int rw, st
> if (check_direct_IO(inode, rw, iter, offset))
> return 0;
>
> + if (rw == WRITE)
> + f2fs_balance_fs(sbi);
> +
> /* clear fsync mark to recover these blocks */
> - fsync_mark_clear(F2FS_SB(inode->i_sb), inode->i_ino);
> + fsync_mark_clear(sbi, inode->i_ino);
>
> return blockdev_direct_IO(rw, iocb, inode, iter, offset,
> get_data_block);
--
Jaegeuk Kim
Hi, Jaegeuk,
On Tue, 2014-07-08 at 00:58 -0700, Jaegeuk Kim wrote:
> Hi Huang,
>
> It occurs a merge conflict.
> Please check the patch.
Sorry, my fault. I think I need to base my patch on f2fs tree instead
of latest Linus' tree?
> Anyway, IMO, it needs to place f2fs_balance_fs somewhere in get_data_block.
Yes. I think so too. So you prefer to add f2fs_balance_fs in both
f2fs_direct_IO and get_data_block, or just add f2fs_blance_fs in
get_data_block only?
Best Regards,
Huang, Ying
> Thanks,
>
> On Mon, Jul 07, 2014 at 01:46:28PM +0800, Huang Ying wrote:
> > Otherwise, if a large amount of direct IO writes were done, the
> > segment allocation may be failed because no enough segments are gced.
> >
> > Signed-off-by: Huang, Ying <[email protected]>
> > ---
> > fs/f2fs/data.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -1038,6 +1038,7 @@ static ssize_t f2fs_direct_IO(int rw, st
> > {
> > struct file *file = iocb->ki_filp;
> > struct inode *inode = file->f_mapping->host;
> > + struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
> >
> > /* Let buffer I/O handle the inline data case. */
> > if (f2fs_has_inline_data(inode))
> > @@ -1046,8 +1047,11 @@ static ssize_t f2fs_direct_IO(int rw, st
> > if (check_direct_IO(inode, rw, iter, offset))
> > return 0;
> >
> > + if (rw == WRITE)
> > + f2fs_balance_fs(sbi);
> > +
> > /* clear fsync mark to recover these blocks */
> > - fsync_mark_clear(F2FS_SB(inode->i_sb), inode->i_ino);
> > + fsync_mark_clear(sbi, inode->i_ino);
> >
> > return blockdev_direct_IO(rw, iocb, inode, iter, offset,
> > get_data_block);
>
Hi Huang,
> -----Original Message-----
> From: Huang Ying [mailto:[email protected]]
> Sent: Tuesday, July 08, 2014 4:17 PM
> To: Jaegeuk Kim
> Cc: [email protected]; [email protected]
> Subject: Re: [f2fs-dev] f2fs: Add f2fs_balance_fs for direct IO
>
> Hi, Jaegeuk,
>
> On Tue, 2014-07-08 at 00:58 -0700, Jaegeuk Kim wrote:
> > Hi Huang,
> >
> > It occurs a merge conflict.
> > Please check the patch.
>
> Sorry, my fault. I think I need to base my patch on f2fs tree instead
> of latest Linus' tree?
I think you patch is not conflict to both latest linux's tree and f2fs dev tree.
but conflict to this patch
"f2fs: introduce f2fs_write_failed to handle error case when write" which
maybe in Jaegeuk's local tree, not update to f2fs dev tree.
If possible, you could wait for Jaegeuk to update f2fs tree, or merge this patch,
then apply your new patch.
Thanks,
Yu
>
> > Anyway, IMO, it needs to place f2fs_balance_fs somewhere in get_data_block.
>
> Yes. I think so too. So you prefer to add f2fs_balance_fs in both
> f2fs_direct_IO and get_data_block, or just add f2fs_blance_fs in
> get_data_block only?
>
> Best Regards,
> Huang, Ying
>
> > Thanks,
> >
> > On Mon, Jul 07, 2014 at 01:46:28PM +0800, Huang Ying wrote:
> > > Otherwise, if a large amount of direct IO writes were done, the
> > > segment allocation may be failed because no enough segments are gced.
> > >
> > > Signed-off-by: Huang, Ying <[email protected]>
> > > ---
> > > fs/f2fs/data.c | 6 +++++-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > --- a/fs/f2fs/data.c
> > > +++ b/fs/f2fs/data.c
> > > @@ -1038,6 +1038,7 @@ static ssize_t f2fs_direct_IO(int rw, st
> > > {
> > > struct file *file = iocb->ki_filp;
> > > struct inode *inode = file->f_mapping->host;
> > > + struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
> > >
> > > /* Let buffer I/O handle the inline data case. */
> > > if (f2fs_has_inline_data(inode))
> > > @@ -1046,8 +1047,11 @@ static ssize_t f2fs_direct_IO(int rw, st
> > > if (check_direct_IO(inode, rw, iter, offset))
> > > return 0;
> > >
> > > + if (rw == WRITE)
> > > + f2fs_balance_fs(sbi);
> > > +
> > > /* clear fsync mark to recover these blocks */
> > > - fsync_mark_clear(F2FS_SB(inode->i_sb), inode->i_ino);
> > > + fsync_mark_clear(sbi, inode->i_ino);
> > >
> > > return blockdev_direct_IO(rw, iocb, inode, iter, offset,
> > > get_data_block);
> >
>
>
>
> ------------------------------------------------------------------------------
> Open source business process management suite built on Java and Eclipse
> Turn processes into business applications with Bonita BPM Community Edition
> Quickly connect people, data, and systems into organized workflows
> Winner of BOSSIE, CODIE, OW2 and Gartner awards
> http://p.sf.net/sfu/Bonitasoft
> _______________________________________________
> Linux-f2fs-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
On Tue, Jul 08, 2014 at 04:16:52PM +0800, Huang Ying wrote:
> Hi, Jaegeuk,
>
> On Tue, 2014-07-08 at 00:58 -0700, Jaegeuk Kim wrote:
> > Hi Huang,
> >
> > It occurs a merge conflict.
> > Please check the patch.
>
> Sorry, my fault. I think I need to base my patch on f2fs tree instead
> of latest Linus' tree?
My apologies.
As Chao said, it was due to the other patch.
I'll merge them soon.
>
> > Anyway, IMO, it needs to place f2fs_balance_fs somewhere in get_data_block.
>
> Yes. I think so too. So you prefer to add f2fs_balance_fs in both
> f2fs_direct_IO and get_data_block, or just add f2fs_blance_fs in
> get_data_block only?
I think it should be placed on get_data_block only.
But, at a glance, we need to figure out some conditions when it should be
called.
Since we don't or shouldn't call that in the look-up paths (e.g., read).
Thanks,
>
> Best Regards,
> Huang, Ying
>
> > Thanks,
> >
> > On Mon, Jul 07, 2014 at 01:46:28PM +0800, Huang Ying wrote:
> > > Otherwise, if a large amount of direct IO writes were done, the
> > > segment allocation may be failed because no enough segments are gced.
> > >
> > > Signed-off-by: Huang, Ying <[email protected]>
> > > ---
> > > fs/f2fs/data.c | 6 +++++-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > --- a/fs/f2fs/data.c
> > > +++ b/fs/f2fs/data.c
> > > @@ -1038,6 +1038,7 @@ static ssize_t f2fs_direct_IO(int rw, st
> > > {
> > > struct file *file = iocb->ki_filp;
> > > struct inode *inode = file->f_mapping->host;
> > > + struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
> > >
> > > /* Let buffer I/O handle the inline data case. */
> > > if (f2fs_has_inline_data(inode))
> > > @@ -1046,8 +1047,11 @@ static ssize_t f2fs_direct_IO(int rw, st
> > > if (check_direct_IO(inode, rw, iter, offset))
> > > return 0;
> > >
> > > + if (rw == WRITE)
> > > + f2fs_balance_fs(sbi);
> > > +
> > > /* clear fsync mark to recover these blocks */
> > > - fsync_mark_clear(F2FS_SB(inode->i_sb), inode->i_ino);
> > > + fsync_mark_clear(sbi, inode->i_ino);
> > >
> > > return blockdev_direct_IO(rw, iocb, inode, iter, offset,
> > > get_data_block);
> >
--
Jaegeuk Kim