linux-kernel - Re: [PATCH v5] devres: combine function devm

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: <20180123084233.GD21463@kroah.com>
Date:   Tue, 23 Jan 2018 09:42:33 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Yisheng Xie <xieyisheng1@...wei.com>
Cc:     christophe.leroy@....fr, thomas.lendacky@....com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5] devres: combine function devm_ioremap*

On Tue, Jan 16, 2018 at 08:03:41PM +0800, Yisheng Xie wrote:
> When I tried to use devm_ioremap function and review related
> code, I found devm_ioremap_* almost have the similar realize
> with each other, which can be combined.
> 
> In the former version, I have tried to kill ioremap_cache to
> reduce the size of devres, which can not work for ioremap is
> not the same as ioremap_nocache in some ARCHs likes ia64.
> Therefore, as the suggestion of Christophe, I introduce a help
> function __devm_ioremap, let devm_ioremap* inline and call
> __devm_ioremap with different devm_ioremap_type.
> 
> After apply the patch, the size of devres.o can be reduce from
> 8216 Bytes to 7352Bytes in my compile environment.
> 
> Suggested-by: Christophe LEROY <christophe.leroy@....fr>
> Signed-off-by: Yisheng Xie <xieyisheng1@...wei.com>
> ---
> v2:
>  - use MARCO for ioremap
> v3:
>  - kill dev_ioremap_nocache
> v4:
>  - combine function devm_ioremap*
> v5:
>  - fix code style.
> 
>  include/linux/io.h | 61 +++++++++++++++++++++++++++++++++++----
>  lib/devres.c       | 84 ++++++++++--------------------------------------------
>  2 files changed, 70 insertions(+), 75 deletions(-)
> 
> diff --git a/include/linux/io.h b/include/linux/io.h
> index 32e30e8..4d0a640 100644
> --- a/include/linux/io.h
> +++ b/include/linux/io.h
> @@ -73,12 +73,61 @@ static inline void devm_ioport_unmap(struct device *dev, void __iomem *addr)
>  
>  #define IOMEM_ERR_PTR(err) (__force void __iomem *)ERR_PTR(err)
>  
> -void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
> -			   resource_size_t size);
> -void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
> -				   resource_size_t size);
> -void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
> -				   resource_size_t size);
> +enum devm_ioremap_type {
> +	DEVM_IOREMAP = 0,
> +	DEVM_IOREMAP_NC,
> +	DEVM_IOREMAP_WC,
> +};

Why do these types need to be in a public .h file?

Why not just keep the .h file as-is and then just put the cleanup in the
.c file like you did?

thanks,

greg k-h