On Tue, Dec 02, 2014 at 01:21:31PM +0900, Changman Lee wrote:
> Hi,
>
> f2fs_dirty_inode just set fi->flag as FI_DIRTY_INODE not to
> call update_inode_page. Instead, we do it when f2fs_write_indoe is called.
> Do you have any reason to do like this?
Actually, I'd like to use inode caches instead of dirty node pages as much as
possible to mitigate memory pressure as well as reduce node page writes.
But, the reality is that f2fs triggers update_inode_page frequently, since some
inode information like i_blocks and i_links should be recovered consistently
from sudden power-cuts.
> How about move update_inode_page from write_inode to dirty_inode?
> And we can update inode page when mark_inode_dirty or
> mark_inode_dirty_sync is called. Then, we control write I/O in
> write_inode according to wbc->sync_mode.
What do you mean controlling write I/O in write_inode?
The write_inode does not trigger any I/Os.
We're controlling node page writes by f2fs_write_node_pages.
Anyway, if we call update_inode_page in mark_inode_dirty, f2fs would suffer from
a lot of dirty node pages.
Thanks,
> Could you consider this once?
>
> Thanks,
>
> On Mon, Dec 01, 2014 at 02:52:57PM -0800, Jaegeuk Kim wrote:
> > On Mon, Dec 01, 2014 at 04:05:20PM +0900, Changman Lee wrote:
> > > It makes sense to check inode's state than check if
> > > inode page is dirty or not.
> >
> > Nice catch.
> > However, we should leave the original condition, since write_inode can be called
> > in prior to this fsync call.
> > And, this is not a proper fix, since it still can skip to write its inode page.
> >
> > How about this one?
> >
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index 146e58a..6690599 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -168,6 +168,12 @@ int f2fs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
> > return ret;
> > }
> >
> > + /* if the inode is dirty, let's recover all the time */
> > + if (is_inode_flag_set(fi, FI_DIRTY_INODE)) {
> > + update_inode_page(inode);
> > + goto go_write;
> > + }
> > +
> > /*
> > * if there is no written data, don't waste time to write recovery info.
> > */
> > --
> > 2.1.1
> >
> > >
> > > Signed-off-by: Changman Lee <cm224.lee@xxxxxxxxxxx>
> > > ---
> > > fs/f2fs/file.c | 7 ++-----
> > > 1 file changed, 2 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > > index 7c2ec3e..0c5ae87 100644
> > > --- a/fs/f2fs/file.c
> > > +++ b/fs/f2fs/file.c
> > > @@ -173,14 +173,11 @@ int f2fs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
> > > */
> > > if (!is_inode_flag_set(fi, FI_APPEND_WRITE) &&
> > > !exist_written_data(sbi, ino, APPEND_INO)) {
> > > - struct page *i = find_get_page(NODE_MAPPING(sbi), ino);
> > >
> > > /* But we need to avoid that there are some inode updates */
> > > - if ((i && PageDirty(i)) || need_inode_block_update(sbi, ino)) {
> > > - f2fs_put_page(i, 0);
> > > + if (is_inode_flag_set(fi, FI_DIRTY_INODE) ||
> > > + need_inode_block_update(sbi, ino))
> > > goto go_write;
> > > - }
> > > - f2fs_put_page(i, 0);
> > >
> > > if (is_inode_flag_set(fi, FI_UPDATE_WRITE) ||
> > > exist_written_data(sbi, ino, UPDATE_INO))
> > > --
> > > 1.9.1
> > >
> > >
> > > ------------------------------------------------------------------------------
> > > Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
> > > from Actuate! Instantly Supercharge Your Business Reports and Dashboards
> > > with Interactivity, Sharing, Native Excel Exports, App Integration & more
> > > Get technology previously reserved for billion-dollar corporations, FREE
> > > http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
> > > _______________________________________________
> > > Linux-f2fs-devel mailing list
> > > Linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxx
> > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
--
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
