From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from stravinsky.debian.org (stravinsky.debian.org [82.195.75.108]) (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 0A2732EB0F for ; Tue, 10 Mar 2026 16:32:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=82.195.75.108 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773160341; cv=none; b=bm8uUTaXbJ6asEfpQmyeRLUHN+a9yBkST6m0e8P7hR7JidbU/4ASsX05ksfrSme+8hJSL4+RgXImO14IAVX1RkCfxjmMmMAOuW1BjZFWMTvqynHuZ+23oM/IR9NEQ/EEZef3acZGK4C3e8bRdiTr54iU9KtfCrI8Q/KrEtJJmXg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773160341; c=relaxed/simple; bh=TG5DNomL7HxGfZPhJCvLia5vxNe8+sadJwi7twlQFfU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=C5uujBFYgb0wBngJDC/+D5VOVrVYDgiJQWkqgeQAERaIOlwDaFnmJiYtyFYXAnKcaBDUDxX2+xHDAK1JdIF+2i1R12XuUqc08DRaAV8IjvHtNjKDcBID15oVvdIiCg98RGh4qpcfE9y/V5loUkSe0WOws2WcMhtZFgVdyVMKTCk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=debian.org; spf=none smtp.mailfrom=debian.org; dkim=pass (2048-bit key) header.d=debian.org header.i=@debian.org header.b=VKyNj+IB; arc=none smtp.client-ip=82.195.75.108 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=debian.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=debian.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=debian.org header.i=@debian.org header.b="VKyNj+IB" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debian.org; s=smtpauto.stravinsky; h=X-Debian-User:In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=GCRfJr6MPjn4DphBdnrts5oH/u8hqxi4oo9mnLAJdfI=; b=VKyNj+IBMRhuSfakqpigRiDlpa ao2dQIquGGT/aOBg4BhZDShCvzICeQ/qVAc3nRpnopfYYwf7fhcN9EbzFgQXR9LfTPCQ7kYyJrqSe 31GsGjiuTk/fxcg7MUxbbMe0UgIr7cpTmTgDdWHMh6Qs73MDFnMmI3kkHiZwjQLd5NSyOsx6wuvW7 HSifkXxybWNO3lGqFnI1daWa0pmY05yusUwqbrz7V/sAPhk7KDr2eOh26JYX2hvJbCGI3q/xrgilE QMh7zBgdK1VBebfSFaZ9CtEPcYRPtXA3UNwmgpq9ZtSo/adwin+h6SdsjiIErRSRfYJOcMe8BNUDj GCdA4ctw==; Received: from authenticated user by stravinsky.debian.org with esmtpsa (TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_256_GCM:256) (Exim 4.94.2) (envelope-from ) id 1vzzzt-0037gW-BA; Tue, 10 Mar 2026 16:31:53 +0000 Date: Tue, 10 Mar 2026 09:31:46 -0700 From: Breno Leitao To: "David Hildenbrand (Arm)" Cc: Andrew Morton , 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 , Mike Rapoport , linux-mm@kvack.org, linux-kernel@vger.kernel.org, usamaarif642@gmail.com, kas@kernel.org, kernel-team@meta.com, "Lorenzo Stoakes (Oracle)" Subject: Re: [PATCH v4 2/4] mm: huge_memory: refactor anon_enabled_store() with change_anon_orders() Message-ID: References: <20260309-thp_logs-v4-0-926b9840083e@debian.org> <20260309-thp_logs-v4-2-926b9840083e@debian.org> <4f2abf42-983f-4cc2-92f5-c81827e7b7e2@kernel.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: <4f2abf42-983f-4cc2-92f5-c81827e7b7e2@kernel.org> X-Debian-User: leitao On Mon, Mar 09, 2026 at 02:43:11PM +0100, David Hildenbrand (Arm) wrote: > On 3/9/26 12:07, Breno Leitao wrote: > > +static bool change_anon_orders(int order, enum anon_enabled_mode mode) > > I would suggest something a bit longer but clearer > > "set_anon_enabled_mode_for_order()" > > Or shorter > > "set_anon_enabled_mode" set_anon_enabled_mode() seems to be better. Then I have: set_global_enabled_mode() and set_anon_enabled_mode(). > 1) set vs. change. the function returns whether actually something > changed. > > 2) We're not really changing "anon_orders". Yeah, we're updating > variables that are named "huge_anon_orders_XXX", but that's more an > implementation detail when setting the anon_enabled mode for a > specific order. > > > +{ > > + static unsigned long *orders[] = { > > + &huge_anon_orders_always, > > + &huge_anon_orders_madvise, > > + &huge_anon_orders_inherit, > > + }; > > Having a "order" and "orders" variable that have different semantics is > a bit confusing. Don't really have a better suggestion. "enabled_orders" > ? hm. Ack. renaming to enabled_orders. > > + enum anon_enabled_mode m; > > + bool changed = false; > > + > > + spin_lock(&huge_anon_orders_lock); > > + for (m = 0; m < ARRAY_SIZE(orders); m++) { > > + if (m == mode) > > + changed |= !test_and_set_bit(order, orders[m]); > > + else > > + changed |= test_and_clear_bit(order, orders[m]); > > + } > > Can we use the non-atomic variant here? __test_and_set_bit(). Just > wondering as the lock protects concurrent modifications. Ack! I will respin a new version, --breno