Messages in this thread | | | Date | Thu, 20 May 2010 07:32:35 -0700 (PDT) | From | Linus Torvalds <> | Subject | Re: [PATCH] arch/tile: new multi-core architecture for Linux |
| |
On Thu, 20 May 2010, Barry Song wrote:
> On Thu, May 20, 2010 at 1:43 PM, Chris Metcalf <cmetcalf@tilera.com> wrote: > > > > static __always_inline void *lowmem_page_address(struct page *page) > > { > > - return __va(page_to_pfn(page) << PAGE_SHIFT); > > + return __va((phys_addr_t)page_to_pfn(page) << PAGE_SHIFT); > > Here doesn't make sense. you give a u64 type cast, but change the > meaning of pfn. Is pfn phys_addr_t? Anyway, page_to_pfn can be > re-fulfilled in your arch, but not change it in common code.
No, it actually makes a lot of sense.
The PFN may well be 32-bit, but then shifting it by PAGE_SHIFT turns the PFN from a PFN to a physical address. So the cast makes sense as a way to make sure that the code allows a 32-bit PFN with a 64-bit physical address.
So I don't thionk there's anything tile-specific about it, and it looks like a sane patch. If anything, it might make some sense to make this an explicit thing, ie have a "pfn_to_phys()" helper, because there's a _lot_ of these things open-coded.
And some of them actually have the cast already. See for example
#define pfn_to_nid(pfn) pa_to_nid(((u64)(pfn) << PAGE_SHIFT))
in the alpha <asm/mmzone.h>. Also:
resource_size_t offset = ((resource_size_t)pfn) << PAGE_SHIFT;
in the powerpc PCI code, of
#define page_to_phys(page) ((dma_addr_t)page_to_pfn(page) << PAGE_SHIFT)
in the x86 io code.
In fact, UM has that "pfn_to_phys()" helper already (and has a (phys_t) cast).
So we do already have a lot of casts (just grep for "pfn.*<<.*SHIFT" and you'll see them in generic code already, and the new one for tile makes 100% sense. In fact, we should clean up the existing ones.
Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |