2024-03-13 04:51:08

by Barry Song

[permalink] [raw]
Subject: [PATCH] xtensa: remove redundant flush_dcache_page and ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE macros

From: Barry Song <[email protected]>

xtensa's flush_dcache_page() can be a no-op sometimes. There is a
generic implementation for this case in include/asm-generic/
cacheflush.h.
#ifndef ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE
static inline void flush_dcache_page(struct page *page)
{
}

#define ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE 0
#endif

So remove the superfluous flush_dcache_page() definition, which also
helps silence potential build warnings complaining the page variable
passed to flush_dcache_page() is not used.

In file included from crypto/scompress.c:12:
include/crypto/scatterwalk.h: In function 'scatterwalk_pagedone':
include/crypto/scatterwalk.h:76:30: warning: variable 'page' set but not used [-Wunused-but-set-variable]
76 | struct page *page;
| ^~~~
crypto/scompress.c: In function 'scomp_acomp_comp_decomp':
>> crypto/scompress.c:174:38: warning: unused variable 'dst_page' [-Wunused-variable]
174 | struct page *dst_page = sg_page(req->dst);
|

The issue was originally reported on LoongArch by kernel test
robot. And Huacai fixed it on LoongArch, but I found xtensa
obviously has the same issue.

Cc: Huacai Chen <[email protected]>
Cc: Herbert Xu <[email protected]>
Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
Closes: https://lore.kernel.org/all/CAGsJ_4yDk1+axbte7FKQEwD7X2oxUCFrEc9M5YOS1BobfDFXPA@mail.gmail.com/
Signed-off-by: Barry Song <[email protected]>
---
arch/xtensa/include/asm/cacheflush.h | 3 ---
1 file changed, 3 deletions(-)

diff --git a/arch/xtensa/include/asm/cacheflush.h b/arch/xtensa/include/asm/cacheflush.h
index 38bcecb0e457..11efdc8eb262 100644
--- a/arch/xtensa/include/asm/cacheflush.h
+++ b/arch/xtensa/include/asm/cacheflush.h
@@ -144,9 +144,6 @@ void local_flush_cache_page(struct vm_area_struct *vma,
#define flush_cache_vmap_early(start,end) do { } while (0)
#define flush_cache_vunmap(start,end) do { } while (0)

-#define ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE 0
-#define flush_dcache_page(page) do { } while (0)
-
#define flush_icache_range local_flush_icache_range
#define flush_cache_page(vma, addr, pfn) do { } while (0)
#define flush_cache_range(vma, start, end) do { } while (0)
--
2.34.1



2024-03-18 00:16:17

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH] xtensa: remove redundant flush_dcache_page and ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE macros

On Wed, Mar 13, 2024 at 05:50:36PM +1300, Barry Song wrote:
> From: Barry Song <[email protected]>
>
> xtensa's flush_dcache_page() can be a no-op sometimes. There is a
> generic implementation for this case in include/asm-generic/
> cacheflush.h.
> #ifndef ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE
> static inline void flush_dcache_page(struct page *page)
> {
> }
>
> #define ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE 0
> #endif
>
> So remove the superfluous flush_dcache_page() definition, which also
> helps silence potential build warnings complaining the page variable
> passed to flush_dcache_page() is not used.
>
> In file included from crypto/scompress.c:12:
> include/crypto/scatterwalk.h: In function 'scatterwalk_pagedone':
> include/crypto/scatterwalk.h:76:30: warning: variable 'page' set but not used [-Wunused-but-set-variable]
> 76 | struct page *page;
> | ^~~~
> crypto/scompress.c: In function 'scomp_acomp_comp_decomp':
> >> crypto/scompress.c:174:38: warning: unused variable 'dst_page' [-Wunused-variable]
> 174 | struct page *dst_page = sg_page(req->dst);
> |
>
> The issue was originally reported on LoongArch by kernel test
> robot. And Huacai fixed it on LoongArch, but I found xtensa
> obviously has the same issue.
>

Maybe I am doing something wrong, but this patch doesn't build
for me.

CC arch/xtensa/kernel/asm-offsets.s
In file included from ./include/linux/highmem.h:8,
from ./include/linux/bvec.h:10,
from ./include/linux/blk_types.h:10,
from ./include/linux/writeback.h:13,
from ./include/linux/memcontrol.h:23,
from ./include/linux/swap.h:9,
from ./include/linux/suspend.h:5,
from arch/xtensa/kernel/asm-offsets.c:24:
/include/linux/cacheflush.h:9:5: error: "ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE" is not defined, evaluates to 0 [-Werror=undef]
9 | #if ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE

Guenter

2024-03-18 00:28:06

by Barry Song

[permalink] [raw]
Subject: Re: [PATCH] xtensa: remove redundant flush_dcache_page and ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE macros

On Mon, Mar 18, 2024 at 8:16 AM Guenter Roeck <[email protected]> wrote:
>
> On Wed, Mar 13, 2024 at 05:50:36PM +1300, Barry Song wrote:
> > From: Barry Song <[email protected]>
> >
> > xtensa's flush_dcache_page() can be a no-op sometimes. There is a
> > generic implementation for this case in include/asm-generic/
> > cacheflush.h.
> > #ifndef ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE
> > static inline void flush_dcache_page(struct page *page)
> > {
> > }
> >
> > #define ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE 0
> > #endif
> >
> > So remove the superfluous flush_dcache_page() definition, which also
> > helps silence potential build warnings complaining the page variable
> > passed to flush_dcache_page() is not used.
> >
> > In file included from crypto/scompress.c:12:
> > include/crypto/scatterwalk.h: In function 'scatterwalk_pagedone':
> > include/crypto/scatterwalk.h:76:30: warning: variable 'page' set but not used [-Wunused-but-set-variable]
> > 76 | struct page *page;
> > | ^~~~
> > crypto/scompress.c: In function 'scomp_acomp_comp_decomp':
> > >> crypto/scompress.c:174:38: warning: unused variable 'dst_page' [-Wunused-variable]
> > 174 | struct page *dst_page = sg_page(req->dst);
> > |
> >
> > The issue was originally reported on LoongArch by kernel test
> > robot. And Huacai fixed it on LoongArch, but I found xtensa
> > obviously has the same issue.
> >
>
> Maybe I am doing something wrong, but this patch doesn't build
> for me.
>
> CC arch/xtensa/kernel/asm-offsets.s
> In file included from ./include/linux/highmem.h:8,
> from ./include/linux/bvec.h:10,
> from ./include/linux/blk_types.h:10,
> from ./include/linux/writeback.h:13,
> from ./include/linux/memcontrol.h:23,
> from ./include/linux/swap.h:9,
> from ./include/linux/suspend.h:5,
> from arch/xtensa/kernel/asm-offsets.c:24:
> ./include/linux/cacheflush.h:9:5: error: "ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE" is not defined, evaluates to 0 [-Werror=undef]
> 9 | #if ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE

is it because xtensa doesn't include this at the end of
arch/xtensa/include/asm/cacheflush.h
while other architectures do?

#include <asm-generic/cacheflush.h>



>
> Guenter

2024-03-18 00:35:00

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH] xtensa: remove redundant flush_dcache_page and ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE macros

On 3/17/24 17:27, Barry Song wrote:
> On Mon, Mar 18, 2024 at 8:16 AM Guenter Roeck <[email protected]> wrote:
>>
>> On Wed, Mar 13, 2024 at 05:50:36PM +1300, Barry Song wrote:
>>> From: Barry Song <[email protected]>
>>>
>>> xtensa's flush_dcache_page() can be a no-op sometimes. There is a
>>> generic implementation for this case in include/asm-generic/
>>> cacheflush.h.
>>> #ifndef ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE
>>> static inline void flush_dcache_page(struct page *page)
>>> {
>>> }
>>>
>>> #define ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE 0
>>> #endif
>>>
>>> So remove the superfluous flush_dcache_page() definition, which also
>>> helps silence potential build warnings complaining the page variable
>>> passed to flush_dcache_page() is not used.
>>>
>>> In file included from crypto/scompress.c:12:
>>> include/crypto/scatterwalk.h: In function 'scatterwalk_pagedone':
>>> include/crypto/scatterwalk.h:76:30: warning: variable 'page' set but not used [-Wunused-but-set-variable]
>>> 76 | struct page *page;
>>> | ^~~~
>>> crypto/scompress.c: In function 'scomp_acomp_comp_decomp':
>>>>> crypto/scompress.c:174:38: warning: unused variable 'dst_page' [-Wunused-variable]
>>> 174 | struct page *dst_page = sg_page(req->dst);
>>> |
>>>
>>> The issue was originally reported on LoongArch by kernel test
>>> robot. And Huacai fixed it on LoongArch, but I found xtensa
>>> obviously has the same issue.
>>>
>>
>> Maybe I am doing something wrong, but this patch doesn't build
>> for me.
>>
>> CC arch/xtensa/kernel/asm-offsets.s
>> In file included from ./include/linux/highmem.h:8,
>> from ./include/linux/bvec.h:10,
>> from ./include/linux/blk_types.h:10,
>> from ./include/linux/writeback.h:13,
>> from ./include/linux/memcontrol.h:23,
>> from ./include/linux/swap.h:9,
>> from ./include/linux/suspend.h:5,
>> from arch/xtensa/kernel/asm-offsets.c:24:
>> ./include/linux/cacheflush.h:9:5: error: "ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE" is not defined, evaluates to 0 [-Werror=undef]
>> 9 | #if ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE
>
> is it because xtensa doesn't include this at the end of
> arch/xtensa/include/asm/cacheflush.h
> while other architectures do?
>
> #include <asm-generic/cacheflush.h>
>

Looks like it (see my other e-mail).

Guenter


2024-03-18 00:38:07

by Barry Song

[permalink] [raw]
Subject: Re: [PATCH] xtensa: remove redundant flush_dcache_page and ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE macros

On Mon, Mar 18, 2024 at 8:34 AM Guenter Roeck <[email protected]> wrote:
>
> On 3/17/24 17:27, Barry Song wrote:
> > On Mon, Mar 18, 2024 at 8:16 AM Guenter Roeck <[email protected]> wrote:
> >>
> >> On Wed, Mar 13, 2024 at 05:50:36PM +1300, Barry Song wrote:
> >>> From: Barry Song <[email protected]>
> >>>
> >>> xtensa's flush_dcache_page() can be a no-op sometimes. There is a
> >>> generic implementation for this case in include/asm-generic/
> >>> cacheflush.h.
> >>> #ifndef ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE
> >>> static inline void flush_dcache_page(struct page *page)
> >>> {
> >>> }
> >>>
> >>> #define ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE 0
> >>> #endif
> >>>
> >>> So remove the superfluous flush_dcache_page() definition, which also
> >>> helps silence potential build warnings complaining the page variable
> >>> passed to flush_dcache_page() is not used.
> >>>
> >>> In file included from crypto/scompress.c:12:
> >>> include/crypto/scatterwalk.h: In function 'scatterwalk_pagedone':
> >>> include/crypto/scatterwalk.h:76:30: warning: variable 'page' set but not used [-Wunused-but-set-variable]
> >>> 76 | struct page *page;
> >>> | ^~~~
> >>> crypto/scompress.c: In function 'scomp_acomp_comp_decomp':
> >>>>> crypto/scompress.c:174:38: warning: unused variable 'dst_page' [-Wunused-variable]
> >>> 174 | struct page *dst_page = sg_page(req->dst);
> >>> |
> >>>
> >>> The issue was originally reported on LoongArch by kernel test
> >>> robot. And Huacai fixed it on LoongArch, but I found xtensa
> >>> obviously has the same issue.
> >>>
> >>
> >> Maybe I am doing something wrong, but this patch doesn't build
> >> for me.
> >>
> >> CC arch/xtensa/kernel/asm-offsets.s
> >> In file included from ./include/linux/highmem.h:8,
> >> from ./include/linux/bvec.h:10,
> >> from ./include/linux/blk_types.h:10,
> >> from ./include/linux/writeback.h:13,
> >> from ./include/linux/memcontrol.h:23,
> >> from ./include/linux/swap.h:9,
> >> from ./include/linux/suspend.h:5,
> >> from arch/xtensa/kernel/asm-offsets.c:24:
> >> ./include/linux/cacheflush.h:9:5: error: "ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE" is not defined, evaluates to 0 [-Werror=undef]
> >> 9 | #if ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE
> >
> > is it because xtensa doesn't include this at the end of
> > arch/xtensa/include/asm/cacheflush.h
> > while other architectures do?
> >
> > #include <asm-generic/cacheflush.h>
> >
>
> Looks like it (see my other e-mail).

i will send v2 and it seems a lot of other code can also be removed in
arch/xtensa/include/asm/cacheflush.h,

for example

#define flush_cache_all() do { } while (0)
#define flush_cache_mm(mm) do { } while (0)
#define flush_cache_dup_mm(mm) do { } while (0)

#define flush_cache_vmap(start,end) do { } while (0)
#define flush_cache_vmap_early(start,end) do { } while (0)
#define flush_cache_vunmap(start,end) do { } while (0)

#define ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE 0
#define flush_dcache_page(page) do { } while (0)

#define flush_cache_page(vma, addr, pfn) do { } while (0)
#define flush_cache_range(vma, start, end) do { } while (0)


#define flush_dcache_mmap_lock(mapping) do { } while (0)
#define flush_dcache_mmap_unlock(mapping) do { } while (0)

>
> Guenter
>