linux-kernel - Re: [PATCH v18 04/26] drm/shmem-helper: Refactor locked/unlocked functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231124114436.7c8ef723@collabora.com>
Date:   Fri, 24 Nov 2023 11:44:36 +0100
From:   Boris Brezillon <boris.brezillon@...labora.com>
To:     Maxime Ripard <mripard@...nel.org>
Cc:     Dmitry Osipenko <dmitry.osipenko@...labora.com>,
        David Airlie <airlied@...il.com>,
        Gerd Hoffmann <kraxel@...hat.com>,
        Gurchetan Singh <gurchetansingh@...omium.org>,
        Chia-I Wu <olvaffe@...il.com>, Daniel Vetter <daniel@...ll.ch>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Christian König <christian.koenig@....com>,
        Qiang Yu <yuq825@...il.com>,
        Steven Price <steven.price@....com>,
        Emma Anholt <emma@...olt.net>, Melissa Wen <mwen@...lia.com>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        kernel@...labora.com, virtualization@...ts.linux-foundation.org
Subject: Re: [PATCH v18 04/26] drm/shmem-helper: Refactor locked/unlocked
 functions

On Fri, 24 Nov 2023 11:40:06 +0100
Maxime Ripard <mripard@...nel.org> wrote:

> On Mon, Oct 30, 2023 at 02:01:43AM +0300, Dmitry Osipenko wrote:
> > Add locked and remove unlocked postfixes from drm-shmem function names,
> > making names consistent with the drm/gem core code.
> > 
> > Reviewed-by: Boris Brezillon <boris.brezillon@...labora.com>
> > Suggested-by: Boris Brezillon <boris.brezillon@...labora.com>
> > Signed-off-by: Dmitry Osipenko <dmitry.osipenko@...labora.com>  
> 
> This contradicts my earlier ack on a patch but...
> 
> > ---
> >  drivers/gpu/drm/drm_gem_shmem_helper.c        | 64 +++++++++----------
> >  drivers/gpu/drm/lima/lima_gem.c               |  8 +--
> >  drivers/gpu/drm/panfrost/panfrost_drv.c       |  2 +-
> >  drivers/gpu/drm/panfrost/panfrost_gem.c       |  6 +-
> >  .../gpu/drm/panfrost/panfrost_gem_shrinker.c  |  2 +-
> >  drivers/gpu/drm/panfrost/panfrost_mmu.c       |  2 +-
> >  drivers/gpu/drm/v3d/v3d_bo.c                  |  4 +-
> >  drivers/gpu/drm/virtio/virtgpu_object.c       |  4 +-
> >  include/drm/drm_gem_shmem_helper.h            | 36 +++++------
> >  9 files changed, 64 insertions(+), 64 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > index 0d61f2b3e213..154585ddae08 100644
> > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > @@ -43,8 +43,8 @@ static const struct drm_gem_object_funcs drm_gem_shmem_funcs = {
> >  	.pin = drm_gem_shmem_object_pin,
> >  	.unpin = drm_gem_shmem_object_unpin,
> >  	.get_sg_table = drm_gem_shmem_object_get_sg_table,
> > -	.vmap = drm_gem_shmem_object_vmap,
> > -	.vunmap = drm_gem_shmem_object_vunmap,
> > +	.vmap = drm_gem_shmem_object_vmap_locked,
> > +	.vunmap = drm_gem_shmem_object_vunmap_locked,  
> 
> While I think we should indeed be consistent with the names, I would
> also expect helpers to get the locking right by default.
> 
> I'm not sure how reasonable it is, but I think I'd prefer to turn this
> around and keep the drm_gem_shmem_object_vmap/unmap helpers name, and
> convert whatever function needs to be converted to the unlock suffix so
> we get a consistent naming.
> 
> Does that make sense?

I don't mind, as long as it's consistent, it's just that that there's
probably more to patch if we do it the other way around.