From mboxrd@z Thu Jan  1 00:00:00 1970
From: Christoph Hellwig <hch@infradead.org>
Date: Mon, 15 Mar 2021 07:51:13 +0000
Subject: Re: [PATCH v6 8/8] nouveau/svm: Implement atomic SVM access
Message-Id: <20210315075113.GD4136862@infradead.org>
List-Id: <kvm-ppc.vger.kernel.org>
References: <20210312083851.15981-1-apopple@nvidia.com>
 <20210312083851.15981-9-apopple@nvidia.com>
In-Reply-To: <20210312083851.15981-9-apopple@nvidia.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: Alistair Popple <apopple@nvidia.com>
Cc: linux-mm@kvack.org, nouveau@lists.freedesktop.org, bskeggs@redhat.com, akpm@linux-foundation.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, kvm-ppc@vger.kernel.org, dri-devel@lists.freedesktop.org, jhubbard@nvidia.com, rcampbell@nvidia.com, jglisse@redhat.com, jgg@nvidia.com, hch@infradead.org, daniel@ffwll.ch, willy@infradead.org

> -	/*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..

> +		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);
	}