| Message ID | 003601d099fd$837394b0$8a5abe10$@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Chao, [snip] > > We can see that file still uses 16 Blocks > > > > How do we deal with such situation? > > Now, f2fs does not convert non-inline inode to inline one for these potential > inodes with small size. So, once inline inode is converted, there is no > procedure to revert it. > > > it's meaningful to reuse inline_data? > > Actually, I did not know the history reason why we don't implement the > reverse procedure, as inline_data feature is designed and implemented > by Huajun and Jaegeuk. I can't find any clues in old threads. Initial proposal implemented this sort of conversion. But, in the production level, I suffered from many bugs like eating data. The major attack to me was the lock coverage which was not fully designed concretely. The real complexity lied on allocation and truncation paths in parallel. And, it turned out the key data structures was not covered by a lock correctly. So, I changed them not only to use node page lock as much as possible, but also to implement one-way conversion to reduce lock contention, inline_data conversion overhead, and code complexity. In addition, the reason why I decided to do that is from the question how many large files are converted to small-sized (1B ~ 3.5KB) files. In terms of life of files, I believe it is enough to set inline_data for originally small files, which mitigates cleaning overhead as my expectation. For space utilization, I'm not quite sure since most of space are normally occupied by many large files for multimedia. Let me correct, if I'm missing something. Thanks, -- 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
Hi Jaegeuk, Sorry for the long delay! > -----Original Message----- > From: Jaegeuk Kim [mailto:jaegeuk@kernel.org] > Sent: Saturday, May 30, 2015 7:14 AM > To: Chao Yu > Cc: 'He YunLei'; linux-fsdevel@vger.kernel.org; linux-f2fs-devel@lists.sourceforge.net > Subject: Re: [f2fs-dev] Not use inline_data after file shrink > > Hi Chao, > > [snip] > > > > We can see that file still uses 16 Blocks > > > > > > How do we deal with such situation? > > > > Now, f2fs does not convert non-inline inode to inline one for these potential > > inodes with small size. So, once inline inode is converted, there is no > > procedure to revert it. > > > > > it's meaningful to reuse inline_data? > > > > Actually, I did not know the history reason why we don't implement the > > reverse procedure, as inline_data feature is designed and implemented > > by Huajun and Jaegeuk. I can't find any clues in old threads. > > Initial proposal implemented this sort of conversion. > But, in the production level, I suffered from many bugs like eating data. > The major attack to me was the lock coverage which was not fully designed > concretely. > > The real complexity lied on allocation and truncation paths in parallel. > And, it turned out the key data structures was not covered by a lock correctly. > So, I changed them not only to use node page lock as much as possible, but > also to implement one-way conversion to reduce lock contention, inline_data > conversion overhead, and code complexity. Thank you for letting me know the history! :) > > In addition, the reason why I decided to do that is from the question how many > large files are converted to small-sized (1B ~ 3.5KB) files. I think the scenario is existing as below: a. sqlite may use truncate mode for its log file. b. file size can be shrunk automatically if we have enable auto-vacuum feature in db system. But unfortunately it's not very common, we can hardly benefit a lot from the scenario, and base on intensive lock contention due to two-way conversion as you mentioned, I can't say we should implement it. So for now I don't mind keeping original inline_data implementation. Thanks, > > In terms of life of files, I believe it is enough to set inline_data for > originally small files, which mitigates cleaning overhead as my expectation. > > For space utilization, I'm not quite sure since most of space are normally > occupied by many large files for multimedia. > > Let me correct, if I'm missing something. > > Thanks, -- 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
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index ab30a31..e1b2ce0 100644 --- a/fs/f2fs/data.c +++ b/fs/f2fs/data.c @@ -1765,6 +1765,8 @@ write: if (f2fs_has_inline_data(inode)) err = f2fs_write_inline_data(inode, page); if (err == -EAGAIN) + err = try_convert_to_inline_inode(inode, page); + if (err == -EAGAIN) err = do_write_data_page(&fio); f2fs_unlock_op(sbi); done: diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 3587576..011bc1f 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -1936,6 +1936,7 @@ bool truncate_inline_inode(struct page *, u64); int f2fs_read_inline_data(struct inode *, struct page *); int f2fs_convert_inline_page(struct dnode_of_data *, struct page *); int f2fs_convert_inline_inode(struct inode *); +int try_convert_to_inline_inode(struct inode *, struct page *); int f2fs_write_inline_data(struct inode *, struct page *); bool recover_inline_data(struct inode *, struct page *); struct f2fs_dir_entry *find_in_inline_dir(struct inode *, diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c index 38e75fb..87620e3 100644 --- a/fs/f2fs/inline.c +++ b/fs/f2fs/inline.c @@ -199,6 +199,40 @@ out: return err; } +int try_convert_to_inline_inode(struct inode *inode, struct page *page) +{ + void *src_addr, *dst_addr; + struct dnode_of_data dn; + int err; + + if (!f2fs_may_inline_data(inode)) + return -EAGAIN; + + set_new_dnode(&dn, inode, NULL, NULL, 0); + err = get_dnode_of_data(&dn, 0, LOOKUP_NODE); + if (err) + return -EAGAIN; + + f2fs_bug_on(F2FS_I_SB(inode), f2fs_has_inline_data(inode)); + + f2fs_wait_on_page_writeback(dn.inode_page, NODE); + + src_addr = kmap_atomic(page); + dst_addr = inline_data_addr(dn.inode_page); + memcpy(dst_addr, src_addr, MAX_INLINE_DATA); + kunmap_atomic(src_addr); + + truncate_data_blocks_range(&dn, 1); + + set_inode_flag(F2FS_I(inode), FI_INLINE_DATA); + set_inode_flag(F2FS_I(inode), FI_APPEND_WRITE); + set_inode_flag(F2FS_I(inode), FI_DATA_EXIST); + + sync_inode_page(&dn); + f2fs_put_dnode(&dn); + return 0; +} + int f2fs_write_inline_data(struct inode *inode, struct page *page) { void *src_addr, *dst_addr;