On June 28, 2017 7:12:04 PM PDT, zhong jiang <zhongjiang@xxxxxxxxxx> wrote:
>On 2017/6/29 5:43, hpa@xxxxxxxxx wrote:
>> On June 27, 2017 9:35:10 PM PDT, zhong jiang <zhongjiang@xxxxxxxxxx>
>wrote:
>>> Hi, Ingo
>>>
>>> Thank you for the comment.
>>> On 2017/6/22 0:40, Ingo Molnar wrote:
>>>> * zhong jiang <zhongjiang@xxxxxxxxxx> wrote:
>>>>
>>>>> when shift expoment is negative, left shift alway zero. therefore,
>>> we
>>>>> modify the logic to avoid the warining.
>>>>>
>>>>> Signed-off-by: zhong jiang <zhongjiang@xxxxxxxxxx>
>>>>> ---
>>>>> arch/x86/include/asm/futex.h | 8 ++++++--
>>>>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/arch/x86/include/asm/futex.h
>>> b/arch/x86/include/asm/futex.h
>>>>> index b4c1f54..2425fca 100644
>>>>> --- a/arch/x86/include/asm/futex.h
>>>>> +++ b/arch/x86/include/asm/futex.h
>>>>> @@ -49,8 +49,12 @@ static inline int futex_atomic_op_inuser(int
>>> encoded_op, u32 __user *uaddr)
>>>>> int cmparg = (encoded_op << 20) >> 20;
>>>>> int oldval = 0, ret, tem;
>>>>>
>>>>> - if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
>>>>> - oparg = 1 << oparg;
>>>>> + if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28)) {
>>>>> + if (oparg >= 0)
>>>>> + oparg = 1 << oparg;
>>>>> + else
>>>>> + oparg = 0;
>>>>> + }
>>>> Could we avoid all these complications by using an unsigned type?
>>> I think it is not feasible. a negative shift exponent is likely
>>> existence and reasonable.
>>> as the above case, oparg is a negative is common.
>>>
>>> I think it can be avoided by following change.
>>>
>>> diff --git a/arch/x86/include/asm/futex.h
>>> b/arch/x86/include/asm/futex.h
>>> index b4c1f54..3205e86 100644
>>> --- a/arch/x86/include/asm/futex.h
>>> +++ b/arch/x86/include/asm/futex.h
>>> @@ -50,7 +50,7 @@ static inline int futex_atomic_op_inuser(int
>>> encoded_op, u32 __user *uaddr)
>>> int oldval = 0, ret, tem;
>>>
>>> if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
>>> - oparg = 1 << oparg;
>>> + oparg = safe_shift(1, oparg);
>>>
>>> if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
>>> return -EFAULT;
>>> diff --git a/drivers/video/fbdev/core/fbmem.c
>>> b/drivers/video/fbdev/core/fbmem.c
>>> index 069fe79..b4edda3 100644
>>> --- a/drivers/video/fbdev/core/fbmem.c
>>> +++ b/drivers/video/fbdev/core/fbmem.c
>>> @@ -190,11 +190,6 @@ char* fb_get_buffer_offset(struct fb_info
>*info,
>>> struct fb_pixmap *buf, u32 size
>>>
>>> #ifdef CONFIG_LOGO
>>>
>>> -static inline unsigned safe_shift(unsigned d, int n)
>>> -{
>>> - return n < 0 ? d >> -n : d << n;
>>> -}
>>> -
>>> static void fb_set_logocmap(struct fb_info *info,
>>> const struct linux_logo *logo)
>>> {
>>> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
>>> index d043ada..f3b8856 100644
>>> --- a/include/linux/kernel.h
>>> +++ b/include/linux/kernel.h
>>> @@ -841,6 +841,10 @@ static inline void ftrace_dump(enum
>>> ftrace_dump_mode oops_dump_mode) { }
>>> */
>>> #define clamp_val(val, lo, hi) clamp_t(typeof(val), val, lo, hi)
>>>
>>> +static inline unsigned safe_shift(unsigned d, int n)
>>> +{
>>> + return n < 0 ? d >> -n : d << n;
>>> +}
>>>
>>> Thansk
>>> zhongjiang
>>>
>>>> Thanks,
>>>>
>>>> Ingo
>>>>
>>>> .
>>>>
>> What makes it reasonable? It is totally ill-defined and doesn't do
>anything useful now?
> Thanks you for comments.
>
>Maybe I mismake the meaning. I test the negative cases in x86 , all
>case is zero. so I come to a conclusion.
>
>zj.c:15:8: warning: left shift count is negative
>[-Wshift-count-negative]
> j = 1 << -2048;
> ^
>[root@localhost zhongjiang]# ./zj
>j = 0
>j.c:15:8: warning: left shift count is negative
>[-Wshift-count-negative]
> j = 1 << -2047;
> ^
>[root@localhost zhongjiang]# ./zj
>j = 0
>
>I insmod a module into kernel to test the testcasts, all of the result
>is zero.
>
>I wonder whether I miss some point or not. Do you point out to me?
>please
>
>Thanks
>zhongjiang
>
>
When you use compile-time constants, the compiler generates the value at compile time, which can be totally different.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href
