Re: [PATCH mm-unstable v2 4/5] mm: restart if multiple traversals raced

["Michal Koutný" <mkoutny@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 1052 bytes --]

On Tue, Aug 13, 2024 at 08:47:14PM GMT, Kinsey Ho <kinseyho@google.com> wrote:
> @@ -1072,21 +1073,26 @@ struct mem_cgroup *mem_cgroup_iter(struct mem_cgroup *root,
>  		 * and kicking, and don't take an extra reference.
>  		 */
>  		if (css == &root->css || css_tryget(css)) {
> -			memcg = mem_cgroup_from_css(css);
>  			break;
>  		}
>  	}
>  
> +	memcg = mem_cgroup_from_css(css);
> +
>  	if (reclaim) {
>  		/*
>  		 * The position could have already been updated by a competing
>  		 * thread, so check that the value hasn't changed since we read
>  		 * it to avoid reclaiming from the same cgroup twice.
>  		 */
> -		(void)cmpxchg(&iter->position, pos, memcg);
> +		if (cmpxchg(&iter->position, pos, memcg) != pos) {
> +			if (css && css != &root->css)
> +				css_put(css);
> +			goto restart;
> +		}

I may be missing (literal) context but I'd suggest not moving the memcg
assignment and leverage
	if (memcg != NULL)
		css_put(memcg->css)
so that the is-root comparison needn't be repeated.

Thanks,
Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]