| Message ID | 20220420030418.3189040-2-tongtiangen@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: add machine check safe support | expand |
Tong Tiangen <tongtiangen@huawei.com> writes: > x86/powerpc has it's implementation of copy_mc_to_user but not use #define > to declare. > > This may cause problems, for example, if other architectures open > CONFIG_ARCH_HAS_COPY_MC, but want to use copy_mc_to_user() outside the > architecture, the code add to include/linux/uaddess.h is as follows: > > #ifndef copy_mc_to_user > static inline unsigned long __must_check > copy_mc_to_user(void *dst, const void *src, size_t cnt) > { > ... > } > #endif The above doesn't exist yet, you add it in patch 3, which is a little confusing for a reader of this commit in isolation. I think you could safely move that into this patch, and then this patch would be ~= "Add generic fallback version of copy_mc_to_user()". It's probably not worth doing a whole new version of the series just for that, but if you need to do a new version for some other reason I think it would be cleaner to introduce the fallback in this commit. > Then this definition will conflict with the implementation of x86/powerpc > and cause compilation errors as follow: > > Fixes: ec6347bb4339 ("x86, powerpc: Rename memcpy_mcsafe() to copy_mc_to_{user, kernel}()") > Signed-off-by: Tong Tiangen <tongtiangen@huawei.com> > --- > arch/powerpc/include/asm/uaccess.h | 1 + Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc) cheers
在 2022/4/22 17:45, Michael Ellerman 写道: > Tong Tiangen <tongtiangen@huawei.com> writes: >> x86/powerpc has it's implementation of copy_mc_to_user but not use #define >> to declare. >> >> This may cause problems, for example, if other architectures open >> CONFIG_ARCH_HAS_COPY_MC, but want to use copy_mc_to_user() outside the >> architecture, the code add to include/linux/uaddess.h is as follows: >> >> #ifndef copy_mc_to_user >> static inline unsigned long __must_check >> copy_mc_to_user(void *dst, const void *src, size_t cnt) >> { >> ... >> } >> #endif > > The above doesn't exist yet, you add it in patch 3, which is a little > confusing for a reader of this commit in isolation. > > I think you could safely move that into this patch, and then this patch > would be ~= "Add generic fallback version of copy_mc_to_user()". > > It's probably not worth doing a whole new version of the series just for > that, but if you need to do a new version for some other reason I think > it would be cleaner to introduce the fallback in this commit. > Agreed, will do in next version. Thanks, Tong. >> Then this definition will conflict with the implementation of x86/powerpc >> and cause compilation errors as follow: >> >> Fixes: ec6347bb4339 ("x86, powerpc: Rename memcpy_mcsafe() to copy_mc_to_{user, kernel}()") >> Signed-off-by: Tong Tiangen <tongtiangen@huawei.com> >> --- >> arch/powerpc/include/asm/uaccess.h | 1 + > > Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc) > > cheers > .
Le 20/04/2022 à 05:04, Tong Tiangen a écrit : > x86/powerpc has it's implementation of copy_mc_to_user but not use #define > to declare. > > This may cause problems, for example, if other architectures open > CONFIG_ARCH_HAS_COPY_MC, but want to use copy_mc_to_user() outside the > architecture, the code add to include/linux/uaddess.h is as follows: > > #ifndef copy_mc_to_user > static inline unsigned long __must_check > copy_mc_to_user(void *dst, const void *src, size_t cnt) > { > ... > } > #endif > > Then this definition will conflict with the implementation of x86/powerpc > and cause compilation errors as follow: > > Fixes: ec6347bb4339 ("x86, powerpc: Rename memcpy_mcsafe() to copy_mc_to_{user, kernel}()") I don't understand, what does it fix really ? What was the (existing/real) bug introduced by that patch and that your are fixing ? If those defined had been expected and missing, we would have had a build failure. If you have one, can you describe it ? > Signed-off-by: Tong Tiangen <tongtiangen@huawei.com> > --- > arch/powerpc/include/asm/uaccess.h | 1 + > arch/x86/include/asm/uaccess.h | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h > index 9b82b38ff867..58dbe8e2e318 100644 > --- a/arch/powerpc/include/asm/uaccess.h > +++ b/arch/powerpc/include/asm/uaccess.h > @@ -358,6 +358,7 @@ copy_mc_to_user(void __user *to, const void *from, unsigned long n) > > return n; > } > +#define copy_mc_to_user copy_mc_to_user > #endif > > extern long __copy_from_user_flushcache(void *dst, const void __user *src, > diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h > index f78e2b3501a1..e18c5f098025 100644 > --- a/arch/x86/include/asm/uaccess.h > +++ b/arch/x86/include/asm/uaccess.h > @@ -415,6 +415,7 @@ copy_mc_to_kernel(void *to, const void *from, unsigned len); > > unsigned long __must_check > copy_mc_to_user(void *to, const void *from, unsigned len); > +#define copy_mc_to_user copy_mc_to_user > #endif > > /*
在 2022/5/2 22:24, Christophe Leroy 写道: > > > Le 20/04/2022 à 05:04, Tong Tiangen a écrit : >> x86/powerpc has it's implementation of copy_mc_to_user but not use #define >> to declare. >> >> This may cause problems, for example, if other architectures open >> CONFIG_ARCH_HAS_COPY_MC, but want to use copy_mc_to_user() outside the >> architecture, the code add to include/linux/uaddess.h is as follows: >> >> #ifndef copy_mc_to_user >> static inline unsigned long __must_check >> copy_mc_to_user(void *dst, const void *src, size_t cnt) >> { >> ... >> } >> #endif >> >> Then this definition will conflict with the implementation of x86/powerpc >> and cause compilation errors as follow: >> >> Fixes: ec6347bb4339 ("x86, powerpc: Rename memcpy_mcsafe() to copy_mc_to_{user, kernel}()") > > I don't understand, what does it fix really ? What was the > (existing/real) bug introduced by that patch and that your are fixing ? > > If those defined had been expected and missing, we would have had a > build failure. If you have one, can you describe it ? There will be build failure after patch 3 is added, there is a little confusing for a reader of this commit in isolation. In the next version, I will put this patch after patch 3. Thanks, Tong. > >> Signed-off-by: Tong Tiangen <tongtiangen@huawei.com> >> --- >> arch/powerpc/include/asm/uaccess.h | 1 + >> arch/x86/include/asm/uaccess.h | 1 + >> 2 files changed, 2 insertions(+) >> >> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h >> index 9b82b38ff867..58dbe8e2e318 100644 >> --- a/arch/powerpc/include/asm/uaccess.h >> +++ b/arch/powerpc/include/asm/uaccess.h >> @@ -358,6 +358,7 @@ copy_mc_to_user(void __user *to, const void *from, unsigned long n) >> >> return n; >> } >> +#define copy_mc_to_user copy_mc_to_user >> #endif >> >> extern long __copy_from_user_flushcache(void *dst, const void __user *src, >> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h >> index f78e2b3501a1..e18c5f098025 100644 >> --- a/arch/x86/include/asm/uaccess.h >> +++ b/arch/x86/include/asm/uaccess.h >> @@ -415,6 +415,7 @@ copy_mc_to_kernel(void *to, const void *from, unsigned len); >> >> unsigned long __must_check >> copy_mc_to_user(void *to, const void *from, unsigned len); >> +#define copy_mc_to_user copy_mc_to_user >> #endif >> >> /*
On 2022/5/3 9:06, Tong Tiangen wrote: > > > 在 2022/5/2 22:24, Christophe Leroy 写道: >> >> >> Le 20/04/2022 à 05:04, Tong Tiangen a écrit : >>> x86/powerpc has it's implementation of copy_mc_to_user but not use >>> #define >>> to declare. >>> >>> This may cause problems, for example, if other architectures open >>> CONFIG_ARCH_HAS_COPY_MC, but want to use copy_mc_to_user() outside the >>> architecture, the code add to include/linux/uaddess.h is as follows: >>> >>> #ifndef copy_mc_to_user >>> static inline unsigned long __must_check >>> copy_mc_to_user(void *dst, const void *src, size_t cnt) >>> { >>> ... >>> } >>> #endif >>> >>> Then this definition will conflict with the implementation of >>> x86/powerpc >>> and cause compilation errors as follow: >>> >>> Fixes: ec6347bb4339 ("x86, powerpc: Rename memcpy_mcsafe() to >>> copy_mc_to_{user, kernel}()") >> >> I don't understand, what does it fix really ? What was the >> (existing/real) bug introduced by that patch and that your are fixing ? >> >> If those defined had been expected and missing, we would have had a >> build failure. If you have one, can you describe it ? > It could prevent future problems when patch3 is introduced， and yes，for now， this patch won't fix any issue，we could drop the fix tag, and update the changelog. > There will be build failure after patch 3 is added, there is a little > confusing for a reader of this commit in isolation. > In the next version, I will put this patch after patch 3. This is an alternative. > > Thanks, > Tong. > .
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h index 9b82b38ff867..58dbe8e2e318 100644 --- a/arch/powerpc/include/asm/uaccess.h +++ b/arch/powerpc/include/asm/uaccess.h @@ -358,6 +358,7 @@ copy_mc_to_user(void __user *to, const void *from, unsigned long n) return n; } +#define copy_mc_to_user copy_mc_to_user #endif extern long __copy_from_user_flushcache(void *dst, const void __user *src, diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h index f78e2b3501a1..e18c5f098025 100644 --- a/arch/x86/include/asm/uaccess.h +++ b/arch/x86/include/asm/uaccess.h @@ -415,6 +415,7 @@ copy_mc_to_kernel(void *to, const void *from, unsigned len); unsigned long __must_check copy_mc_to_user(void *to, const void *from, unsigned len); +#define copy_mc_to_user copy_mc_to_user #endif /*
x86/powerpc has it's implementation of copy_mc_to_user but not use #define to declare. This may cause problems, for example, if other architectures open CONFIG_ARCH_HAS_COPY_MC, but want to use copy_mc_to_user() outside the architecture, the code add to include/linux/uaddess.h is as follows: #ifndef copy_mc_to_user static inline unsigned long __must_check copy_mc_to_user(void *dst, const void *src, size_t cnt) { ... } #endif Then this definition will conflict with the implementation of x86/powerpc and cause compilation errors as follow: Fixes: ec6347bb4339 ("x86, powerpc: Rename memcpy_mcsafe() to copy_mc_to_{user, kernel}()") Signed-off-by: Tong Tiangen <tongtiangen@huawei.com> --- arch/powerpc/include/asm/uaccess.h | 1 + arch/x86/include/asm/uaccess.h | 1 + 2 files changed, 2 insertions(+)