From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id F04613659F6 for ; Wed, 4 Mar 2026 16:24:57 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772641498; cv=none; b=u+cKBwmLfK+YIDDJbPzE036tZndcw4WJcDqDuh781HK4/V8PA8IsqKYoKxPHtqEOukVU2hexB3P1GN1Z6cRI2QM3ZGwaXOQJURKj0bZZ83L1kYzaDLzLOt/+68+tDRLo2/HywcUyvHomQyBa6NAI3YzUiwfywQehuCTidMzCYys= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772641498; c=relaxed/simple; bh=j4hbTQdifMqORZ0PrgMoZSeSr+3dRg5OwZLs7vTvKYA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=h3y4phC0JaYsnzvTfxyg+gcCJI6P598IvTX1bzRhtyoJiqPfWxZ9KQxMYI01Sj+YQLjqgUBDUBfdgOKqS4b9t/DKN9PQslHdjX8efYO0hGqIIYgw0PbIcag3tNojDAVWXZm0fAhcGc6jGtj6Ta4uxzt5HRmTtB3HKdP/SVo8RkI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=XfKLtNiN; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="XfKLtNiN" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 07EBCC4CEF7; Wed, 4 Mar 2026 16:24:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1772641497; bh=j4hbTQdifMqORZ0PrgMoZSeSr+3dRg5OwZLs7vTvKYA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=XfKLtNiN+7cXhgOaY/4IIZr4wb0AwwH2qIi6du/TOf1O1/mrsfn3Q8GhUXRRWkm1t tHogCZXYYJ/R7nlsZ+0kAadJyuecltdOxTq5OK9lSwsY5MLUI/lSi7qc7Kxy+dhU6w SHx/+lncGEgD7T/UkYR9YLULkQAdShd5YWj12Gra4z8xi3AtsBm7ieFvHjvc/XHVuQ o859Yfo7gI0iBBQvPEklLQW1Txl1H6xIBfOOTAeMNsL3Lx5OXnfuStqvUDR2dIk24C pcu3FT5lI54AOKJ6p+aPdUCB+DvfpHDHW1GFE79MzkxNH1JJzEcIwr7slAddsYD4fC T9IB00is680tA== Date: Wed, 4 Mar 2026 16:24:54 +0000 From: "Lorenzo Stoakes (Oracle)" To: Kiryl Shutsemau Cc: Breno Leitao , Andrew Morton , David Hildenbrand , Lorenzo Stoakes , Zi Yan , Baolin Wang , "Liam R. Howlett" , Nico Pache , Ryan Roberts , Dev Jain , Barry Song , Lance Yang , Vlastimil Babka , Suren Baghdasaryan , Michal Hocko , Brendan Jackman , Johannes Weiner , linux-mm@kvack.org, linux-kernel@vger.kernel.org, usamaarif642@gmail.com, kernel-team@meta.com Subject: Re: [PATCH 0/2] mm: thp: reduce unnecessary start_stop_khugepaged() calls Message-ID: References: <20260304-thp_logs-v1-0-59038218a253@debian.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Wed, Mar 04, 2026 at 11:18:37AM +0000, Kiryl Shutsemau wrote: > On Wed, Mar 04, 2026 at 02:22:32AM -0800, Breno Leitao wrote: > > Breno Leitao (2): > > mm: thp: avoid calling start_stop_khugepaged() in anon_enabled_store() > > mm: thp: avoid calling start_stop_khugepaged() in enabled_store() > > I think the same can be achieved cleaner from within start_stop_khugepaged(). I'm kind of with you in doing it here but... > > Completely untested patch is below. > > One noticeable difference that with the patch we don't kick > khugepaged_wait if khugepaged_thread is there. But I don't think it > should make a difference. > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > index 1dd3cfca610d..80f818d3a094 100644 > --- a/mm/khugepaged.c > +++ b/mm/khugepaged.c > @@ -2683,18 +2683,18 @@ static void set_recommended_min_free_kbytes(void) > > int start_stop_khugepaged(void) > { > - int err = 0; > + guard(mutex)(&khugepaged_mutex); This is nice :) we need more guard usage in mm. > + > + /* Check if anything has to be done */ > + if (hugepage_pmd_enabled() == !!khugepaged_thread) This is horrible, better to break out the latter expression as a small static function like: static bool is_khugepaged_thread_running(void) { if (khugepaged_thread) return true; return false; } > + return 0; > > - mutex_lock(&khugepaged_mutex); > if (hugepage_pmd_enabled()) { Can we race on this function call, meaning check above might vary from one here? Better to have bool enabled = hugepage_pmd_enabled(); etc. etc. > - if (!khugepaged_thread) > - khugepaged_thread = kthread_run(khugepaged, NULL, > - "khugepaged"); > + khugepaged_thread = kthread_run(khugepaged, NULL, "khugepaged"); > if (IS_ERR(khugepaged_thread)) { > pr_err("khugepaged: kthread_run(khugepaged) failed\n"); > - err = PTR_ERR(khugepaged_thread); > khugepaged_thread = NULL; > - goto fail; > + return PTR_ERR(khugepaged_thread); > } > > if (!list_empty(&khugepaged_scan.mm_head)) > @@ -2703,10 +2703,9 @@ int start_stop_khugepaged(void) > kthread_stop(khugepaged_thread); > khugepaged_thread = NULL; > } > + > set_recommended_min_free_kbytes(); We are setting recommended min free kbytes each time somebody touches these flags, maybe somebody somewhere relies on that and this change would break them? So maybe we just want to do this each time anyway, but repress/rate limit output? > -fail: > - mutex_unlock(&khugepaged_mutex); > - return err; > + return 0; > } > > void khugepaged_min_free_kbytes_update(void) > -- > Kiryl Shutsemau / Kirill A. Shutemov Thanks, Lorenzo