Subject: Re: [PATCH 3/8] arm64: memory: Rewrite default page_to_virt()/virt_to_page()

From: Mark Rutland <[email-masked]>
Date: Wed, 14 Aug 2019 12:26:23 +0100

On Wed, Aug 14, 2019 at 12:17:22PM +0100, Will Deacon wrote:
> On Wed, Aug 14, 2019 at 11:56:39AM +0100, Mark Rutland wrote:
> > On Wed, Aug 14, 2019 at 10:41:19AM +0100, Will Deacon wrote:
> > > On Wed, Aug 14, 2019 at 10:30:19AM +0100, Catalin Marinas wrote:
> > > > On Tue, Aug 13, 2019 at 06:01:44PM +0100, Will Deacon wrote:
> > > > > diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> > > > > index 47b4dc73b8bf..77074b3a1025 100644
> > > > > --- a/arch/arm64/include/asm/memory.h
> > > > > +++ b/arch/arm64/include/asm/memory.h
> > > > > @@ -313,19 +313,18 @@ static inline void *phys_to_virt(phys_addr_t x)
> > > > >  #if !defined(CONFIG_SPARSEMEM_VMEMMAP) || defined(CONFIG_DEBUG_VIRTUAL)
> > > > >  #define virt_to_page(kaddr)	pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
> > > > >  #else
> > > > > -#define __virt_to_pgoff(kaddr)	(((u64)(kaddr) - PAGE_OFFSET) / PAGE_SIZE * sizeof(struct page))
> > > > > -#define __page_to_voff(kaddr)	(((u64)(kaddr) - VMEMMAP_START) * PAGE_SIZE / sizeof(struct page))
> > > > > -
> > > > > -#define page_to_virt(page)	({					\
> > > > > -	unsigned long __addr =						\
> > > > > -		((__page_to_voff(page)) + PAGE_OFFSET);			\
> > > > > -	const void *__addr_tag =					\
> > > > > -		__tag_set((void *)__addr, page_kasan_tag(page));	\
> > > > > -	((void *)__addr_tag);						\
> > > > > +#define page_to_virt(x)	({						\
> > > > > +	__typeof__(x) __page = x;					\
> > > > 
> > > > Why not struct page * directly here?
> > > 
> > > I started out with that, but then you have to deal with const struct page *
> > > as well and it gets pretty messy.
> > 
> > What goes wrong if you always use const struct page *__page?
> 
> It would probably work, but then I wondered about the possibility of
> volatile and decided that __typeof__ was cleaner.

Ok, but I'd suggest that volatile struct page * that's never sane to use
in the first place.

If you don't want to change this, then no worries -- I just can't
follow.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel