Hi Jaegeuk,
Thanks for your suggestion.

On Wed, Jun 5, 2013 at 3:13 PM, Jaegeuk Kim <jaegeuk....@samsung.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 ?
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 ?

 `- 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?


>  `- 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.


> 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....@samsung.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 majord...@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> Jaegeuk Kim
> Samsung
>
------------------------------------------------------------------------------
How ServiceNow helps IT people transform IT departments:
1. A cloud service to automate IT design, transition and operations
2. Dashboards that offer high-level views of enterprise services
3. A single system of record for all IT processes
http://p.sf.net/sfu/servicenow-d2d-j
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to