| Message ID | 20230126141144.11042-1-fmdefrancesco@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/highmem: Align-down to page the address for kunmap_flush_on_unmap() | expand |
On giovedì 26 gennaio 2023 15:11:44 CET Fabio M. De Francesco wrote: > If ARCH_HAS_FLUSH_ON_KUNMAP is defined (PA-RISC case), __kunmap_local() > calls kunmap_flush_on_unmap(). The latter currently flushes the wrong > address (as confirmed by Matthew Wilcox and Helge Deller). Al Viro > proposed to call kunmap_flush_on_unmap() on an aligned-down to page > address in order to fix this issue. Consensus has been reached on this > solution. > > Therefore, if ARCH_HAS_FLUSH_ON_KUNMAP is defined, call > kunmap_flush_on_unmap() on an aligned-down to page address computed with > the PTR_ALIGN_DOWN() macro. > > Cc: Ira Weiny <ira.weiny@intel.com> > Suggested-by: Al Viro <viro@zeniv.linux.org.uk> > Confirmed-by: Helge Deller <deller@gmx.de> > Confirmed-by: Matthew Wilcox <willy@infradead.org> > Fixes: f3ba3c710ac5 ("mm/highmem: Provide kmap_local*") > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> > --- > > I have (at least) two problems with this patch... > > 1) checkpatch.pl complains about the use of the non-standard > "Confirmed-by" tags. I don't know how else I can give credit to Helge > and Matthew. However, this is not the first time that I see non-standard > tags in patches applied upstream (I too had a non-standard > "Analysed-by" tag in patch which fixes a SAC bug). Any objections? > > 2) I'm not sure whether or not the "Fixes" tag is appropriate in this > patch. Can someone either confirm or deny it? > > include/linux/highmem-internal.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/linux/highmem-internal.h > b/include/linux/highmem-internal.h index 034b1106d022..e247c9ac4583 100644 > --- a/include/linux/highmem-internal.h > +++ b/include/linux/highmem-internal.h > @@ -200,7 +200,7 @@ static inline void *kmap_local_pfn(unsigned long pfn) > static inline void __kunmap_local(const void *addr) > { > #ifdef ARCH_HAS_FLUSH_ON_KUNMAP > - kunmap_flush_on_unmap(addr); > + kunmap_flush_on_unmap(PTR_ALIGN_DOWN(addr, PAGE_SIZE)); > #endif > } > > -- > 2.39.0 I just realized that I forgot to Cc Thomas Gleixner. Therefore, I think I'd better add him to the list and resend this patch. I'm doing it immediately, so please drop this. Thanks, Fabio
diff --git a/include/linux/highmem-internal.h b/include/linux/highmem-internal.h index 034b1106d022..e247c9ac4583 100644 --- a/include/linux/highmem-internal.h +++ b/include/linux/highmem-internal.h @@ -200,7 +200,7 @@ static inline void *kmap_local_pfn(unsigned long pfn) static inline void __kunmap_local(const void *addr) { #ifdef ARCH_HAS_FLUSH_ON_KUNMAP - kunmap_flush_on_unmap(addr); + kunmap_flush_on_unmap(PTR_ALIGN_DOWN(addr, PAGE_SIZE)); #endif }
If ARCH_HAS_FLUSH_ON_KUNMAP is defined (PA-RISC case), __kunmap_local() calls kunmap_flush_on_unmap(). The latter currently flushes the wrong address (as confirmed by Matthew Wilcox and Helge Deller). Al Viro proposed to call kunmap_flush_on_unmap() on an aligned-down to page address in order to fix this issue. Consensus has been reached on this solution. Therefore, if ARCH_HAS_FLUSH_ON_KUNMAP is defined, call kunmap_flush_on_unmap() on an aligned-down to page address computed with the PTR_ALIGN_DOWN() macro. Cc: Ira Weiny <ira.weiny@intel.com> Suggested-by: Al Viro <viro@zeniv.linux.org.uk> Confirmed-by: Helge Deller <deller@gmx.de> Confirmed-by: Matthew Wilcox <willy@infradead.org> Fixes: f3ba3c710ac5 ("mm/highmem: Provide kmap_local*") Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> --- I have (at least) two problems with this patch... 1) checkpatch.pl complains about the use of the non-standard "Confirmed-by" tags. I don't know how else I can give credit to Helge and Matthew. However, this is not the first time that I see non-standard tags in patches applied upstream (I too had a non-standard "Analysed-by" tag in patch which fixes a SAC bug). Any objections? 2) I'm not sure whether or not the "Fixes" tag is appropriate in this patch. Can someone either confirm or deny it? include/linux/highmem-internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)