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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 19 Mar 2020 15:27:29 +0300 (EAT) From: Wambui Karuga <wambui.karugax@...il.com> To: Daniel Vetter <daniel@...ll.ch>, Greg KH <gregkh@...uxfoundation.org>, Wambui Karuga <wambui.karugax@...il.com>, Dave Airlie <airlied@...ux.ie>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de> cc: dri-devel <dri-devel@...ts.freedesktop.org>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org> Subject: Re: [PATCH v2 10/17] drm/vram-helper: make drm_vram_mm_debugfs_init() return 0 On Thu, 19 Mar 2020, Daniel Vetter wrote: > On Thu, Mar 19, 2020 at 08:55:24AM +0100, Greg KH wrote: >> On Wed, Mar 18, 2020 at 08:10:43PM +0100, Daniel Vetter wrote: >>> On Wed, Mar 18, 2020 at 5:58 PM Greg KH <gregkh@...uxfoundation.org> wrote: >>>> >>>> On Wed, Mar 18, 2020 at 05:31:47PM +0100, Daniel Vetter wrote: >>>>> On Wed, Mar 18, 2020 at 5:03 PM Wambui Karuga <wambui.karugax@...il.com> wrote: >>>>>> >>>>>> >>>>>> >>>>>> On Wed, 18 Mar 2020, Daniel Vetter wrote: >>>>>> >>>>>>> On Tue, Mar 10, 2020 at 04:31:14PM +0300, Wambui Karuga wrote: >>>>>>>> Since 987d65d01356 (drm: debugfs: make >>>>>>>> drm_debugfs_create_files() never fail), drm_debugfs_create_files() never >>>>>>>> fails and should return void. Therefore, remove its use as the >>>>>>>> return value of drm_vram_mm_debugfs_init(), and have the function >>>>>>>> return 0 directly. >>>>>>>> >>>>>>>> v2: have drm_vram_mm_debugfs_init() return 0 instead of void to avoid >>>>>>>> introducing build issues and build breakage. >>>>>>>> >>>>>>>> References: https://lists.freedesktop.org/archives/dri-devel/2020-February/257183.html >>>>>>>> Signed-off-by: Wambui Karuga <wambui.karugax@...il.com> >>>>>>>> Acked-by: Thomas Zimmermann <tzimmermann@...e.de> >>>>>>>> --- >>>>>>>> drivers/gpu/drm/drm_gem_vram_helper.c | 10 ++++------ >>>>>>>> 1 file changed, 4 insertions(+), 6 deletions(-) >>>>>>>> >>>>>>>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c >>>>>>>> index 92a11bb42365..c8bcc8609650 100644 >>>>>>>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c >>>>>>>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c >>>>>>>> @@ -1048,14 +1048,12 @@ static const struct drm_info_list drm_vram_mm_debugfs_list[] = { >>>>>>>> */ >>>>>>>> int drm_vram_mm_debugfs_init(struct drm_minor *minor) >>>>>>>> { >>>>>>>> - int ret = 0; >>>>>>>> - >>>>>>>> #if defined(CONFIG_DEBUG_FS) >>>>>>> >>>>>>> Just noticed that this #if here is not needed, we already have a dummy >>>>>>> function for that case. Care to write a quick patch to remove it? On top >>>>>>> of this patch series here ofc, I'm in the processing of merging the entire >>>>>>> pile. >>>>>>> >>>>>>> Thanks, Daniel >>>>>> Hi Daniel, >>>>>> Without this check here, and compiling without CONFIG_DEBUG_FS, this >>>>>> function is run and the drm_debugfs_create_files() does not have access to >>>>>> the parameters also protected by an #if above this function. So the change >>>>>> throws an error for me. Is that correct? >>>>> >>>>> Hm right. Other drivers don't #ifdef out their debugfs file functions >>>>> ... kinda a bit disappointing that we can't do this in the neatest way >>>>> possible. >>>>> >>>>> Greg, has anyone ever suggested to convert the debugfs_create_file >>>>> function (and similar things) to macros that don't use any of the >>>>> arguments, and then also annotating all the static functions/tables as >>>>> __maybe_unused and let the compiler garbage collect everything? >>>>> Instead of explicit #ifdef in all the drivers ... >>>> >>>> No, no one has suggested that, having the functions be static inline >>>> should make it all "just work" properly if debugfs is not enabled. The >>>> variables will not be used, so the compiler should just optimize them >>>> away properly. >>>> >>>> No checks for CONFIG_DEBUG_FS should be needed anywhere in .c code. >>> >>> So the trouble with this one is that the static inline functions for >>> the debugfs file are wrapped in a #if too, and hence if we drop the >>> #if around the function call stuff won't compile. Should we drop all >>> the #if in the .c file and assume the compiler will remove all the >>> dead code and dead functions? >> >> Yes you should :) >> >> there should not be any need for #if in a .c file for debugfs stuff. > > Wambui, can you pls try that out? I.e. removing all the #if for > CONFIG_DEBUG_FS from that file. Removing them works with CONFIG_DEBUG_FS enabled or disabled. I can send a patch for that. wambui karuga > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch >
Powered by blists - more mailing lists