Re: [PATCH v6 8/8] nouveau/svm: Implement atomic SVM access

On Monday, 15 March 2021 6:51:13 PM AEDT Christoph Hellwig wrote:

quoted

-	/*XXX: atomic? */
-	return (fa->access == 0 || fa->access == 3) -
-	       (fb->access == 0 || fb->access == 3);
+	/* Atomic access (2) has highest priority */
+	return (-1*(fa->access == 2) + (fa->access == 0 || fa->access == 3)) -
+	       (-1*(fb->access == 2) + (fb->access == 0 || fb->access == 3));

This looks really unreabable.  If the magic values 0, 2 and 3 had names
it might become a little more understadable, then factor the duplicated
calculation of the priority value into a helper and we'll have code that
mere humans can understand..

Fair enough, will add some definitions for the magic values.

quoted

+		mutex_lock(&svmm->mutex);
+		if (mmu_interval_read_retry(&notifier->notifier,
+					    notifier_seq)) {
+			mutex_unlock(&svmm->mutex);
+			continue;
+		}
+		break;
+	}

This looks good, why not:

		mutex_lock(&svmm->mutex);
		if (!mmu_interval_read_retry(&notifier->notifier,
					     notifier_seq))
			break;
		mutex_unlock(&svmm->mutex);
	}

I had copied that from nouveau_range_fault() but this suggestion is better. 
Will update, thanks for looking.