Re: [PATCH] m68knommu: switch to using asm-generic/uaccess.h

[Greg Ungerer <gerg@linux-m68k.org>]

Hi Geert,

On 9/9/20 4:55 pm, Geert Uytterhoeven wrote:
> Hi Greg,
> 
> On Wed, Sep 9, 2020 at 4:18 AM Greg Ungerer <gerg@linux-m68k.org> wrote:
>> Switch to using the asm-generic/uaccess functions for non-MMU builds.
>> Remove all the m68knommu local specific uaccess defines and macros.
>>
>> There is nothing so special about the m68knommu targets that they cannot
>> use all of the asm-generic uaccess support. Using the asm-generic
>> uaccess definitions also resolves some of the existing problems with
>> missing __user annotations in the m68knommu specific functions.
>>
>> The elimination of all of the contents of uaccess_no.h means we can fold
>> the uaccess_mm.h back into uaccess.h - and just have the single file
>> now.
>>
>> The resulting generated code ends up being slightly smaller (by a few
>> hundred bytes) due to the compilers ability to better optimize load
>> and stores without forcing its hand with asm statements.
>>
>> Specifically trivial cases like this contrived example:
>>
>>      get_user(x, ptr);
>>      x++;
>>      put_user(x, ptr);
>>
>> end up now being optimized to a single instruction on m68k. More
>> generally the compiler can avoid using a temporary register in many
>> cases as well.
>>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Signed-off-by: Greg Ungerer <gerg@linux-m68k.org>
> 
> Thanks for your patch!
> 
>> rename from arch/m68k/include/asm/uaccess_mm.h
>> rename to arch/m68k/include/asm/uaccess.h
>> index 9ae9f8d05925..f98208ccbbcd 100644
>> --- a/arch/m68k/include/asm/uaccess_mm.h
>> +++ b/arch/m68k/include/asm/uaccess.h
>> @@ -2,12 +2,15 @@
>>   #ifndef __M68K_UACCESS_H
>>   #define __M68K_UACCESS_H
>>
>> +#ifdef CONFIG_MMU
>> +
>>   /*
>>    * User space memory access functions
>>    */
>>   #include <linux/compiler.h>
>>   #include <linux/types.h>
>>   #include <asm/segment.h>
>> +#include <asm/extable.h>
> 
> For a moment, I wondered where this new include came from...
> 
> Seems like git doesn't show what happened to the old
> arch/m68k/include/asm/uaccess.h file, which was overwritten by the
> rename:
> 
>      -/* SPDX-License-Identifier: GPL-2.0 */
>      -#ifdef __uClinux__
>      -#include <asm/uaccess_no.h>
>      -#else
>      -#include <asm/uaccess_mm.h>
>      -#endif
>      -#include <asm/extable.h>
> 
> There it is ;-)
> 
> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

Thanks Geert.

I noticed that in the diff too. I generated this one with
"git format-patch -1 -M -B" to make it easier to review. Otherwise you end up
with all those identical lines being inserted and deleted.
But it makes that "#include <asm/extable.h>" appear to come out of nowhere.

Regards
Greg



>>   /* We let the MMU do all checking */
>>   static inline int access_ok(const void __user *addr,
>> @@ -387,4 +390,8 @@ unsigned long __clear_user(void __user *to, unsigned long n);
>>
>>   #define clear_user     __clear_user
>>
>> +#else /* !CONFIG_MMU */
>> +#include <asm-generic/uaccess.h>
>> +#endif
>> +
>>   #endif /* _M68K_UACCESS_H */
> 
> Gr{oetje,eeting}s,
> 
>                          Geert
>