[<prev] [next>] [day] [month] [year] [list]
Message-id: <1370818554.3600.73.camel@kjgkr>
Date: Mon, 10 Jun 2013 07:55:54 +0900
From: Jaegeuk Kim <jaegeuk.kim@...sung.com>
To: Huajun Li <huajun.li.lee@...il.com>
Cc: Haicheng Li <haicheng.li@...ux.intel.com>,
Namjae Jeon <linkinjeon@...il.com>,
Huajun Li <huajun.li@...el.com>,
linux-fsdevel <linux-fsdevel@...r.kernel.org>,
namjae.jeon@...sung.com,
linux-kernel <linux-kernel@...r.kernel.org>,
linux-f2fs-devel <linux-f2fs-devel@...ts.sourceforge.net>
Subject: Re: [RFC 0/5] Enable f2fs support inline data
Hi,
2013-06-08 (토), 15:25 +0800, Huajun Li:
>
> Hi Jaegeuk,
> Thanks for your suggestion.
> On Wed, Jun 5, 2013 at 3:13 PM, Jaegeuk Kim <jaegeuk.kim@...sung.com>
> wrote:
> Hi Haicheng,
> 2013-06-04 (화), 14:01 +0800, Haicheng Li:
> > Hi Jaegeuk & Namjae,
> >
> > Sure, we'll address your comments. And this version is RFC,
> just wanna to
> > make sure this feature is meaningful for f2fs project, and
> there is no obvious
> > mistake, e.g. missing some critical path.
> IMO, it is worth to design and implement the inline feature
> though, I'd
> like to review the total design before looking at trivial
> mistakes.
> Since if the initial design is changed frequently, we need to
> review
> again and again.
>
> Agree. So let's understand/clarify your following proposal firstly.
> So, we need to decide the overall inline design.
> Currently we have to consider three data structures at a same
> time for
> the on-disk *inline* inode block structure, which are data,
> dentry, and
> xattr as well.
>
> IMO, we can give three inode flags: F2FS_INLINE_DATA,
> F2FS_INLINE_DENT,
> and F2FS_INLINE_XATTR.
>
> Small data and dentries can be inline. And xattr (such as XATTR_USER,
> XATTR_TRUSTED, eg. ) can be inline too, right ?
Right.
> Or inline xattr is just working for inline data/dentries?
>
> < on-disk inode block >
> - metadata
> - if F2FS_INLINE_XATTR is set,
> : use fixed 2*255 bytes to *cache* two xattrs for simplicity
>
> These 2 xattrs are working for the ones storing in xattr_nid block
> before, or just providing info for inline data/dent ?
I meant that we can store a fixed number of xattrs in the inode block
like *inline* data. The number, 2, is just for example.
If users give only one or two xattrs, we can store them in the inode
block instead of using extra node blocks.
If a system enables SELinux, it is able to set security labels to all
the files according to the policy, so that we need to consider inline
xattr seriously.
>
>
> `- if F2FS_INLINE_DATA is set,
> : use the remained space varied by F2FS_INLINE_XATTR.
> `- if F2FS_INLINE_DENT is set,
> : use variable number of dentries determined by
> F2FS_INLINE_XATTR.
> Do you mean inline dent depends on inline xattr, that is, we need a
> dedicated xattr entry providing info for inline denteries, right?
What I meant was the size of inline dentry space. We need to calculate
the size for inline dentries on-the-fly as the F2FS_INLINE_XATTR is set
or not.
>
>
> `- Otherwise, use as pointers
>
> And then, we need to define how to deal with their
> combinations.
>
> Operational conditions
> ----------------------
> - use inline xattr or not, by checking other inline flags and
> inline
> data size.
> - calculate inline data size and its offset according to the
> use of
> inline xattrs.
> - the places of inline operaions wrt the lock consistency and
> coverage
> - Power-off-recovery routine should care about the on-disk
> inode
> structure.
> - unset F2FS_INLINE_DATA if i_size = 0
> - unset F2FS_INLINE_XATTR if xattr entries = 0
> - unset F2FS_INLINE_DENT if dentries = 0
>
> - what else?
> - clear these flags after the inline data/dent is moved to normal data
> block
> - maybe we need store xattr_header while making xattr inline.
It seems that there is nothing to store any header information.
Thanks,
>
> Once we design the whole thing, we can make general functions
> to deal
> with them gracefully.
>
> > And if you team has some special opensource test suites used
> in your daily
> > f2fs test cycle, pls. kindly share the info with us, then we
> can make sure our
> > patchset can pass these cases before we send out next
> version.
>
>
> 1. xfstests for functionality
> 2. fsstress for deadlock/consistency check
> 3. power-off with fsstress
>
> Thanks,
>
> > BTW, test the kernel source tree or kernel build is a good
> suggestion. thanks.
> >
> > On Tue, Jun 04, 2013 at 01:23:57PM +0900, Namjae Jeon wrote:
> > > Hi. Huajun.
> > >
> > > I agree jaegeuk's opinion.
> > > Additionally, It is better that you describe the effect in
> change-log
> > > when this feature is added to f2fs.
> > > e.g.
> > > 1. how much space is saved when storing
> kernel-tree(small files) ?
> > > 2. small files creation performance test.
> > > 3. file look-up performance test.
> > > 4. other performance tools 's result.
> > >
> > > Thanks.
> > >
> > > 2013/6/4 Jaegeuk Kim <jaegeuk.kim@...sung.com>:
> > > > Hi,
> > > >
> > > > This feature is one of my todo items. ;)
> > > > Thank you for the contribution.
> > > >
> > > > Before reviewing the below code intensively, we need to
> check the
> > > > following issues.
> > > >
> > > > - deadlock conditions
> > > > - FS consistency
> > > > - recovery routine
> > > >
> > > > Could you check one more time?
> > > > Thanks again,
> > > >
> > > > 2013-06-03 (월), 18:04 +0800, Huajun Li:
> > > >> f2fs inode is so large, small files can be stored
> directly in the inode,
> > > >> rather than just storing a single block address and
> storing the data elsewhere.
> > > >>
> > > >> This RFC patch set is just to enable f2fs support
> inline data: files less than
> > > >> about 3.6K can be stored directly in inode block.
> > > >>
> > > >> TODO: make small dirs inline too.
> > > >>
> > > >>
> > > >> Haicheng Li (3):
> > > >> f2fs: Add helper functions and flag to support inline
> data
> > > >> f2fs: Add interface for inline data support
> > > >> f2fs: add tracepoints to debug inline data operations
> > > >>
> > > >> Huajun Li (2):
> > > >> f2fs: Handle inline data read and write
> > > >> f2fs: Key functions to handle inline data
> > > >>
> > > >> fs/f2fs/Kconfig | 10 +++
> > > >> fs/f2fs/Makefile | 1 +
> > > >> fs/f2fs/data.c | 78
> +++++++++++++++++++++-
> > > >> fs/f2fs/f2fs.h | 70 +++++++++++++++++++
> > > >> fs/f2fs/file.c | 9 ++-
> > > >> fs/f2fs/inline.c | 156
> +++++++++++++++++++++++++++++++++++++++++++
> > > >> fs/f2fs/inode.c | 8 +++
> > > >> include/linux/f2fs_fs.h | 5 ++
> > > >> include/trace/events/f2fs.h | 69 +++++++++++++++++++
> > > >> 9 files changed, 402 insertions(+), 4 deletions(-)
> > > >> create mode 100644 fs/f2fs/inline.c
> > > >>
> > > >
> > > > --
> > > > Jaegeuk Kim
> > > > Samsung
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe
> linux-fsdevel" in
> > > the body of a message to majordomo@...r.kernel.org
> > > More majordomo info at
> http://vger.kernel.org/majordomo-info.html
>
>
> --
> Jaegeuk Kim
> Samsung
>
>
--
Jaegeuk Kim
Samsung
Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)
Powered by blists - more mailing lists