[PATCH] arm64: mm: Optimize querying asid from reserved_asids

[PATCH] arm64: mm: Optimize querying asid from reserved_asids

[Guo Hui]

Move reserved_asids updates into function flush_context. When
asid_generation increases, reserved_asids are updated synchronously.
The execution frequency of function flush_context is far less than
that of function check_update_reserved_asid. In function
check_update_reserved_asid, you only need to query whether it is
in reserved_asids based on the new newasid, and there is no need
to update it.

In the function check_update_reserved_asid, among all the times
reserved_asids are hit, the probability that newasid is equal to
the reserved_asids of the current CPU is about greater than 70%.

Signed-off-by: Guo Hui <guohui@uniontech.com>
---
 arch/arm64/mm/context.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
index 188197590fc9..76e0beb14466 100644
--- a/arch/arm64/mm/context.c
+++ b/arch/arm64/mm/context.c
@@ -101,7 +101,7 @@ static void set_reserved_asid_bits(void)
 #define asid_gen_match(asid) \
 	(!(((asid) ^ atomic64_read(&asid_generation)) >> asid_bits))
 
-static void flush_context(void)
+static void flush_context(u64 generation)
 {
 	int i;
 	u64 asid;
@@ -120,6 +120,8 @@ static void flush_context(void)
 		 */
 		if (asid == 0)
 			asid = per_cpu(reserved_asids, i);
+
+		asid = generation | (asid & ~ASID_MASK);
 		__set_bit(ctxid2asid(asid), asid_map);
 		per_cpu(reserved_asids, i) = asid;
 	}
@@ -131,24 +133,21 @@ static void flush_context(void)
 	cpumask_setall(&tlb_flush_pending);
 }
 
-static bool check_update_reserved_asid(u64 asid, u64 newasid)
+static bool check_update_reserved_asid(u64 newasid)
 {
-	int cpu;
+	int cpu, cur_cpu = smp_processor_id();
 	bool hit = false;
 
-	/*
-	 * Iterate over the set of reserved ASIDs looking for a match.
-	 * If we find one, then we can update our mm to use newasid
-	 * (i.e. the same ASID in the current generation) but we can't
-	 * exit the loop early, since we need to ensure that all copies
-	 * of the old ASID are updated to reflect the mm. Failure to do
-	 * so could result in us missing the reserved ASID in a future
-	 * generation.
-	 */
+	if (per_cpu(reserved_asids, cur_cpu) == newasid)
+		return true;
+
 	for_each_possible_cpu(cpu) {
-		if (per_cpu(reserved_asids, cpu) == asid) {
+		if (cpu == cur_cpu)
+			continue;
+
+		if (per_cpu(reserved_asids, cpu) == newasid) {
 			hit = true;
-			per_cpu(reserved_asids, cpu) = newasid;
+			break;
 		}
 	}
 
@@ -168,7 +167,7 @@ static u64 new_context(struct mm_struct *mm)
 		 * If our current ASID was active during a rollover, we
 		 * can continue to use it and this was just a false alarm.
 		 */
-		if (check_update_reserved_asid(asid, newasid))
+		if (check_update_reserved_asid(newasid))
 			return newasid;
 
 		/*
@@ -201,7 +200,7 @@ static u64 new_context(struct mm_struct *mm)
 	/* We're out of ASIDs, so increment the global generation count */
 	generation = atomic64_add_return_relaxed(ASID_FIRST_VERSION,
 						 &asid_generation);
-	flush_context();
+	flush_context(generation);
 
 	/* We have more ASIDs than CPUs, so this will always succeed */
 	asid = find_next_zero_bit(asid_map, NUM_USER_ASIDS, 1);
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: mm: Optimize querying asid from reserved_asids

[Will Deacon]

On Wed, Sep 27, 2023 at 01:51:06PM +0800, Guo Hui wrote:
> Move reserved_asids updates into function flush_context. When
> asid_generation increases, reserved_asids are updated synchronously.
> The execution frequency of function flush_context is far less than
> that of function check_update_reserved_asid. In function
> check_update_reserved_asid, you only need to query whether it is
> in reserved_asids based on the new newasid, and there is no need
> to update it.
> 
> In the function check_update_reserved_asid, among all the times
> reserved_asids are hit, the probability that newasid is equal to
> the reserved_asids of the current CPU is about greater than 70%.

Do you have any performance numbers to justify this change?

> diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
> index 188197590fc9..76e0beb14466 100644
> --- a/arch/arm64/mm/context.c
> +++ b/arch/arm64/mm/context.c
> @@ -101,7 +101,7 @@ static void set_reserved_asid_bits(void)
>  #define asid_gen_match(asid) \
>  	(!(((asid) ^ atomic64_read(&asid_generation)) >> asid_bits))
>  
> -static void flush_context(void)
> +static void flush_context(u64 generation)
>  {
>  	int i;
>  	u64 asid;
> @@ -120,6 +120,8 @@ static void flush_context(void)
>  		 */
>  		if (asid == 0)
>  			asid = per_cpu(reserved_asids, i);
> +
> +		asid = generation | (asid & ~ASID_MASK);

I'm struggling to see how this is safe with multiple rollovers. For
example, if we have the following sequence of events on one CPU:

1. Task A runs with ASID 42
2. Context switch to Task C with ASID 66
3. A rollover occurs
4. Task B runs for the first time; gets allocated ASID 42
5. Another rollover occurs
6. Task A runs

Won't task A get ASID 42, even though that's reserved for task B?

But this code is subtle, so maybe I missed something.

Also, if we change this, then you should update the TLA+ model over at

https://git.kernel.org/pub/scm/linux/kernel/git/cmarinas/kernel-tla.git/tree/asidalloc.tla

Cheers,

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel