linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Su Yue <Damenly_Su@gmx.com>
To: Qu Wenruo <wqu@suse.com>, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH 3/9] btrfs-progs: image: Fix a access-beyond-boundary bug when there are 32 online CPUs
Date: Mon, 10 Jun 2019 09:23:42 +0800	[thread overview]
Message-ID: <d6909877-43e9-b66d-6d77-cf58820e64a9@gmx.com> (raw)
In-Reply-To: <20190606110611.27176-4-wqu@suse.com>



On 2019/6/6 7:06 PM, Qu Wenruo wrote:
> [BUG]
> When there are over 32 (in my example, 35) online CPUs, btrfs-image -c9
> will just hang.
>
> [CAUSE]
> Btrfs-image has a hard coded limit (32) on how many threads we can use.
> For the "-t" option we do the up limit check.
>
> But when we don't specify "-t" option and speicified "-c" option, then
> btrfs-image will try to auto detect the number of online CPUs, and use
> it without checking if it's over the up limit.
>
> And for num_threads larger than the up limit, we will over write the
> adjust members of metadump_struct/mdrestore_struct, corrupting
> pthread_mutex_t and pthread_cond_t, causing synchronising problem.
>
> Nowadays, with SMT/HT and higher cpu core counts, it's not hard to go
> beyond 32 threads, and hit the bug.
>
> [FIX]
> Just do extra num_threads check before using the number from sysconf().
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>

This does fix an issue.
And as the commit says, why limit the max threads to 32?
Does it still make sense in nowadays multiple cores CPU?
Can we increase the limit?
However, this is another story.

For this patch:
Reviewed-by: Su Yue <Damenly_Su@gmx.com>

> ---
>   image/main.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/image/main.c b/image/main.c
> index fb9fc48c..80f09c21 100644
> --- a/image/main.c
> +++ b/image/main.c
> @@ -2758,6 +2758,7 @@ int main(int argc, char *argv[])
>
>   			if (tmp <= 0)
>   				tmp = 1;
> +			tmp = min_t(long, tmp, MAX_WORKER_THREADS);
>   			num_threads = tmp;
>   		}
>   	} else {
>


  reply	other threads:[~2019-06-10  1:23 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-06 11:06 [PATCH 0/9] btrfs-progs: image: Data dump support, restore optimization and small fixes Qu Wenruo
2019-06-06 11:06 ` [PATCH 1/9] btrfs-progs: image: Use SZ_* to replace intermediate size Qu Wenruo
2019-06-06 11:06 ` [PATCH 2/9] btrfs-progs: image: Fix a indent misalign Qu Wenruo
2019-06-06 11:06 ` [PATCH 3/9] btrfs-progs: image: Fix a access-beyond-boundary bug when there are 32 online CPUs Qu Wenruo
2019-06-10  1:23   ` Su Yue [this message]
2019-06-10  1:28     ` Qu Wenruo
2019-06-06 11:06 ` [PATCH 4/9] btrfs-progs: image: Verify the superblock before restore Qu Wenruo
2019-06-06 11:06 ` [PATCH 5/9] btrfs-progs: image: Introduce framework for more dump versions Qu Wenruo
2019-06-06 11:06 ` [PATCH 6/9] btrfs-progs: image: Introduce -d option to dump data Qu Wenruo
2019-06-06 11:06 ` [PATCH 7/9] btrfs-progs: image: Allow restore to record system chunk ranges for later usage Qu Wenruo
2019-06-06 11:06 ` [PATCH 8/9] btrfs-progs: image: Introduce helper to determine if a tree block is in the range of system chunks Qu Wenruo
2019-06-06 11:06 ` [PATCH 9/9] btrfs-progs: image: Rework how we search chunk tree blocks Qu Wenruo
2019-06-14 15:48 ` [PATCH 0/9] btrfs-progs: image: Data dump support, restore optimization and small fixes David Sterba

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=d6909877-43e9-b66d-6d77-cf58820e64a9@gmx.com \
    --to=damenly_su@gmx.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=wqu@suse.com \
    /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 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).