On 2017/6/29 12:29, hpa@xxxxxxxxx wrote:
> 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.
yes, I test that. Thanks.
Thanks
zhongjiang
--
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=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
