On 29. 01. 25 07:43, Sergey Senozhatsky wrote:
+/*
+ * zspage lock permits preemption on the reader-side (there can be multiple
+ * readers). Writers (exclusive zspage ownership), on the other hand, are
+ * always run in atomic context and cannot spin waiting for a (potentially
+ * preempted) reader to unlock zspage. This, basically, means that writers
+ * can only call write-try-lock and must bail out if it didn't succeed.
+ *
+ * At the same time, writers cannot reschedule under zspage write-lock,
+ * so readers can spin waiting for the writer to unlock zspage.
+ */
+static void zspage_read_lock(struct zspage *zspage)
+{
+ atomic_t *lock = &zspage->lock;
+ int old;
+
+ while (1) {
+ old = atomic_read(lock);
+ if (old == ZS_PAGE_WRLOCKED) {
+ cpu_relax();
+ continue;
+ }
+
+ if (atomic_try_cmpxchg(lock, &old, old + 1))
+ return;
+
+ cpu_relax();
+ }
+}
Please note that atomic_try_cmpxchg updates old variable on failure, so
the whole loop can be rewritten as:
{
atomic_t *lock = &zspage->lock;
int old = atomic_read(lock);
while (1) {
if (old == ZS_PAGE_WRLOCKED) {
cpu_relax();
old = atomic_read(lock);
continue;
}
if (atomic_try_cmpxchg(lock, &old, old + 1))
return;
cpu_relax();
}
}
Please note that cpu_relax() in the cmpxchg() loop is actually harmful
[1] because:
--q--
On the x86-64 architecture even a failing cmpxchg grants exclusive
access to the cacheline, making it preferable to retry the failed op
immediately instead of stalling with the pause instruction.
--/q--
[1]
https://lore.kernel.org/all/20230113184447.1707316-1-mjguzik@xxxxxxxxx/
Based on the above, cpu_relax() should be removed from the loop, which
becomes:
{
atomic_t *lock = &zspage->lock;
int old = atomic_read(lock);
do {
if (old == ZS_PAGE_WRLOCKED) {
cpu_relax();
old = atomic_read(lock);
continue;
}
} while (!atomic_try_cmpxchg(lock, &old, old + 1));
}
+static int zspage_try_write_lock(struct zspage *zspage)
This function can be declared as bool, returning true/false.
Uros.
