All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chao Yu <chao2.yu@samsung.com>
To: jaegeuk.kim@samsung.com
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [PATCH 3/3 V3] f2fs: introduce f2fs_cache_node_page() to add page into node_inode cache
Date: Mon, 09 Dec 2013 16:43:56 +0800	[thread overview]
Message-ID: <000b01cef4ba$eea9e890$cbfdb9b0$@samsung.com> (raw)
In-Reply-To: <1386576057.2101.106.camel@kjgkr>

Hi,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk.kim@samsung.com]
> Sent: Monday, December 09, 2013 4:01 PM
> To: Chao Yu
> Cc: linux-fsdevel@vger.kernel.org; linux-kernel@vger.kernel.org; linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH 3/3 V3] f2fs: introduce f2fs_cache_node_page() to add page into node_inode cache
> 
> Hi,
> 
> Sorry for the noise.
> Once I tested this patch, I recognized that I made a mistake:
> 
> f2fs_fill_super()
>  1. build_segment_manager()
>     -> restore_node_summary()
>        -> NULL pointer exception, due to no NAT cache.
>  2. build_node_manager()
> 
> So, we should not call get_node_info() prior to build_node_manager().
> And, it's not easy to change the order of build_*_manager simply.

Oh, sound bad. And sorry for my carelessness.
Let's reconsider this patch in details.

> 
> Let's consider in more details.
> Thanks,
> 
> 2013-12-09 (월), 13:35 +0800, Chao Yu:
> > This patch introduces f2fs_cache_node_page(), in this function, page which is
> > readed ahead will be copy to node_inode's mapping cache.
> > It will avoid rereading these node pages.
> >
> > change log:
> >  o check validity of page by searching NAT suggested by Jaegeuk Kim.
> >  o add 'unlikely' for compiler optimization suggested by Jaegeuk Kim.
> >
> > Suggested-by: Jaegeuk Kim <jaegeuk.kim@samsung.com>
> > Signed-off-by: Chao Yu <chao2.yu@samsung.com>
> > ---
> >  fs/f2fs/node.c |   39 ++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 38 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> > index 099f06f..4c6da98 100644
> > --- a/fs/f2fs/node.c
> > +++ b/fs/f2fs/node.c
> > @@ -1600,13 +1600,46 @@ static int ra_sum_pages(struct f2fs_sb_info *sbi, struct list_head *pages,
> >  	return 0;
> >  }
> >
> > +/*
> > + * f2fs_cache_node_page() check validaty of input page by searching NAT.
> > + * Then, it will copy updated data of vaild page to node_inode cache.
> > + */
> > +void f2fs_cache_node_page(struct f2fs_sb_info *sbi, struct page *page,
> > +						nid_t nid, block_t blkaddr)
> > +{
> > +	struct address_space *mapping = sbi->node_inode->i_mapping;
> > +	struct page *npage;
> > +	struct node_info ni;
> > +
> > +	get_node_info(sbi, nid, &ni);
> > +
> > +	if (ni.blk_addr != blkaddr)
> > +		return;
> > +
> > +	npage = grab_cache_page(mapping, nid);
> > +	if (unlikely(!npage))
> > +		return;
> > +
> > +	if (PageUptodate(npage)) {
> > +		f2fs_put_page(npage, 1);
> > +		return;
> > +	}
> > +
> > +	memcpy(page_address(npage), page_address(page), PAGE_CACHE_SIZE);
> > +
> > +	SetPageUptodate(npage);
> > +	f2fs_put_page(npage, 1);
> > +
> > +	return;
> > +}
> > +
> >  int restore_node_summary(struct f2fs_sb_info *sbi,
> >  			unsigned int segno, struct f2fs_summary_block *sum)
> >  {
> >  	struct f2fs_node *rn;
> >  	struct f2fs_summary *sum_entry;
> >  	struct page *page, *tmp;
> > -	block_t addr;
> > +	block_t addr, blkaddr;
> >  	int bio_blocks = MAX_BIO_BLOCKS(max_hw_blocks(sbi));
> >  	int i, last_offset, nrpages, err = 0;
> >  	LIST_HEAD(page_list);
> > @@ -1624,6 +1657,7 @@ int restore_node_summary(struct f2fs_sb_info *sbi,
> >  		if (err)
> >  			return err;
> >
> > +		blkaddr = addr;
> >  		list_for_each_entry_safe(page, tmp, &page_list, lru) {
> >
> >  			lock_page(page);
> > @@ -1633,6 +1667,8 @@ int restore_node_summary(struct f2fs_sb_info *sbi,
> >  				sum_entry->version = 0;
> >  				sum_entry->ofs_in_node = 0;
> >  				sum_entry++;
> > +				f2fs_cache_node_page(sbi, page,
> > +					le32_to_cpu(rn->footer.nid), blkaddr);
> >  			} else {
> >  				err = -EIO;
> >  			}
> > @@ -1640,6 +1676,7 @@ int restore_node_summary(struct f2fs_sb_info *sbi,
> >  			list_del(&page->lru);
> >  			unlock_page(page);
> >  			__free_pages(page, 0);
> > +			blkaddr++;
> >  		}
> >  	}
> >  	return err;
> 
> --
> Jaegeuk Kim
> Samsung


------------------------------------------------------------------------------
Sponsored by Intel(R) XDK 
Develop, test and display web and hybrid apps with a single code base.
Download it for free now!
http://pubads.g.doubleclick.net/gampad/clk?id=111408631&iu=/4140/ostg.clktrk
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

WARNING: multiple messages have this Message-ID (diff)
From: Chao Yu <chao2.yu@samsung.com>
To: jaegeuk.kim@samsung.com
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net
Subject: RE: [f2fs-dev] [PATCH 3/3 V3] f2fs: introduce f2fs_cache_node_page() to add page into node_inode cache
Date: Mon, 09 Dec 2013 16:43:56 +0800	[thread overview]
Message-ID: <000b01cef4ba$eea9e890$cbfdb9b0$@samsung.com> (raw)
In-Reply-To: <1386576057.2101.106.camel@kjgkr>

Hi,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk.kim@samsung.com]
> Sent: Monday, December 09, 2013 4:01 PM
> To: Chao Yu
> Cc: linux-fsdevel@vger.kernel.org; linux-kernel@vger.kernel.org; linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH 3/3 V3] f2fs: introduce f2fs_cache_node_page() to add page into node_inode cache
> 
> Hi,
> 
> Sorry for the noise.
> Once I tested this patch, I recognized that I made a mistake:
> 
> f2fs_fill_super()
>  1. build_segment_manager()
>     -> restore_node_summary()
>        -> NULL pointer exception, due to no NAT cache.
>  2. build_node_manager()
> 
> So, we should not call get_node_info() prior to build_node_manager().
> And, it's not easy to change the order of build_*_manager simply.

Oh, sound bad. And sorry for my carelessness.
Let's reconsider this patch in details.

> 
> Let's consider in more details.
> Thanks,
> 
> 2013-12-09 (월), 13:35 +0800, Chao Yu:
> > This patch introduces f2fs_cache_node_page(), in this function, page which is
> > readed ahead will be copy to node_inode's mapping cache.
> > It will avoid rereading these node pages.
> >
> > change log:
> >  o check validity of page by searching NAT suggested by Jaegeuk Kim.
> >  o add 'unlikely' for compiler optimization suggested by Jaegeuk Kim.
> >
> > Suggested-by: Jaegeuk Kim <jaegeuk.kim@samsung.com>
> > Signed-off-by: Chao Yu <chao2.yu@samsung.com>
> > ---
> >  fs/f2fs/node.c |   39 ++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 38 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> > index 099f06f..4c6da98 100644
> > --- a/fs/f2fs/node.c
> > +++ b/fs/f2fs/node.c
> > @@ -1600,13 +1600,46 @@ static int ra_sum_pages(struct f2fs_sb_info *sbi, struct list_head *pages,
> >  	return 0;
> >  }
> >
> > +/*
> > + * f2fs_cache_node_page() check validaty of input page by searching NAT.
> > + * Then, it will copy updated data of vaild page to node_inode cache.
> > + */
> > +void f2fs_cache_node_page(struct f2fs_sb_info *sbi, struct page *page,
> > +						nid_t nid, block_t blkaddr)
> > +{
> > +	struct address_space *mapping = sbi->node_inode->i_mapping;
> > +	struct page *npage;
> > +	struct node_info ni;
> > +
> > +	get_node_info(sbi, nid, &ni);
> > +
> > +	if (ni.blk_addr != blkaddr)
> > +		return;
> > +
> > +	npage = grab_cache_page(mapping, nid);
> > +	if (unlikely(!npage))
> > +		return;
> > +
> > +	if (PageUptodate(npage)) {
> > +		f2fs_put_page(npage, 1);
> > +		return;
> > +	}
> > +
> > +	memcpy(page_address(npage), page_address(page), PAGE_CACHE_SIZE);
> > +
> > +	SetPageUptodate(npage);
> > +	f2fs_put_page(npage, 1);
> > +
> > +	return;
> > +}
> > +
> >  int restore_node_summary(struct f2fs_sb_info *sbi,
> >  			unsigned int segno, struct f2fs_summary_block *sum)
> >  {
> >  	struct f2fs_node *rn;
> >  	struct f2fs_summary *sum_entry;
> >  	struct page *page, *tmp;
> > -	block_t addr;
> > +	block_t addr, blkaddr;
> >  	int bio_blocks = MAX_BIO_BLOCKS(max_hw_blocks(sbi));
> >  	int i, last_offset, nrpages, err = 0;
> >  	LIST_HEAD(page_list);
> > @@ -1624,6 +1657,7 @@ int restore_node_summary(struct f2fs_sb_info *sbi,
> >  		if (err)
> >  			return err;
> >
> > +		blkaddr = addr;
> >  		list_for_each_entry_safe(page, tmp, &page_list, lru) {
> >
> >  			lock_page(page);
> > @@ -1633,6 +1667,8 @@ int restore_node_summary(struct f2fs_sb_info *sbi,
> >  				sum_entry->version = 0;
> >  				sum_entry->ofs_in_node = 0;
> >  				sum_entry++;
> > +				f2fs_cache_node_page(sbi, page,
> > +					le32_to_cpu(rn->footer.nid), blkaddr);
> >  			} else {
> >  				err = -EIO;
> >  			}
> > @@ -1640,6 +1676,7 @@ int restore_node_summary(struct f2fs_sb_info *sbi,
> >  			list_del(&page->lru);
> >  			unlock_page(page);
> >  			__free_pages(page, 0);
> > +			blkaddr++;
> >  		}
> >  	}
> >  	return err;
> 
> --
> Jaegeuk Kim
> Samsung


  reply	other threads:[~2013-12-09  8:45 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-09  5:35 [f2fs-dev] [PATCH 3/3 V3] f2fs: introduce f2fs_cache_node_page() to add page into node_inode cache Chao Yu
2013-12-09  8:00 ` Jaegeuk Kim
2013-12-09  8:00   ` Jaegeuk Kim
2013-12-09  8:43   ` Chao Yu [this message]
2013-12-09  8:43     ` Chao Yu
2013-12-10  1:51   ` Chao Yu
2013-12-10  1:51     ` Chao Yu
2013-12-10  9:43     ` Chao Yu
2013-12-10  9:43       ` [f2fs-dev] " Chao Yu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='000b01cef4ba$eea9e890$cbfdb9b0$@samsung.com' \
    --to=chao2.yu@samsung.com \
    --cc=jaegeuk.kim@samsung.com \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.