lkml.org 
[lkml]   [2010]   [May]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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/

\
 
 \ /
  Last update: 2010-05-20 16:37    [W:1.719 / U:0.188 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site