linux-kernel - Re: [lm-sensors] [PATCH 1/2] Create a DIV_ROUND_CLOSEST macro to do division with rounding

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]

Date:	Fri, 14 Nov 2008 13:46:42 -0800 (PST)
From:	Trent Piepho <tpiepho@...escale.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
cc:	Trent Piepho <tpiepho@...escale.com>, djwong@...ibm.com,
	khali@...ux-fr.org, linux-kernel@...r.kernel.org,
	lm-sensors@...sensors.org
Subject: Re: [lm-sensors] [PATCH 1/2] Create a DIV_ROUND_CLOSEST macro to do
 division with rounding

On Tue, 11 Nov 2008, Andrew Morton wrote:
>>>>> +#define DIV_ROUND_CLOSEST(x, divisor)(			\
>>>>> +{							\
>>>>> +	typeof(divisor) __divisor = divisor;		\
>>>>> +	(((x) + ((__divisor) / 2)) / (__divisor));	\
>>>>> +}							\
>>>>> +)
>>>>
>>>> Maybe you can do away with the statement-expression extension?  I've seen
>>>> cases where it cases gcc to generate worse code.  It seems like it
>>>> shouldn't, but it does.  I know DIV_ROUND_CLOSEST (maybe DIV_ROUND_NEAR?)
>>>> uses divisor twice, but all the also divide macros do that too, so why does
>>>> this one need to be different?
>>>
>>> The others need fixing too.
>>
>> Is it worth generating worse code for these simple macros?
>
> Well that's an interesting question.
>
> The risks with the current code are
>
> a) It will introduce straightforward bugs, where pointers are
>   incremented twice, etc.
>
>   Hopefully these things will be apparent during testing and we'll
>   fix them up in the usual fashion.
>
> b) It will introduce subtle slowdowns due to needlessly executing
>   code more than once, as in the hugepage case which I identified.
>   These problems will hang around for long periods.
>
> So they're good reasons to fix the macros.  If these fixes cause the
> compiler to generate worse code then we should quantify and understand
> that.  Perhaps it is only certain compiler versions.  Perhaps we can
> find a test case (should be easy?) and send it over to the gcc guys to
> fix.  Perhaps we can find some C-level construct which prevents the
> compiler from going into stupid mode without reintroducing the existing
> problems.

My question was more along the lines of is it worth it to even have macros for
something as simple rounding up when dividing?

For an example of statement expression problems, I noticed something with
swab16(), addressed in commit 8e2c20023f34b652605a5fb7c68bb843d2b100a8

#define ___swab16(x) \
({ \
        __u16 __x = (x); \
        ((__u16)( \
                (((__u16)(__x) & (__u16)0x00ffU) << 8) | \
                (((__u16)(__x) & (__u16)0xff00U) >> 8) )); \
})

Produces this code:

             movzwl  %ax, %eax
             movl    %eax, %edx
             shrl    $8, %eax
             sall    $8, %edx
             orl     %eax, %edx

While this:

static __inline__ __attribute_const__ __u16 ___swab16(__u16 x)
{
        return x<<8 | x>>8;
}

Produces this code:

             rolw    $8, %ax

--
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/