From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (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 E371229BDB1 for ; Wed, 22 Jul 2026 00:55:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784681747; cv=none; b=ZWU4yE8NLK7H6oRymnCPhMY7Gg/gGfFsd06FjXD3f0n+lgsYn5XrCwXNEOIoklTh+P7ooLUCJH0DuTCdfl2sXZ3sNn3Zy+hGyuPSdBIotLd0EPAVQSmuaQ35RyEUgYN43yUCwDs7pRVrPE9x/1kJ6Lw5vKs4vrAF97YXTuSx42A= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784681747; c=relaxed/simple; bh=9A3h3eu8YrzUdWvqeH8Xe0qqnOKxY02Sj4FpSa4UsEE=; h=Date:From:To:Cc:Subject:Message-Id:In-Reply-To:References: Mime-Version:Content-Type; b=Aoz9hJOrbTmcinNMES4Sps8nlok22KpY5Xnd5+zWGsRHZ/8WW0tKGO7X3D9uxu/rEViZlYXkAarKrl8tN3OAe8XwJp7iIoD10N5STuO2BfFfdHgTrvX6lZhaTI/trkixa6Aceth2nS1Ny7gZX6/BIW/C6YThLSoo90Le0IvMToY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux-foundation.org header.i=@linux-foundation.org header.b=hWtJVb9g; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux-foundation.org header.i=@linux-foundation.org header.b="hWtJVb9g" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 732701F000E9; Wed, 22 Jul 2026 00:55:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=korg; t=1784681744; bh=U0QW8ZsmS+nE37A5nW/s8wH8YzZiNTBe124WuGdmmtU=; h=Date:From:To:Cc:Subject:In-Reply-To:References; b=hWtJVb9gAKDbevXQH16mN1PBzSL0omMRBophVdF5wVB80PYyvhZKSBymL4sDtfSpN 2rxPd4LwvBeYJuC8u27gjjqBfCcxNkLi72yxHOh6MKoY2BTNgql+vI2eKyvYfTg6Yd va71f/k+h+LUbeBtvQSsX6/biO02PW6Uw7vDKFNo= Date: Tue, 21 Jul 2026 17:55:44 -0700 From: Andrew Morton To: Yichong Chen Cc: Muchun Song , Oscar Salvador , Joshua Hahn , David Hildenbrand , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] hugetlb: evaluate subpool free state while locked Message-Id: <20260721175544.11f896c43d2b0fa1cf80c1e9@linux-foundation.org> In-Reply-To: <20260721035207.1437935-1-chenyichong@uniontech.com> References: <20260721035207.1437935-1-chenyichong@uniontech.com> X-Mailer: Sylpheed 3.8.0beta1 (GTK+ 2.24.33; x86_64-pc-linux-gnu) 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-Transfer-Encoding: 7bit On Tue, 21 Jul 2026 11:52:07 +0800 Yichong Chen wrote: > unlock_or_release_subpool() drops spool->lock before calling > subpool_is_free(). However, subpool_is_free() reads fields that are > updated under spool->lock, including count, used_hpages and rsv_hpages. > > Keep the free-state evaluation under the same lock that protects those > fields. The reservation accounting and kfree() calls still happen after > dropping spool->lock. > > Reviewed-by: Joshua Hahn > Signed-off-by: Yichong Chen > --- > v2: > - Reword the changelog based on Joshua's observation. > - Drop the Fixes tag because this is not known to cause a user-visible bug. Retaining the Fixes: would be OK. It's potentially useful information. If a patch doesn't fix a user-visible bug then we consider it inappropriate to backport it (by including Cc:stable). The -stable maintainers have been asked not to backport Fixes: patches which lack the cc:stable. > > ... > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -140,12 +140,14 @@ static inline bool subpool_is_free(struct hugepage_subpool *spool) > static inline void unlock_or_release_subpool(struct hugepage_subpool *spool, > unsigned long irq_flags) > { > - spin_unlock_irqrestore(&spool->lock, irq_flags); > + bool free_subpool = subpool_is_free(spool); > > /* If no pages are used, and no other handles to the subpool > * remain, give up any reservations based on minimum size and > * free the subpool */ > - if (subpool_is_free(spool)) { > + spin_unlock_irqrestore(&spool->lock, irq_flags); > + > + if (free_subpool) { > if (spool->min_hpages != -1) > hugetlb_acct_memory(spool->hstate, > -spool->min_hpages); OK, better, but the value of `free_subpool' can become out of date as soon as we drop that lock - some other thread could get in and start using *spool. If that subpool is still findable, which it hopefully isn't.