Hi Jan,
On 09/06/2021 12:20, Jan Beulich wrote:
On 08.06.2021 12:08, Julien Grall wrote:
From: Julien Grall <jgr...@amazon.com>
Since XSA-228 (commit 02cbeeb62075 "gnttab: split maptrack lock
to make it fulfill its purpose again"), v->maptrack_head,
v->maptrack_tail and the content of the freelist are accessed with
the lock v->maptrack_freelist_lock held.
Therefore it is not necessary to update the fields using cmpxchg()
and also read them atomically.
Note that there are two cases where v->maptrack_tail is accessed without
the lock. They both happen in get_maptrack_handle() when initializing
the free list of the current vCPU. Therefore there is no possible race.
The code is now reworked to remove any use of cmpxch() and read_atomic()
when accessing the fields v->maptrack_{head, tail} as wel as the
freelist.
Take the opportunity to add a comment on top of the lock definition
and explain what it protects.
Signed-off-by: Julien Grall <jgr...@amazon.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
Thanks. I committed with...
with one nit:
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -255,7 +255,13 @@ struct vcpu
/* VCPU paused by system controller. */
int controller_pause_count;
- /* Grant table map tracking. */
+ /*
+ * Grant table map tracking. The lock maptrack_freelist_lock
+ * protects to:
I don't think you want "to" here.
... this addressed and ...
Jan
+ * - The entries in the freelist
... "The" removed.
+ * - maptrack_head
+ * - maptrack_tail
+ */
spinlock_t maptrack_freelist_lock;
unsigned int maptrack_head;
unsigned int maptrack_tail;
Cheers,
--
Julien Grall