RE: [f2fs-dev] [PATCH] f2fs: introduce a new direct_IO write path

[Jaegeuk Kim <jaegeuk.kim@samsung.com>]

Hi,

2013-12-20 (금), 10:14 +0800, Chao Yu:
> Hi Kim,
> 
> One comment as following:
> 
> > -----Original Message-----
> > From: Jaegeuk Kim [mailto:jaegeuk.kim@samsung.com]
> > Sent: Wednesday, December 18, 2013 8:12 AM
> > To: linux-fsdevel@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org; linux-f2fs-devel@lists.sourceforge.net
> > Subject: Re: [f2fs-dev] [PATCH] f2fs: introduce a new direct_IO write path
> > 
> > Change log from v1:
> >  o fix NOSPC error handling
> > 
> > >From b8511a74fe98b67247a9feeed58441e8f5ffd705 Mon Sep 17 00:00:00 2001
> > From: Jaegeuk Kim <jaegeuk.kim@samsung.com>
> > Date: Mon, 16 Dec 2013 19:04:05 +0900
> > Subject: [PATCH] f2fs: introduce a new direct_IO write path
> > Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
> > linux-f2fs-devel@lists.sourceforge.net
> > 
> > Previously, f2fs doesn't support direct IOs with high performance, which
> > throws
> > every write requests via the buffered write path, resulting in highly
> > performance degradation due to memory opeations like copy_from_user.
> > 
> > This patch introduces a new direct IO path in which every write requests
> > are
> > processed by generic blockdev_direct_IO() with enhanced get_block
> > function.
> > 
> > The get_data_block() in f2fs handles:
> > 1. if original data blocks are allocates, then give them to blockdev.
> > 2. otherwise,
> >   a. preallocate requested block addresses
> >   b. do not use extent cache for better performance
> >   c. give the block addresses to blockdev
> > 
> > This policy induces that:
> > - new allocated data are sequentially written to the disk
> > - updated data are randomly written to the disk.
> > - f2fs gives consistency on its file meta, not file data.
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk.kim@samsung.com>
> > ---
> >  fs/f2fs/data.c    | 152
> > ++++++++++++++++++++++++++++++++++++++++--------------
> >  fs/f2fs/f2fs.h    |   2 +
> >  fs/f2fs/segment.c |  23 ++++++---
> >  3 files changed, 129 insertions(+), 48 deletions(-)
> > 
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 15956fa..a0950bc 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -570,74 +570,151 @@ repeat:
> >  	return page;
> >  }
> > 
> > +static int __allocate_data_block(struct dnode_of_data *dn)
> > +{
> > +	struct f2fs_sb_info *sbi = F2FS_SB(dn->inode->i_sb);
> > +	struct f2fs_summary sum;
> > +	block_t new_blkaddr;
> > +	struct node_info ni;
> > +	int type;
> > +
> > +	if (unlikely(is_inode_flag_set(F2FS_I(dn->inode), FI_NO_ALLOC)))
> > +		return -EPERM;
> > +	if (unlikely(!inc_valid_block_count(sbi, dn->inode, 1)))
> > +		return -ENOSPC;
> > +
> > +	__set_data_blkaddr(dn, NEW_ADDR);
> > +	dn->data_blkaddr = NEW_ADDR;
> > +
> > +	get_node_info(sbi, dn->nid, &ni);
> > +	set_summary(&sum, dn->nid, dn->ofs_in_node, ni.version);
> > +
> > +	type = CURSEG_WARM_DATA;
> 
> If so, our cold data will be written to WARM_DATA segment.
> How about check segment type here?

Actually, I'm not sure this kind of data requested through direct IOs
are cold or hot data.
But I just intended to gather such the direct IO'ed data into one type
of log, not separately.
So, I selected WARM_DATA by the fact that such the data will be
frequently updated through direct IO either.
Thanks,

> 
> > +
> > +	allocate_data_block(sbi, NULL, NULL_ADDR, &new_blkaddr, &sum, type);
> > +
> > +	/* direct IO doesn't use extent cache to maximize the performance */
> > +	set_inode_flag(F2FS_I(dn->inode), FI_NO_EXTENT);
> > +	update_extent_cache(new_blkaddr, dn);
> > +	clear_inode_flag(F2FS_I(dn->inode), FI_NO_EXTENT);
> > +
> > +	dn->data_blkaddr = new_blkaddr;
> > +	return 0;
> > +}
> > +
> 
> [snip]
> 

-- 
Jaegeuk Kim
Samsung