* [PATCH] Btrfs: incremental send, apply asynchronous page cache readahead
@ 2017-09-13 6:38 peterh
2017-09-13 9:33 ` Pasi Kärkkäinen
2017-09-13 10:45 ` Filipe Manana
0 siblings, 2 replies; 5+ messages in thread
From: peterh @ 2017-09-13 6:38 UTC (permalink / raw)
To: linux-btrfs; +Cc: Kuanling Huang
From: Kuanling Huang <peterh@synology.com>
By analyzing the perf on btrfs send, we found it take large
amount of cpu time on page_cache_sync_readahead. This effort
can be reduced after switching to asynchronous one. Overall
performance gain on HDD and SSD were 9 and 15 respectively if
simply send a large file.
Signed-off-by: Kuanling Huang <peterh@synology.com>
---
fs/btrfs/send.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 63a6152..ac67ff6 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -4475,16 +4475,27 @@ static ssize_t fill_read_buf(struct send_ctx *sctx, u64 offset, u32 len)
/* initial readahead */
memset(&sctx->ra, 0, sizeof(struct file_ra_state));
file_ra_state_init(&sctx->ra, inode->i_mapping);
- btrfs_force_ra(inode->i_mapping, &sctx->ra, NULL, index,
- last_index - index + 1);
while (index <= last_index) {
unsigned cur_len = min_t(unsigned, len,
PAGE_CACHE_SIZE - pg_offset);
- page = find_or_create_page(inode->i_mapping, index, GFP_NOFS);
+ page = find_lock_page(inode->i_mapping, index);
if (!page) {
- ret = -ENOMEM;
- break;
+ page_cache_sync_readahead(inode->i_mapping,
+ &sctx->ra, NULL, index,
+ last_index + 1 - index);
+
+ page = find_or_create_page(inode->i_mapping, index, GFP_NOFS);
+ if (unlikely(!page)) {
+ ret = -ENOMEM;
+ break;
+ }
+ }
+
+ if (PageReadahead(page)) {
+ page_cache_async_readahead(inode->i_mapping,
+ &sctx->ra, NULL, page, index,
+ last_index + 1 - index);
}
if (!PageUptodate(page)) {
--
1.9.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] Btrfs: incremental send, apply asynchronous page cache readahead
2017-09-13 6:38 [PATCH] Btrfs: incremental send, apply asynchronous page cache readahead peterh
@ 2017-09-13 9:33 ` Pasi Kärkkäinen
2017-09-13 9:46 ` peterh
2017-09-13 10:45 ` Filipe Manana
1 sibling, 1 reply; 5+ messages in thread
From: Pasi Kärkkäinen @ 2017-09-13 9:33 UTC (permalink / raw)
To: peterh; +Cc: linux-btrfs
Hi,
On Wed, Sep 13, 2017 at 02:38:49PM +0800, peterh wrote:
> From: Kuanling Huang <peterh@synology.com>
>
> By analyzing the perf on btrfs send, we found it take large
> amount of cpu time on page_cache_sync_readahead. This effort
> can be reduced after switching to asynchronous one. Overall
> performance gain on HDD and SSD were 9 and 15 respectively if
> simply send a large file.
>
hmm, 9 and 15 what?
-- Pasi
> Signed-off-by: Kuanling Huang <peterh@synology.com>
> ---
> fs/btrfs/send.c | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index 63a6152..ac67ff6 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -4475,16 +4475,27 @@ static ssize_t fill_read_buf(struct send_ctx *sctx, u64 offset, u32 len)
> /* initial readahead */
> memset(&sctx->ra, 0, sizeof(struct file_ra_state));
> file_ra_state_init(&sctx->ra, inode->i_mapping);
> - btrfs_force_ra(inode->i_mapping, &sctx->ra, NULL, index,
> - last_index - index + 1);
>
> while (index <= last_index) {
> unsigned cur_len = min_t(unsigned, len,
> PAGE_CACHE_SIZE - pg_offset);
> - page = find_or_create_page(inode->i_mapping, index, GFP_NOFS);
> + page = find_lock_page(inode->i_mapping, index);
> if (!page) {
> - ret = -ENOMEM;
> - break;
> + page_cache_sync_readahead(inode->i_mapping,
> + &sctx->ra, NULL, index,
> + last_index + 1 - index);
> +
> + page = find_or_create_page(inode->i_mapping, index, GFP_NOFS);
> + if (unlikely(!page)) {
> + ret = -ENOMEM;
> + break;
> + }
> + }
> +
> + if (PageReadahead(page)) {
> + page_cache_async_readahead(inode->i_mapping,
> + &sctx->ra, NULL, page, index,
> + last_index + 1 - index);
> }
>
> if (!PageUptodate(page)) {
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Btrfs: incremental send, apply asynchronous page cache readahead
2017-09-13 9:33 ` Pasi Kärkkäinen
@ 2017-09-13 9:46 ` peterh
0 siblings, 0 replies; 5+ messages in thread
From: peterh @ 2017-09-13 9:46 UTC (permalink / raw)
To: Pasi Kärkkäinen; +Cc: linux-btrfs
Hi Pasi
Sorry for missing the word.
9 and 15 percent performance gain on HDD and SSD after
the patch is applied.
Kuanling
Pasi Kärkkäinen 於 2017-09-13 17:33 寫到:
> Hi,
>
> On Wed, Sep 13, 2017 at 02:38:49PM +0800, peterh wrote:
>> From: Kuanling Huang <peterh@synology.com>
>>
>> By analyzing the perf on btrfs send, we found it take large
>> amount of cpu time on page_cache_sync_readahead. This effort
>> can be reduced after switching to asynchronous one. Overall
>> performance gain on HDD and SSD were 9 and 15 respectively if
>> simply send a large file.
>>
>
> hmm, 9 and 15 what?
>
>
> -- Pasi
>
>> Signed-off-by: Kuanling Huang <peterh@synology.com>
>> ---
>> fs/btrfs/send.c | 21 ++++++++++++++++-----
>> 1 file changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
>> index 63a6152..ac67ff6 100644
>> --- a/fs/btrfs/send.c
>> +++ b/fs/btrfs/send.c
>> @@ -4475,16 +4475,27 @@ static ssize_t fill_read_buf(struct send_ctx
>> *sctx, u64 offset, u32 len)
>> /* initial readahead */
>> memset(&sctx->ra, 0, sizeof(struct file_ra_state));
>> file_ra_state_init(&sctx->ra, inode->i_mapping);
>> - btrfs_force_ra(inode->i_mapping, &sctx->ra, NULL, index,
>> - last_index - index + 1);
>>
>> while (index <= last_index) {
>> unsigned cur_len = min_t(unsigned, len,
>> PAGE_CACHE_SIZE - pg_offset);
>> - page = find_or_create_page(inode->i_mapping, index, GFP_NOFS);
>> + page = find_lock_page(inode->i_mapping, index);
>> if (!page) {
>> - ret = -ENOMEM;
>> - break;
>> + page_cache_sync_readahead(inode->i_mapping,
>> + &sctx->ra, NULL, index,
>> + last_index + 1 - index);
>> +
>> + page = find_or_create_page(inode->i_mapping, index, GFP_NOFS);
>> + if (unlikely(!page)) {
>> + ret = -ENOMEM;
>> + break;
>> + }
>> + }
>> +
>> + if (PageReadahead(page)) {
>> + page_cache_async_readahead(inode->i_mapping,
>> + &sctx->ra, NULL, page, index,
>> + last_index + 1 - index);
>> }
>>
>> if (!PageUptodate(page)) {
>> --
>> 1.9.1
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Btrfs: incremental send, apply asynchronous page cache readahead
2017-09-13 6:38 [PATCH] Btrfs: incremental send, apply asynchronous page cache readahead peterh
2017-09-13 9:33 ` Pasi Kärkkäinen
@ 2017-09-13 10:45 ` Filipe Manana
2017-09-15 8:53 ` peterh
1 sibling, 1 reply; 5+ messages in thread
From: Filipe Manana @ 2017-09-13 10:45 UTC (permalink / raw)
To: peterh; +Cc: linux-btrfs@vger.kernel.org
On Wed, Sep 13, 2017 at 7:38 AM, peterh <peterh@synology.com> wrote:
> From: Kuanling Huang <peterh@synology.com>
>
> By analyzing the perf on btrfs send, we found it take large
> amount of cpu time on page_cache_sync_readahead. This effort
> can be reduced after switching to asynchronous one. Overall
> performance gain on HDD and SSD were 9 and 15 respectively if
> simply send a large file.
Besides what was pointed before, about saying what those 9 and 15 are,
the subject mentions incremental send, but there's nothing here that
is specific to incremental sends, as it applies to full send
operations as well, so please also change the subject.
>
> Signed-off-by: Kuanling Huang <peterh@synology.com>
> ---
> fs/btrfs/send.c | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index 63a6152..ac67ff6 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -4475,16 +4475,27 @@ static ssize_t fill_read_buf(struct send_ctx *sctx, u64 offset, u32 len)
> /* initial readahead */
> memset(&sctx->ra, 0, sizeof(struct file_ra_state));
> file_ra_state_init(&sctx->ra, inode->i_mapping);
> - btrfs_force_ra(inode->i_mapping, &sctx->ra, NULL, index,
> - last_index - index + 1);
>
> while (index <= last_index) {
> unsigned cur_len = min_t(unsigned, len,
> PAGE_CACHE_SIZE - pg_offset);
> - page = find_or_create_page(inode->i_mapping, index, GFP_NOFS);
You based this patch on an old code base. Currently it is GFP_KERNEL
and not GFP_NOFS anymore.
Please update the patch.
> + page = find_lock_page(inode->i_mapping, index);
> if (!page) {
> - ret = -ENOMEM;
> - break;
> + page_cache_sync_readahead(inode->i_mapping,
> + &sctx->ra, NULL, index,
> + last_index + 1 - index);
> +
> + page = find_or_create_page(inode->i_mapping, index, GFP_NOFS);
> + if (unlikely(!page)) {
Please avoid polluting the code with unlikely/likely macros (unless
there's really a significant performance win, which isn't the case
here I bet).
> + ret = -ENOMEM;
> + break;
> + }
> + }
> +
> + if (PageReadahead(page)) {
> + page_cache_async_readahead(inode->i_mapping,
> + &sctx->ra, NULL, page, index,
> + last_index + 1 - index);
> }
>
> if (!PageUptodate(page)) {
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Btrfs: incremental send, apply asynchronous page cache readahead
2017-09-13 10:45 ` Filipe Manana
@ 2017-09-15 8:53 ` peterh
0 siblings, 0 replies; 5+ messages in thread
From: peterh @ 2017-09-15 8:53 UTC (permalink / raw)
To: fdmanana; +Cc: linux-btrfs
Much appreciate your suggestion. I've modified the patch based on your
advice and sent out a new patch with new subject "Btrfs: send, apply
asynchronous
page cache readahead to enhance page read".
Filipe Manana 於 2017-09-13 18:45 寫到:
> On Wed, Sep 13, 2017 at 7:38 AM, peterh <peterh@synology.com> wrote:
>> From: Kuanling Huang <peterh@synology.com>
>>
>> By analyzing the perf on btrfs send, we found it take large
>> amount of cpu time on page_cache_sync_readahead. This effort
>> can be reduced after switching to asynchronous one. Overall
>> performance gain on HDD and SSD were 9 and 15 respectively if
>> simply send a large file.
>
> Besides what was pointed before, about saying what those 9 and 15 are,
> the subject mentions incremental send, but there's nothing here that
> is specific to incremental sends, as it applies to full send
> operations as well, so please also change the subject.
>>
>> Signed-off-by: Kuanling Huang <peterh@synology.com>
>> ---
>> fs/btrfs/send.c | 21 ++++++++++++++++-----
>> 1 file changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
>> index 63a6152..ac67ff6 100644
>> --- a/fs/btrfs/send.c
>> +++ b/fs/btrfs/send.c
>> @@ -4475,16 +4475,27 @@ static ssize_t fill_read_buf(struct send_ctx
>> *sctx, u64 offset, u32 len)
>> /* initial readahead */
>> memset(&sctx->ra, 0, sizeof(struct file_ra_state));
>> file_ra_state_init(&sctx->ra, inode->i_mapping);
>> - btrfs_force_ra(inode->i_mapping, &sctx->ra, NULL, index,
>> - last_index - index + 1);
>>
>> while (index <= last_index) {
>> unsigned cur_len = min_t(unsigned, len,
>> PAGE_CACHE_SIZE - pg_offset);
>> - page = find_or_create_page(inode->i_mapping, index,
>> GFP_NOFS);
>
> You based this patch on an old code base. Currently it is GFP_KERNEL
> and not GFP_NOFS anymore.
> Please update the patch.
>
>> + page = find_lock_page(inode->i_mapping, index);
>> if (!page) {
>> - ret = -ENOMEM;
>> - break;
>> + page_cache_sync_readahead(inode->i_mapping,
>> + &sctx->ra, NULL, index,
>> + last_index + 1 - index);
>> +
>> + page = find_or_create_page(inode->i_mapping,
>> index, GFP_NOFS);
>> + if (unlikely(!page)) {
>
> Please avoid polluting the code with unlikely/likely macros (unless
> there's really a significant performance win, which isn't the case
> here I bet).
>
>
>> + ret = -ENOMEM;
>> + break;
>> + }
>> + }
>> +
>> + if (PageReadahead(page)) {
>> + page_cache_async_readahead(inode->i_mapping,
>> + &sctx->ra, NULL, page, index,
>> + last_index + 1 - index);
>> }
>>
>> if (!PageUptodate(page)) {
>> --
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
>> in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-09-15 8:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-13 6:38 [PATCH] Btrfs: incremental send, apply asynchronous page cache readahead peterh
2017-09-13 9:33 ` Pasi Kärkkäinen
2017-09-13 9:46 ` peterh
2017-09-13 10:45 ` Filipe Manana
2017-09-15 8:53 ` peterh
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).