linux-kernel - Re: [RFC][PATCH] introduce ptr

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: <1282219058.10440.26.camel@thorin>
Date:	Thu, 19 Aug 2010 13:57:38 +0200
From:	Bernd Petrovitsch <bernd@...prog.at>
To:	Namhyung Kim <namhyung@...il.com>
Cc:	Arnd Bergmann <arnd@...db.de>, linux-arch@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC][PATCH] introduce ptr_diff()

On Don, 2010-08-19 at 20:37 +0900, Namhyung Kim wrote:
> When I compiled allyesconfig'ed kernel with C=1, I got 1519 lines of
> following message:
> 
>  include/linux/mm.h:599:16: warning: potentially expensive pointer subtraction
> 
> which is around 10% of total warnings. this was caused by page_to_pfn() macro
> so I think it's worth to remove it by calculating pointer subtraction
> manually.
> 
> ptr_diff() does subtraction of two pointers as if they were integer values
> and divides it by the size of their base type. Currently it does not deal
> with a difference alignment requirement than the size of base type, it
> would be a trivial job.
> 
> Impact: remove a _lot_ of sparse warnings
> Signed-off-by: Namhyung Kim <namhyung@...il.com>
> ---
> 
> I have no idea where the right place is.
> Please kindly point me to the place if here is not the one.
> And any comments are greatly welcomed also.
> 
> Thanks.
> 
>  include/asm-generic/memory_model.h |   23 +++++++++++++++++++----
>  1 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/include/asm-generic/memory_model.h b/include/asm-generic/memory_model.h
> index fb2d63f..ffec625 100644
> --- a/include/asm-generic/memory_model.h
> +++ b/include/asm-generic/memory_model.h
> @@ -22,13 +22,28 @@
>  
>  #endif /* CONFIG_DISCONTIGMEM */
>  
> +#include <linux/log2.h>
> +
> +#define ptr_diff(p1, p2)					\
> +({	long __diff;						\
> +	unsigned long  __addr1 = (unsigned long) (p1);		\
> +	unsigned long  __addr2 = (unsigned long) (p2);		\
> +	unsigned __size = sizeof(typeof(*p1));			\
The typeof() is actually redundant here.
> +								\
> +	if (is_power_of_2(__size))				\
> +		__diff = (__addr1 - __addr2) >> ilog2(__size);	\
> +	else							\
> +		__diff = (__addr1 - __addr2) / __size;		\
> +	__diff;							\
> +})
> +

IMHO this kills gcc's type compatibility checks on p1 and p2 (even if
they are suboptimal).
There is the "__same_type" macro in compiler.h for this or the "else"
branch uses plain C
	__diff = p1 - p2;
to keep them (which probably also keeps sparse's warnings).

Actually one would assume that gcc internally just does the above as the
size of the type is statically known (and sparse could be improved to
not warn where sizeof(*p1) is a power of 2).

	Bernd
-- 
mobile: +43 664 4416156              http://www.sysprog.at/
    Linux Software Development, Consulting and Services

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/