Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Sender: Tejun Heo <htejun@gmail.com>
Date:   Mon, 8 May 2023 13:50:50 -1000
From:   Tejun Heo <tj@kernel.org>
To:     Wang Yugui <wangyugui@e16-tech.com>
Cc:     jiangshanlai@gmail.com, linux-kernel@vger.kernel.org,
        kernel-team@meta.com, Chris Mason <clm@fb.com>,
        Josef Bacik <josef@toxicpanda.com>,
        David Sterba <dsterba@suse.com>, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH v2 16/22] btrfs: Use alloc_ordered_workqueue() to create
 ordered workqueues
Message-ID: <ZFmK2uV6n8cJ1ec0@slm.duckdns.org>
References: <20230430124006.49D2.409509F4@e16-tech.com>
 <ZFWKHRCDsaNDNXIj@slm.duckdns.org>
 <20230506094013.29A6.409509F4@e16-tech.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230506094013.29A6.409509F4@e16-tech.com>
Precedence: bulk

On Sat, May 06, 2023 at 09:40:14AM +0800, Wang Yugui wrote:
> by test, I noticed some warning caused by
> void workqueue_set_max_active(struct workqueue_struct *wq, int max_active)
> 	if (WARN_ON(wq->flags & __WQ_ORDERED_EXPLICIT))
> 		return;
> 
> so I tested again  with the flowing fix
> 
> diff --git a/fs/btrfs/async-thread.c b/fs/btrfs/async-thread.c
> index 43c8995..e4b68e9 100644
> --- a/fs/btrfs/async-thread.c
> +++ b/fs/btrfs/async-thread.c
> @@ -99,8 +99,11 @@ struct btrfs_workqueue *btrfs_alloc_workqueue(struct btrfs_fs_info *fs_info,
>  		ret->thresh = thresh;
>  	}
>  
> -	ret->normal_wq = alloc_workqueue("btrfs-%s", flags, ret->current_active,
> -					 name);
> +	if(limit_active == 1)
> +		ret->normal_wq = alloc_ordered_workqueue("btrfs-%s", flags, name);
> +	else
> +		ret->normal_wq = alloc_workqueue("btrfs-%s", flags,
> +					 ret->current_active, name);
>  	if (!ret->normal_wq) {
>  		kfree(ret);
>  		return NULL;
> @@ -139,7 +139,7 @@ static inline void thresh_exec_hook(struct btrfs_workqueue *wq)
>  	long pending;
>  	int need_change = 0;
>  
> -	if (wq->thresh == NO_THRESHOLD)
> +	if (wq->thresh == NO_THRESHOLD || wq->limit_active == 1)
>  		return;
>  
>  	atomic_dec(&wq->pending);
> 
> we need 'limit_active' at 2nd postition, so I used 'limit_active' and 1st
> postition too.

Oh, that most likely means that these workqueues don't need to and shouldn't
be ordered. Will update the patch.

Thanks.

-- 
tejun