Received-SPF: pass (google.com: domain of linux-kernel+bounces-117921-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Sender: Tejun Heo <htejun@gmail.com>
Date: Mon, 25 Mar 2024 10:26:20 -1000
From: Tejun Heo <tj@kernel.org>
To: Kemeng Shi <shikemeng@huaweicloud.com>
Cc: akpm@linux-foundation.org, linux-mm@kvack.org,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	willy@infradead.org, bfoster@redhat.com, jack@suse.cz,
	dsterba@suse.com, mjguzik@gmail.com, dhowells@redhat.com,
	peterz@infradead.org
Subject: Re: [PATCH 6/6] writeback: remove unneeded GDTC_INIT_NO_WB
Message-ID: <ZgHd7GcUslrBEeoi@slm.duckdns.org>
References: <20240320110222.6564-1-shikemeng@huaweicloud.com>
 <20240320110222.6564-7-shikemeng@huaweicloud.com>
 <Zfr9my_tfxO-N6HS@mtj.duckdns.org>
 <becdb16b-a318-ec05-61d2-d190541ae997@huaweicloud.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <becdb16b-a318-ec05-61d2-d190541ae997@huaweicloud.com>

On Thu, Mar 21, 2024 at 03:12:21PM +0800, Kemeng Shi wrote:
> 
> 
> on 3/20/2024 11:15 PM, Tejun Heo wrote:
> > Hello,
> > 
> > On Wed, Mar 20, 2024 at 07:02:22PM +0800, Kemeng Shi wrote:
> >> We never use gdtc->dom set with GDTC_INIT_NO_WB, just remove unneeded
> >> GDTC_INIT_NO_WB
> >>
> >> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
> > ...
> >>  void global_dirty_limits(unsigned long *pbackground, unsigned long *pdirty)
> >>  {
> >> -	struct dirty_throttle_control gdtc = { GDTC_INIT_NO_WB };
> >> +	struct dirty_throttle_control gdtc = { };
> > 
> > Even if it's currently not referenced, wouldn't it still be better to always
> > guarantee that a dtc's dom is always initialized? I'm not sure what we get
> > by removing this.
> As we explicitly use GDTC_INIT_NO_WB to set global_wb_domain before
> calculating dirty limit with domain_dirty_limits, I intuitively think the dirty
> limit calculation in domain_dirty_limits is related to global_wb_domain when
> CONFIG_WRITEBACK_CGROUP is enabled while the truth is not. So this is a little
> confusing to me.
> Would it be acceptable to you that we keep useing GDTC_INIT_NO_WB but
> define GDTC_INIT_NO_WB to null fow now and redefine GDTC_INIT_NO_WB when some
> member of gdtc is really needed.
> Of couse I'm not insistent on this. Would like to hear you suggestion. Thanks!

Ah, I see. In that case, the proposed change of removing GDTC_INIT_NO_WB
looks good to me.

Thanks.

-- 
tejun