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

Greg Ungerer
2020-09-09T07:54:45Z

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
>