* [PATCH] f2fs: modify the readahead method in ra_node_page() @ 2016-02-29 6:29 Fan Li 2016-03-01 9:53 ` Chao Yu 0 siblings, 1 reply; 6+ messages in thread From: Fan Li @ 2016-02-29 6:29 UTC (permalink / raw) To: 'Jaegeuk Kim'; +Cc: linux-f2fs-devel ra_node_page() is used to read ahead one node page. Comparing to regular read, it's faster because it doesn't wait for IO completion. But if it is called twice for reading the same block, and the IO request from the first call hasn't been completed before the second call, the second call will have to wait until the read is over. Here use the code in __do_page_cache_readahead() to solve this problem. It does nothing when someone else already puts the page in mapping. The status of page should be assured by whoever puts it there. This implement also prevents alteration of page reference count. Signed-off-by: Fan li <fanofcode.li@samsung.com> --- fs/f2fs/node.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index 511c0e7..6d8f107 100644 --- a/fs/f2fs/node.c +++ b/fs/f2fs/node.c @@ -1080,12 +1080,11 @@ void ra_node_page(struct f2fs_sb_info *sbi, nid_t nid) return; f2fs_bug_on(sbi, check_nid_range(sbi, nid)); - apage = find_get_page(NODE_MAPPING(sbi), nid); - if (apage && PageUptodate(apage)) { - f2fs_put_page(apage, 0); + rcu_read_lock(); + apage = radix_tree_lookup(&NODE_MAPPING(sbi)->page_tree, nid); + rcu_read_unlock(); + if (apage) return; - } - f2fs_put_page(apage, 0); apage = grab_cache_page(NODE_MAPPING(sbi), nid); if (!apage) -- 1.7.9.5 ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] f2fs: modify the readahead method in ra_node_page() 2016-02-29 6:29 [PATCH] f2fs: modify the readahead method in ra_node_page() Fan Li @ 2016-03-01 9:53 ` Chao Yu 2016-03-02 3:11 ` Fan Li 0 siblings, 1 reply; 6+ messages in thread From: Chao Yu @ 2016-03-01 9:53 UTC (permalink / raw) To: 'Fan Li', 'Jaegeuk Kim'; +Cc: linux-f2fs-devel Hi > -----Original Message----- > From: Fan Li [mailto:fanofcode.li@samsung.com] > Sent: Monday, February 29, 2016 2:30 PM > To: 'Jaegeuk Kim' > Cc: linux-f2fs-devel@lists.sourceforge.net > Subject: [f2fs-dev] [PATCH] f2fs: modify the readahead method in ra_node_page() > > ra_node_page() is used to read ahead one node page. Comparing to regular > read, it's faster because it doesn't wait for IO completion. > But if it is called twice for reading the same block, and the IO request > from the first call hasn't been completed before the second call, the second > call will have to wait until the read is over. > > Here use the code in __do_page_cache_readahead() to solve this problem. > It does nothing when someone else already puts the page in mapping. The > status of page should be assured by whoever puts it there. > This implement also prevents alteration of page reference count. > > Signed-off-by: Fan li <fanofcode.li@samsung.com> > --- > fs/f2fs/node.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c > index 511c0e7..6d8f107 100644 > --- a/fs/f2fs/node.c > +++ b/fs/f2fs/node.c > @@ -1080,12 +1080,11 @@ void ra_node_page(struct f2fs_sb_info *sbi, nid_t nid) > return; > f2fs_bug_on(sbi, check_nid_range(sbi, nid)); > > - apage = find_get_page(NODE_MAPPING(sbi), nid); > - if (apage && PageUptodate(apage)) { > - f2fs_put_page(apage, 0); > + rcu_read_lock(); > + apage = radix_tree_lookup(&NODE_MAPPING(sbi)->page_tree, nid); > + rcu_read_unlock(); > + if (apage) > return; > - } > - f2fs_put_page(apage, 0); How about use trylock_page to avoid contention here? diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index 26eb441..9cdb6f2 100644 --- a/fs/f2fs/node.c +++ b/fs/f2fs/node.c @@ -1085,15 +1085,14 @@ void ra_node_page(struct f2fs_sb_info *sbi, nid_t nid) f2fs_bug_on(sbi, check_nid_range(sbi, nid)); apage = find_get_page(NODE_MAPPING(sbi), nid); - if (apage && PageUptodate(apage)) { + if (!apage) { + apage = grab_cache_page(NODE_MAPPING(sbi), nid); + if (!apage) + return; + } else if (PageUptodate(apage) || !trylock_page(apage)) { f2fs_put_page(apage, 0); return; } - f2fs_put_page(apage, 0); - - apage = grab_cache_page(NODE_MAPPING(sbi), nid); - if (!apage) - return; err = read_node_page(apage, READA); f2fs_put_page(apage, err ? 1 : 0); Thanks, > > apage = grab_cache_page(NODE_MAPPING(sbi), nid); > if (!apage) > -- > 1.7.9.5 > > > ------------------------------------------------------------------------------ > Site24x7 APM Insight: Get Deep Visibility into Application Performance > APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month > Monitor end-to-end web transactions and take corrective actions now > Troubleshoot faster and improve end-user experience. Signup Now! > http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140 > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] f2fs: modify the readahead method in ra_node_page() 2016-03-01 9:53 ` Chao Yu @ 2016-03-02 3:11 ` Fan Li 2016-03-03 13:32 ` Chao Yu 0 siblings, 1 reply; 6+ messages in thread From: Fan Li @ 2016-03-02 3:11 UTC (permalink / raw) To: 'Chao Yu', 'Jaegeuk Kim'; +Cc: linux-f2fs-devel > -----Original Message----- > From: Chao Yu [mailto:chao2.yu@samsung.com] > Sent: Tuesday, March 01, 2016 5:53 PM > To: 'Fan Li'; 'Jaegeuk Kim' > Cc: linux-f2fs-devel@lists.sourceforge.net > Subject: RE: [f2fs-dev] [PATCH] f2fs: modify the readahead method in ra_node_page() > > Hi > > > -----Original Message----- > > From: Fan Li [mailto:fanofcode.li@samsung.com] > > Sent: Monday, February 29, 2016 2:30 PM > > To: 'Jaegeuk Kim' > > Cc: linux-f2fs-devel@lists.sourceforge.net > > Subject: [f2fs-dev] [PATCH] f2fs: modify the readahead method in > > ra_node_page() > > > > ra_node_page() is used to read ahead one node page. Comparing to > > regular read, it's faster because it doesn't wait for IO completion. > > But if it is called twice for reading the same block, and the IO > > request from the first call hasn't been completed before the second > > call, the second call will have to wait until the read is over. > > > > Here use the code in __do_page_cache_readahead() to solve this problem. > > It does nothing when someone else already puts the page in mapping. > > The status of page should be assured by whoever puts it there. > > This implement also prevents alteration of page reference count. > > > > Signed-off-by: Fan li <fanofcode.li@samsung.com> > > --- > > fs/f2fs/node.c | 9 ++++----- > > 1 file changed, 4 insertions(+), 5 deletions(-) > > > > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index 511c0e7..6d8f107 > > 100644 > > --- a/fs/f2fs/node.c > > +++ b/fs/f2fs/node.c > > @@ -1080,12 +1080,11 @@ void ra_node_page(struct f2fs_sb_info *sbi, nid_t nid) > > return; > > f2fs_bug_on(sbi, check_nid_range(sbi, nid)); > > > > - apage = find_get_page(NODE_MAPPING(sbi), nid); > > - if (apage && PageUptodate(apage)) { > > - f2fs_put_page(apage, 0); > > + rcu_read_lock(); > > + apage = radix_tree_lookup(&NODE_MAPPING(sbi)->page_tree, nid); > > + rcu_read_unlock(); > > + if (apage) > > return; > > - } > > - f2fs_put_page(apage, 0); > > How about use trylock_page to avoid contention here? I thought about that, but after I saw the __do_page_cache_readahead(), I think that's better. they works almost the same, but it reduces the performance by altering reference count and trying to lock the page. The only difference between this two methods is how to deal a page which is left unlock and not uptodate in mapping. I think only cause of such page seems to be IO error, and it's too rare to call for optimization. > > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index 26eb441..9cdb6f2 100644 > --- a/fs/f2fs/node.c > +++ b/fs/f2fs/node.c > @@ -1085,15 +1085,14 @@ void ra_node_page(struct f2fs_sb_info *sbi, nid_t nid) > f2fs_bug_on(sbi, check_nid_range(sbi, nid)); > > apage = find_get_page(NODE_MAPPING(sbi), nid); > - if (apage && PageUptodate(apage)) { > + if (!apage) { > + apage = grab_cache_page(NODE_MAPPING(sbi), nid); > + if (!apage) > + return; > + } else if (PageUptodate(apage) || !trylock_page(apage)) { > f2fs_put_page(apage, 0); > return; > } > - f2fs_put_page(apage, 0); > - > - apage = grab_cache_page(NODE_MAPPING(sbi), nid); > - if (!apage) > - return; > > err = read_node_page(apage, READA); > f2fs_put_page(apage, err ? 1 : 0); > > Thanks, > > > > > apage = grab_cache_page(NODE_MAPPING(sbi), nid); > > if (!apage) > > -- > > 1.7.9.5 > > > > > > ---------------------------------------------------------------------- > > -------- > > Site24x7 APM Insight: Get Deep Visibility into Application Performance > > APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month > > Monitor end-to-end web transactions and take corrective actions now > > Troubleshoot faster and improve end-user experience. Signup Now! > > http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140 > > _______________________________________________ > > Linux-f2fs-devel mailing list > > Linux-f2fs-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] f2fs: modify the readahead method in ra_node_page() 2016-03-02 3:11 ` Fan Li @ 2016-03-03 13:32 ` Chao Yu 2016-03-07 3:48 ` Fan Li 0 siblings, 1 reply; 6+ messages in thread From: Chao Yu @ 2016-03-03 13:32 UTC (permalink / raw) To: Fan Li, 'Chao Yu', 'Jaegeuk Kim'; +Cc: linux-f2fs-devel On 2016/3/2 11:11, Fan Li wrote: > > >> -----Original Message----- >> From: Chao Yu [mailto:chao2.yu@samsung.com] >> Sent: Tuesday, March 01, 2016 5:53 PM >> To: 'Fan Li'; 'Jaegeuk Kim' >> Cc: linux-f2fs-devel@lists.sourceforge.net >> Subject: RE: [f2fs-dev] [PATCH] f2fs: modify the readahead method in ra_node_page() >> >> Hi >> >>> -----Original Message----- >>> From: Fan Li [mailto:fanofcode.li@samsung.com] >>> Sent: Monday, February 29, 2016 2:30 PM >>> To: 'Jaegeuk Kim' >>> Cc: linux-f2fs-devel@lists.sourceforge.net >>> Subject: [f2fs-dev] [PATCH] f2fs: modify the readahead method in >>> ra_node_page() >>> >>> ra_node_page() is used to read ahead one node page. Comparing to >>> regular read, it's faster because it doesn't wait for IO completion. >>> But if it is called twice for reading the same block, and the IO >>> request from the first call hasn't been completed before the second >>> call, the second call will have to wait until the read is over. >>> >>> Here use the code in __do_page_cache_readahead() to solve this problem. >>> It does nothing when someone else already puts the page in mapping. >>> The status of page should be assured by whoever puts it there. >>> This implement also prevents alteration of page reference count. >>> >>> Signed-off-by: Fan li <fanofcode.li@samsung.com> >>> --- >>> fs/f2fs/node.c | 9 ++++----- >>> 1 file changed, 4 insertions(+), 5 deletions(-) >>> >>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index 511c0e7..6d8f107 >>> 100644 >>> --- a/fs/f2fs/node.c >>> +++ b/fs/f2fs/node.c >>> @@ -1080,12 +1080,11 @@ void ra_node_page(struct f2fs_sb_info *sbi, nid_t nid) >>> return; >>> f2fs_bug_on(sbi, check_nid_range(sbi, nid)); >>> >>> - apage = find_get_page(NODE_MAPPING(sbi), nid); >>> - if (apage && PageUptodate(apage)) { >>> - f2fs_put_page(apage, 0); >>> + rcu_read_lock(); >>> + apage = radix_tree_lookup(&NODE_MAPPING(sbi)->page_tree, nid); >>> + rcu_read_unlock(); >>> + if (apage) >>> return; >>> - } >>> - f2fs_put_page(apage, 0); >> >> How about use trylock_page to avoid contention here? > > I thought about that, but after I saw the __do_page_cache_readahead(), > I think that's better. > they works almost the same, but it reduces the performance by > altering reference count and trying to lock the page. > > The only difference between this two methods is how to deal a page Concurrent threads can both pass the rcu lookuping at the same time, and then they will be serialized by grab_cache_page invoking, which altering second thread to read node page synchronously instead of reading asynchronously. IMO, we can afford to reference and trylock which takes litte overhead of CPU in order to avoid potential synchronously readahead for node page. Thanks, > which is left unlock and not uptodate in mapping. I think only cause > of such page seems to be IO error, and it's too rare to call for optimization. > >> >> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index 26eb441..9cdb6f2 100644 >> --- a/fs/f2fs/node.c >> +++ b/fs/f2fs/node.c >> @@ -1085,15 +1085,14 @@ void ra_node_page(struct f2fs_sb_info *sbi, nid_t nid) >> f2fs_bug_on(sbi, check_nid_range(sbi, nid)); >> >> apage = find_get_page(NODE_MAPPING(sbi), nid); >> - if (apage && PageUptodate(apage)) { >> + if (!apage) { >> + apage = grab_cache_page(NODE_MAPPING(sbi), nid); >> + if (!apage) >> + return; >> + } else if (PageUptodate(apage) || !trylock_page(apage)) { >> f2fs_put_page(apage, 0); >> return; >> } >> - f2fs_put_page(apage, 0); >> - >> - apage = grab_cache_page(NODE_MAPPING(sbi), nid); >> - if (!apage) >> - return; >> >> err = read_node_page(apage, READA); >> f2fs_put_page(apage, err ? 1 : 0); >> >> Thanks, >> >>> >>> apage = grab_cache_page(NODE_MAPPING(sbi), nid); >>> if (!apage) >>> -- >>> 1.7.9.5 >>> >>> >>> ---------------------------------------------------------------------- >>> -------- >>> Site24x7 APM Insight: Get Deep Visibility into Application Performance >>> APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month >>> Monitor end-to-end web transactions and take corrective actions now >>> Troubleshoot faster and improve end-user experience. Signup Now! >>> http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140 >>> _______________________________________________ >>> Linux-f2fs-devel mailing list >>> Linux-f2fs-devel@lists.sourceforge.net >>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > > > ------------------------------------------------------------------------------ > Site24x7 APM Insight: Get Deep Visibility into Application Performance > APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month > Monitor end-to-end web transactions and take corrective actions now > Troubleshoot faster and improve end-user experience. Signup Now! > http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140 > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] f2fs: modify the readahead method in ra_node_page() 2016-03-03 13:32 ` Chao Yu @ 2016-03-07 3:48 ` Fan Li 2016-03-08 2:29 ` Jaegeuk Kim 0 siblings, 1 reply; 6+ messages in thread From: Fan Li @ 2016-03-07 3:48 UTC (permalink / raw) To: 'Chao Yu', 'Chao Yu', 'Jaegeuk Kim' Cc: linux-f2fs-devel > -----Original Message----- > From: Chao Yu [mailto:chao@kernel.org] > Sent: Thursday, March 03, 2016 9:32 PM > To: Fan Li; 'Chao Yu'; 'Jaegeuk Kim' > Cc: linux-f2fs-devel@lists.sourceforge.net > Subject: Re: [f2fs-dev] [PATCH] f2fs: modify the readahead method in ra_node_page() > > On 2016/3/2 11:11, Fan Li wrote: > > > > > >> -----Original Message----- > >> From: Chao Yu [mailto:chao2.yu@samsung.com] > >> Sent: Tuesday, March 01, 2016 5:53 PM > >> To: 'Fan Li'; 'Jaegeuk Kim' > >> Cc: linux-f2fs-devel@lists.sourceforge.net > >> Subject: RE: [f2fs-dev] [PATCH] f2fs: modify the readahead method in > >> ra_node_page() > >> > >> Hi > >> > >>> -----Original Message----- > >>> From: Fan Li [mailto:fanofcode.li@samsung.com] > >>> Sent: Monday, February 29, 2016 2:30 PM > >>> To: 'Jaegeuk Kim' > >>> Cc: linux-f2fs-devel@lists.sourceforge.net > >>> Subject: [f2fs-dev] [PATCH] f2fs: modify the readahead method in > >>> ra_node_page() > >>> > >>> ra_node_page() is used to read ahead one node page. Comparing to > >>> regular read, it's faster because it doesn't wait for IO completion. > >>> But if it is called twice for reading the same block, and the IO > >>> request from the first call hasn't been completed before the second > >>> call, the second call will have to wait until the read is over. > >>> > >>> Here use the code in __do_page_cache_readahead() to solve this problem. > >>> It does nothing when someone else already puts the page in mapping. > >>> The status of page should be assured by whoever puts it there. > >>> This implement also prevents alteration of page reference count. > >>> > >>> Signed-off-by: Fan li <fanofcode.li@samsung.com> > >>> --- > >>> fs/f2fs/node.c | 9 ++++----- > >>> 1 file changed, 4 insertions(+), 5 deletions(-) > >>> > >>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index 511c0e7..6d8f107 > >>> 100644 > >>> --- a/fs/f2fs/node.c > >>> +++ b/fs/f2fs/node.c > >>> @@ -1080,12 +1080,11 @@ void ra_node_page(struct f2fs_sb_info *sbi, nid_t nid) > >>> return; > >>> f2fs_bug_on(sbi, check_nid_range(sbi, nid)); > >>> > >>> - apage = find_get_page(NODE_MAPPING(sbi), nid); > >>> - if (apage && PageUptodate(apage)) { > >>> - f2fs_put_page(apage, 0); > >>> + rcu_read_lock(); > >>> + apage = radix_tree_lookup(&NODE_MAPPING(sbi)->page_tree, nid); > >>> + rcu_read_unlock(); > >>> + if (apage) > >>> return; > >>> - } > >>> - f2fs_put_page(apage, 0); > >> > >> How about use trylock_page to avoid contention here? > > > > I thought about that, but after I saw the __do_page_cache_readahead(), > > I think that's better. > > they works almost the same, but it reduces the performance by altering > > reference count and trying to lock the page. > > > > The only difference between this two methods is how to deal a page > > Concurrent threads can both pass the rcu lookuping at the same time, and then they will be serialized by grab_cache_page invoking, > which altering second thread to read node page synchronously instead of reading asynchronously. IMO, we can afford to reference > and trylock which takes litte overhead of CPU in order to avoid potential synchronously readahead for node page. My point is that this patch can avoid potential synchronously readahead for node page without taking such overhead. The difference is very small and the method has already been used in vfs codes. > > Thanks, > > > which is left unlock and not uptodate in mapping. I think only cause > > of such page seems to be IO error, and it's too rare to call for optimization. > > > >> > >> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index 26eb441..9cdb6f2 > >> 100644 > >> --- a/fs/f2fs/node.c > >> +++ b/fs/f2fs/node.c > >> @@ -1085,15 +1085,14 @@ void ra_node_page(struct f2fs_sb_info *sbi, nid_t nid) > >> f2fs_bug_on(sbi, check_nid_range(sbi, nid)); > >> > >> apage = find_get_page(NODE_MAPPING(sbi), nid); > >> - if (apage && PageUptodate(apage)) { > >> + if (!apage) { > >> + apage = grab_cache_page(NODE_MAPPING(sbi), nid); > >> + if (!apage) > >> + return; > >> + } else if (PageUptodate(apage) || !trylock_page(apage)) { > >> f2fs_put_page(apage, 0); > >> return; > >> } > >> - f2fs_put_page(apage, 0); > >> - > >> - apage = grab_cache_page(NODE_MAPPING(sbi), nid); > >> - if (!apage) > >> - return; > >> > >> err = read_node_page(apage, READA); > >> f2fs_put_page(apage, err ? 1 : 0); > >> > >> Thanks, > >> > >>> > >>> apage = grab_cache_page(NODE_MAPPING(sbi), nid); > >>> if (!apage) > >>> -- > >>> 1.7.9.5 > >>> > >>> > >>> -------------------------------------------------------------------- > >>> -- > >>> -------- > >>> Site24x7 APM Insight: Get Deep Visibility into Application > >>> Performance APM + Mobile APM + RUM: Monitor 3 App instances at just > >>> $35/Month Monitor end-to-end web transactions and take corrective > >>> actions now Troubleshoot faster and improve end-user experience. Signup Now! > >>> http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140 > >>> _______________________________________________ > >>> Linux-f2fs-devel mailing list > >>> Linux-f2fs-devel@lists.sourceforge.net > >>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > > > > > > ---------------------------------------------------------------------- > > -------- > > Site24x7 APM Insight: Get Deep Visibility into Application Performance > > APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month > > Monitor end-to-end web transactions and take corrective actions now > > Troubleshoot faster and improve end-user experience. Signup Now! > > http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140 > > _______________________________________________ > > Linux-f2fs-devel mailing list > > Linux-f2fs-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > > ------------------------------------------------------------------------------ Transform Data into Opportunity. Accelerate data analysis in your applications with Intel Data Analytics Acceleration Library. Click to learn more. http://makebettercode.com/inteldaal-eval ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] f2fs: modify the readahead method in ra_node_page() 2016-03-07 3:48 ` Fan Li @ 2016-03-08 2:29 ` Jaegeuk Kim 0 siblings, 0 replies; 6+ messages in thread From: Jaegeuk Kim @ 2016-03-08 2:29 UTC (permalink / raw) To: Fan Li; +Cc: 'Chao Yu', linux-f2fs-devel > > >>> fs/f2fs/node.c | 9 ++++----- > > >>> 1 file changed, 4 insertions(+), 5 deletions(-) > > >>> > > >>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index 511c0e7..6d8f107 > > >>> 100644 > > >>> --- a/fs/f2fs/node.c > > >>> +++ b/fs/f2fs/node.c > > >>> @@ -1080,12 +1080,11 @@ void ra_node_page(struct f2fs_sb_info *sbi, nid_t nid) > > >>> return; > > >>> f2fs_bug_on(sbi, check_nid_range(sbi, nid)); > > >>> > > >>> - apage = find_get_page(NODE_MAPPING(sbi), nid); > > >>> - if (apage && PageUptodate(apage)) { > > >>> - f2fs_put_page(apage, 0); > > >>> + rcu_read_lock(); > > >>> + apage = radix_tree_lookup(&NODE_MAPPING(sbi)->page_tree, nid); > > >>> + rcu_read_unlock(); > > >>> + if (apage) > > >>> return; > > >>> - } > > >>> - f2fs_put_page(apage, 0); > > >> > > >> How about use trylock_page to avoid contention here? > > > > > > I thought about that, but after I saw the __do_page_cache_readahead(), > > > I think that's better. > > > they works almost the same, but it reduces the performance by altering > > > reference count and trying to lock the page. > > > > > > The only difference between this two methods is how to deal a page > > > > Concurrent threads can both pass the rcu lookuping at the same time, and then they will be serialized by grab_cache_page invoking, > > which altering second thread to read node page synchronously instead of reading asynchronously. IMO, we can afford to reference > > and trylock which takes litte overhead of CPU in order to avoid potential synchronously readahead for node page. > > My point is that this patch can avoid potential synchronously readahead for node page without taking such overhead. > The difference is very small and the method has already been used in vfs codes. Let me go with Fan's patch at this moment. Then, it'd good to investigate how contention occurs here later. Thanks, > > > > > Thanks, > > > > > which is left unlock and not uptodate in mapping. I think only cause > > > of such page seems to be IO error, and it's too rare to call for optimization. > > > > > >> > > >> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c index 26eb441..9cdb6f2 > > >> 100644 > > >> --- a/fs/f2fs/node.c > > >> +++ b/fs/f2fs/node.c > > >> @@ -1085,15 +1085,14 @@ void ra_node_page(struct f2fs_sb_info *sbi, nid_t nid) > > >> f2fs_bug_on(sbi, check_nid_range(sbi, nid)); > > >> > > >> apage = find_get_page(NODE_MAPPING(sbi), nid); > > >> - if (apage && PageUptodate(apage)) { > > >> + if (!apage) { > > >> + apage = grab_cache_page(NODE_MAPPING(sbi), nid); > > >> + if (!apage) > > >> + return; > > >> + } else if (PageUptodate(apage) || !trylock_page(apage)) { > > >> f2fs_put_page(apage, 0); > > >> return; > > >> } > > >> - f2fs_put_page(apage, 0); > > >> - > > >> - apage = grab_cache_page(NODE_MAPPING(sbi), nid); > > >> - if (!apage) > > >> - return; > > >> > > >> err = read_node_page(apage, READA); > > >> f2fs_put_page(apage, err ? 1 : 0); > > >> > > >> Thanks, > > >> > > >>> > > >>> apage = grab_cache_page(NODE_MAPPING(sbi), nid); > > >>> if (!apage) > > >>> -- > > >>> 1.7.9.5 > > >>> > > >>> > > >>> -------------------------------------------------------------------- > > >>> -- > > >>> -------- > > >>> Site24x7 APM Insight: Get Deep Visibility into Application > > >>> Performance APM + Mobile APM + RUM: Monitor 3 App instances at just > > >>> $35/Month Monitor end-to-end web transactions and take corrective > > >>> actions now Troubleshoot faster and improve end-user experience. Signup Now! > > >>> http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140 > > >>> _______________________________________________ > > >>> Linux-f2fs-devel mailing list > > >>> Linux-f2fs-devel@lists.sourceforge.net > > >>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > > > > > > > > > ---------------------------------------------------------------------- > > > -------- > > > Site24x7 APM Insight: Get Deep Visibility into Application Performance > > > APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month > > > Monitor end-to-end web transactions and take corrective actions now > > > Troubleshoot faster and improve end-user experience. Signup Now! > > > http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140 > > > _______________________________________________ > > > Linux-f2fs-devel mailing list > > > Linux-f2fs-devel@lists.sourceforge.net > > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > > > ------------------------------------------------------------------------------ Transform Data into Opportunity. Accelerate data analysis in your applications with Intel Data Analytics Acceleration Library. Click to learn more. http://makebettercode.com/inteldaal-eval ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-03-08 2:29 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-02-29 6:29 [PATCH] f2fs: modify the readahead method in ra_node_page() Fan Li 2016-03-01 9:53 ` Chao Yu 2016-03-02 3:11 ` Fan Li 2016-03-03 13:32 ` Chao Yu 2016-03-07 3:48 ` Fan Li 2016-03-08 2:29 ` Jaegeuk Kim
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).