Re: [PATCH v6 3/4] mm: memcontrol: add interfaces for swap tier selection

[Baoquan He]

On 04/21/26 at 02:53pm, Youngjun Park wrote:
...snip...
> diff --git a/mm/swap_tier.c b/mm/swap_tier.c
> index 9019b8978770..b53d6cc67d1e 100644
> --- a/mm/swap_tier.c
> +++ b/mm/swap_tier.c
...snip...
> @@ -374,6 +404,80 @@ bool swap_tiers_update(void)
>  			break;
>  		swap_tiers_assign_dev(swp);
>  	}
> +	/*
> +	 * XXX: Unused tiers default to ON, disabled after next tier added.
> +	 * Use removed tier mask to clear settings for removed/re-added tiers.
> +	 * (Could hold tier refs, but better to keep cgroup config independent)
> +	 */
> +	if (mask)
> +		swap_tier_memcg_propagate(mask);

Code is neat, and high efficiency, while it's not easy to understand. I
just sat in front of my computer the whole day yesterday to recall and
understand why it's done like this, even though I made it clear to my
self in the past. I think more words would be helpful to decrease the
difficulty for people to undersntand it. E.g below two paragraphes. Just
a suggestion and for your reference.

diff --git a/mm/swap_tier.c b/mm/swap_tier.c
index b53d6cc67d1e..0a6adf14ab91 100644
--- a/mm/swap_tier.c
+++ b/mm/swap_tier.c
@@ -405,9 +405,17 @@ bool swap_tiers_update(int mask)
 		swap_tiers_assign_dev(swp);
 	}
 	/*
-	 * XXX: Unused tiers default to ON, disabled after next tier added.
-	 * Use removed tier mask to clear settings for removed/re-added tiers.
-	 * (Could hold tier refs, but better to keep cgroup config independent)
+	 * When a tier is removed, its index (bit position in the mask) becomes
+	 * free for reassignment to a future tier.  If a memcg had previously
+	 * disabled this tier (cleared the bit in its swap.tiers file), the
+	 * effective mask would keep that bit clear -- meaning the new tier at
+	 * the same index would be silently unavailable, an invisible cgroup
+	 * constraint left behind by a tier that no longer exists.
+	 *
+	 * To prevent this, OR the removed tier's mask bit into every memcg's
+	 * tier_mask and tier_effective_mask.  This resets the bit so the new
+	 * tier is accessible by default; users who want to restrict it must
+	 * explicitly disable it after the tier is re-created.
 	 */
 	if (mask)
 		swap_tier_memcg_propagate(mask);

>  
>  	return true;
>  }
> +
...snip...