Re: [PATCH v4 6/6] drm/shmem-helper: Switch to reservation lock

From: Dmitry Osipenko <dmitry.osipenko@collabora.com>
To: Boris Brezillon <boris.brezillon@collabora.com>
Cc: "Sumit Semwal" <sumit.semwal@linaro.org>,
	"Christian König" <christian.koenig@amd.com>,
	"Benjamin Gaignard" <benjamin.gaignard@collabora.com>,
	"Brian Starkey" <Brian.Starkey@arm.com>,
	"John Stultz" <jstultz@google.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Jani Nikula" <jani.nikula@linux.intel.com>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"Tomi Valkeinen" <tomba@kernel.org>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Tomasz Figa" <tfiga@chromium.org>,
	"Marek Szyprowski" <m.szyprowski@samsung.com>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Emil Velikov" <emil.l.velikov@gmail.com>,
	intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org, linux-tegra@vger.kernel.org,
	kernel@collabora.com, linux-media@vger.kernel.org
Subject: Re: [PATCH v4 6/6] drm/shmem-helper: Switch to reservation lock
Date: Mon, 26 Jun 2023 16:05:14 +0300	[thread overview]
Message-ID: <2b932742-0637-3b7a-acb5-ab643781caa3@collabora.com> (raw)
In-Reply-To: <20230626114014.2c837255@collabora.com>

On 6/26/23 12:40, Boris Brezillon wrote:
> Same problem with this renaming: it's confusing because this function
> was previously taking care of the locking, and it's no longer the case.
> That's actually true for other public functions your patching, but I
> won't go over all of them.
> 
> I know this patch has been under discussion for quite some time, and has
> been validated by other devs/maintainers, but I'd like to understand the
> reasoning behind these decisions. Not the decision to replace all locks
> by dma_resv, which I kinda understand, but the decision to change the
> behavior of functions without making the name reflect the new behavior
> (_locked prefix), or the fact we now prohibit some functions to
> succeed when the dma_resv lock is taken by the driver beforehand (which,
> unless I'm mistaken, will happen in the VM_BIND logic, and can happen
> in the SUBMIT ioctl too depending on the driver).

Adding explicit _locked/unlocked postfix to all function names indeed
won't hurt to do. There was no decision made about the function names,
the old functions kept the old name where possible to minimize code
changes during transition to the resv lock. Improving the names could be
the next step.

Thanks for the feedback!

-- 
Best regards,
Dmitry