linux-kernel - Re: [PATCH] fs/resctl: use for_each_bit_from() in __check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aHwQxsmBj1lZBwEF@yury>
Date: Sat, 19 Jul 2025 17:40:22 -0400
From: Yury Norov <yury.norov@...il.com>
To: Tony Luck <tony.luck@...el.com>,
	Reinette Chatre <reinette.chatre@...el.com>,
	Dave Martin <Dave.Martin@....com>,
	James Morse <james.morse@....com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] fs/resctl: use for_each_bit_from() in __check_limbo()

On Sat, Jul 19, 2025 at 05:34:23PM -0400, Yury Norov wrote:
> From: Yury Norov (NVIDIA) <yury.norov@...il.com>
> 
> The function opencodes for_each_set_bit_from(). Switch to the dedicated
> macro, and drop some housekeeping code.
> 
> While there, switch to non-atomic __clear_bit(), because atomicity is
> not possible in this context, and that may confuse readers.

s/possible/guaranteed

Because find_next_bit() and therefore for_each() iterator are not
atomic, we can't make sure that concurrent threads won't pick the
same idx, therefore atomic clear_bit() may confuse readers.

But atomicity is possible if we use external lock, or
test_and_clear_bit(), if needed.

> 
> Signed-off-by: Yury Norov (NVIDIA) <yury.norov@...il.com>
> ---
>  fs/resctrl/monitor.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
> index f5637855c3ac..3e1c14214ea8 100644
> --- a/fs/resctrl/monitor.c
> +++ b/fs/resctrl/monitor.c
> @@ -135,7 +135,7 @@ void __check_limbo(struct rdt_mon_domain *d, bool force_free)
>  	struct rdt_resource *r = resctrl_arch_get_resource(RDT_RESOURCE_L3);
>  	u32 idx_limit = resctrl_arch_system_num_rmid_idx();
>  	struct rmid_entry *entry;
> -	u32 idx, cur_idx = 1;
> +	u32 idx = 1;
>  	void *arch_mon_ctx;
>  	bool rmid_dirty;
>  	u64 val = 0;
> @@ -153,11 +153,7 @@ void __check_limbo(struct rdt_mon_domain *d, bool force_free)
>  	 * is less than the threshold decrement the busy counter of the
>  	 * RMID and move it to the free list when the counter reaches 0.
>  	 */
> -	for (;;) {
> -		idx = find_next_bit(d->rmid_busy_llc, idx_limit, cur_idx);
> -		if (idx >= idx_limit)
> -			break;
> -
> +	for_each_set_bit_from(idx, d->rmid_busy_llc, idx_limit) {
>  		entry = __rmid_entry(idx);
>  		if (resctrl_arch_rmid_read(r, d, entry->closid, entry->rmid,
>  					   QOS_L3_OCCUP_EVENT_ID, &val,
> @@ -178,11 +174,10 @@ void __check_limbo(struct rdt_mon_domain *d, bool force_free)
>  		}
>  
>  		if (force_free || !rmid_dirty) {
> -			clear_bit(idx, d->rmid_busy_llc);
> +			__clear_bit(idx, d->rmid_busy_llc);
>  			if (!--entry->busy)
>  				limbo_release_entry(entry);
>  		}
> -		cur_idx = idx + 1;
>  	}
>  
>  	resctrl_arch_mon_ctx_free(r, QOS_L3_OCCUP_EVENT_ID, arch_mon_ctx);
> -- 
> 2.43.0