Re: [f2fs-dev] [RFC 0/5] Enable f2fs support inline data

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

[-- Attachment #1.1: Type: text/plain, Size: 8219 bytes --]

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

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@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 majordomo@vger.kernel.org
>         > > More majordomo info at
>          http://vger.kernel.org/majordomo-info.html
>         
>         
>         --
>         Jaegeuk Kim
>         Samsung
> 
> 

-- 
Jaegeuk Kim
Samsung

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 355 bytes --]

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

[-- Attachment #3: Type: text/plain, Size: 179 bytes --]

_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel