linux-kernel - Re: [PATCH v2 1/2] drm: Add DRM_GEM

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 for Android: free password hash cracker in your pocket

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

Message-ID: <26a6ed05-264a-eef6-a0ee-527d6968f8a8@suse.de>
Date:   Tue, 7 Jun 2022 19:22:38 +0200
From:   Thomas Zimmermann <tzimmermann@...e.de>
To:     Rob Clark <robdclark@...il.com>
Cc:     Rob Clark <robdclark@...omium.org>,
        Tvrtko Ursulin <tvrtko.ursulin@...ux.intel.com>,
        David Airlie <airlied@...ux.ie>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        freedreno <freedreno@...ts.freedesktop.org>
Subject: Re: [PATCH v2 1/2] drm: Add DRM_GEM_FOPS

Hi

Am 07.06.22 um 16:58 schrieb Rob Clark:
> On Mon, Jun 6, 2022 at 11:56 PM Thomas Zimmermann <tzimmermann@...e.de> wrote:
>>
>> Hi
>>
>> Am 06.06.22 um 21:54 schrieb Rob Clark:
>>> From: Rob Clark <robdclark@...omium.org>
>>>
>>> The DEFINE_DRM_GEM_FOPS() helper is a bit limiting if a driver wants to
>>> provide additional file ops, like show_fdinfo().
>>>
>>> Signed-off-by: Rob Clark <robdclark@...omium.org>
>>> ---
>>>    include/drm/drm_gem.h | 26 ++++++++++++++++++--------
>>>    1 file changed, 18 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
>>> index 9d7c61a122dc..dc88d4a2cdf6 100644
>>> --- a/include/drm/drm_gem.h
>>> +++ b/include/drm/drm_gem.h
>>> @@ -314,6 +314,23 @@ struct drm_gem_object {
>>>        const struct drm_gem_object_funcs *funcs;
>>>    };
>>>
>>> +/**
>>> + * DRM_GEM_FOPS - Default drm GEM file operations
>>> + *
>>> + * This macro provides a shorthand for setting the GEM file ops in the
>>> + * &file_operations structure.
>>
>> I would appreciate a reference to DEFINE_DRM_GEM_FOPS. Something along
>> the lines of 'if all you need are the default ops, use DEFINE_DRM_GEM_FOPS'.
>>
>>> + */
>>> +#define DRM_GEM_FOPS \
>>> +     .open           = drm_open,\
>>> +     .release        = drm_release,\
>>> +     .unlocked_ioctl = drm_ioctl,\
>>> +     .compat_ioctl   = drm_compat_ioctl,\
>>> +     .poll           = drm_poll,\
>>> +     .read           = drm_read,\
>>> +     .llseek         = noop_llseek,\
>>> +     .mmap           = drm_gem_mmap
>>> +
>>> +
>>
>> Only one empty line please.
>>
>>>    /**
>>>     * DEFINE_DRM_GEM_FOPS() - macro to generate file operations for GEM drivers
>>>     * @name: name for the generated structure
>>> @@ -330,14 +347,7 @@ struct drm_gem_object {
>>>    #define DEFINE_DRM_GEM_FOPS(name) \
>>>        static const struct file_operations name = {\
>>>                .owner          = THIS_MODULE,\
>>
>> Is there a specific reason why .owner is still set here? I suspect that
>> DRM_GEM_FOPS is strictly for callback functions?
> 
> I was on the fence about that one, but it seemed better to not mix
> "magic" and the callbacks.. but I could be convinced in either
> direction

I think you made the right choice. It's cleaner and most drivers will 
want to use DEFINE_DRM_GEM_FOPS, which includes the magic.

Best regards
Thomas

> 
>> In any case
>>
>> Acked-by: Thomas Zimmermann <tzimmermann@...e.de>
> 
> thx, I'll fixup the other nits in v3.
> 
>>
>> Best regards
>> Thomas
>>
>>> -             .open           = drm_open,\
>>> -             .release        = drm_release,\
>>> -             .unlocked_ioctl = drm_ioctl,\
>>> -             .compat_ioctl   = drm_compat_ioctl,\
>>> -             .poll           = drm_poll,\
>>> -             .read           = drm_read,\
>>> -             .llseek         = noop_llseek,\
>>> -             .mmap           = drm_gem_mmap,\
>>> +             DRM_GEM_FOPS,\
>>>        }
>>>
>>>    void drm_gem_object_release(struct drm_gem_object *obj);
>>
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Ivo Totev

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

Download attachment "OpenPGP_signature" of type "application/pgp-signature" (841 bytes)