Re: [PATCH v5 04/11] drm/panfrost: Move shrinker initialization and unplug one level down

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Removing the `IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE)` check causes a build failure (undefined reference to `panfrost_transparent_hugepage`) when `CONFIG_TRANSPARENT_HUGEPAGE` is disabled.
--

--- Patch [4]: [PATCH v5 04/11] drm/panfrost: Move shrinker initialization and unplug one level down ---
commit a1e5fed49d6d030d377938e3c3e5a1b2314d6290
Author: Adrián Larumbe <[email protected]>

drm/panfrost: Move shrinker initialization and unplug one level down

Since the moment we call drm_dev_register() the device should be in a
position to accept jobs, so it's best if the shrinker is already
initialized by then.

> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
> index 3a7fce4288987..7b993a089af2d 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gem.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
> @@ -15,20 +15,25 @@
>  #include "panfrost_gem.h"
>  #include "panfrost_mmu.h"
>  
> -void panfrost_gem_init(struct panfrost_device *pfdev)
> +int panfrost_gem_init(struct panfrost_device *pfdev)
>  {
>  	int err;
>  
> -	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
> -	    !panfrost_transparent_hugepage)
> -		return;
> +	if (panfrost_transparent_hugepage) {
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Low]
Will this cause a build failure when CONFIG_TRANSPARENT_HUGEPAGE is disabled?

Looking at drivers/gpu/drm/panfrost/panfrost_drv.c,
panfrost_transparent_hugepage is defined under an #ifdef:

#ifdef CONFIG_TRANSPARENT_HUGEPAGE
bool panfrost_transparent_hugepage = true;
module_param_named(transparent_hugepage, panfrost_transparent_hugepage, bool, 0400);
MODULE_PARM_DESC(transparent_hugepage, "Use a dedicated tmpfs mount point with Transparent Hugepage enabled (true = default)");
#endif

Without the IS_ENABLED() check, evaluating panfrost_transparent_hugepage here
could result in an undefined reference error during linking if the config is
disabled.

> +		err = drm_gem_huge_mnt_create(&pfdev->base, "within_size");
> +		if (drm_gem_get_huge_mnt(&pfdev->base))
> +			drm_info(&pfdev->base, "Using Transparent Hugepage\n");
> +		else if (err)
> +			drm_warn(&pfdev->base,
> +				 "Can't use Transparent Hugepage (%d)\n", err);
> +	}
>  
> -	err = drm_gem_huge_mnt_create(&pfdev->base, "within_size");
> -	if (drm_gem_get_huge_mnt(&pfdev->base))
> -		drm_info(&pfdev->base, "Using Transparent Hugepage\n");
> -	else if (err)
> -		drm_warn(&pfdev->base, "Can't use Transparent Hugepage (%d)\n",
> -			 err);
> +	return panfrost_gem_shrinker_init(pfdev);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4