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@xxxxxxxxxxx> > 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@xxxxxxxxxxx>: > > > > 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@xxxxxxxxxxxxxxx > > > More majordomo info at > http://vger.kernel.org/majordomo-info.html > > > -- > Jaegeuk Kim > Samsung > > -- Jaegeuk Kim Samsung
Attachment:
signature.asc
Description: This is a digitally signed message part
