Prev: dlm: use genl_register_family_with_ops()
Next: Add yield hypercall for KVM guests
From: Christoph Lameter on 26 Jul 2010 12:50
On Tue, 27 Jul 2010, Minchan Kim wrote:

> This patch registers address of mem_section to memmap itself's page struct's
> pg->private field. This means the page is used for memmap of the section.
> Otherwise, the page is used for other purpose and memmap has a hole.

What if page->private just happens to be the value of the page struct?
Even if that is not possible today, someday someone may add new
functionality to the kernel where page->pivage == page is used for some
reason.

Checking for PG_reserved wont work?

> +void mark_valid_memmap(unsigned long start, unsigned long end);
> +
> +#ifdef CONFIG_ARCH_HAS_HOLES_MEMORYMODEL
> +static inline int memmap_valid(unsigned long pfn)
> +{
> + struct page *page = pfn_to_page(pfn);
> + struct page *__pg = virt_to_page(page);
> + return page_private(__pg) == (unsigned long)__pg;

Hmmm.. hmmm....
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Minchan Kim on 26 Jul 2010 18:50
Hi Christoph,

On Tue, Jul 27, 2010 at 1:40 AM, Christoph Lameter
<cl(a)linux-foundation.org> wrote:
> On Tue, 27 Jul 2010, Minchan Kim wrote:
>
>> This patch registers address of mem_section to memmap itself's page struct's
>> pg->private field. This means the page is used for memmap of the section.
>> Otherwise, the page is used for other purpose and memmap has a hole.
>
> What if page->private just happens to be the value of the page struct?
> Even if that is not possible today, someday someone may add new
> functionality to the kernel where page->pivage == page is used for some
> reason.

I agree.

>
> Checking for PG_reserved wont work?

Okay. It would be better to consider page point itself with PG_reserved.
I will reflect your opinion next version. :)

Thanks, Christoph.




--
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: miltonm on 27 Jul 2010 02:00
[Sorry if i missed or added anyone on cc, patchwork.kernel.org LKML is not
working and I'm not subscribed to the list ]

On Mon Jul 26 2010 about 12:47:37 EST, Christoph Lameter wrote:
> On Tue, 27 Jul 2010, Minchan Kim wrote:
>
> > This patch registers address of mem_section to memmap itself's page struct's
> > pg->private field. This means the page is used for memmap of the section.
> > Otherwise, the page is used for other purpose and memmap has a hole.

>
> > +void mark_valid_memmap(unsigned long start, unsigned long end);
> > +
> > +#ifdef CONFIG_ARCH_HAS_HOLES_MEMORYMODEL
> > +static inline int memmap_valid(unsigned long pfn)
> > +{
> > + struct page *page = pfn_to_page(pfn);
> > + struct page *__pg = virt_to_page(page);
> > + return page_private(__pg) == (unsigned long)__pg;
>
>
> What if page->private just happens to be the value of the page struct?
> Even if that is not possible today, someday someone may add new
> functionality to the kernel where page->pivage == page is used for some
> reason.
>
> Checking for PG_reserved wont work?

I had the same thought and suggest setting it to the memory section block,
since that is a uniquie value (unlike PG_reserved),

> > static inline int pfn_valid(unsigned long pfn)
> > {
> > + struct mem_section *ms;
> > if (pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS)
> > return 0;
> > - return valid_section(__nr_to_section(pfn_to_section_nr(pfn)));
> > + ms = __nr_to_section(pfn_to_section_nr(pfn));
> > + return valid_section(ms) && memmap_valid(pfn);

... and we already have computed it when we use it so we could pass it as
a parameter (to both _valid and mark_valid).

milton
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Minchan Kim on 27 Jul 2010 02:20
On Tue, Jul 27, 2010 at 2:55 PM, <miltonm(a)bga.com> wrote:
> [Sorry if i missed or added anyone on cc, patchwork.kernel.org �LKML is not
> working and I'm not subscribed to the list ]

Readd them. :)

>
> On Mon Jul 26 2010 about 12:47:37 EST, Christoph Lameter wrote:
>> On Tue, 27 Jul 2010, Minchan Kim wrote:
>>
>> > This patch registers address of mem_section to memmap itself's page struct's
>> > pg->private field. This means the page is used for memmap of the section.
>> > Otherwise, the page is used for other purpose and memmap has a hole.
>
>>
>> > +void mark_valid_memmap(unsigned long start, unsigned long end);
>> > +
>> > +#ifdef CONFIG_ARCH_HAS_HOLES_MEMORYMODEL
>> > +static inline int memmap_valid(unsigned long pfn)
>> > +{
>> > + struct page *page = pfn_to_page(pfn);
>> > + struct page *__pg = virt_to_page(page);
>> > + return page_private(__pg) == (unsigned long)__pg;
>>
>>
>> What if page->private just happens to be the value of the page struct?
>> Even if that is not possible today, someday someone may add new
>> functionality to the kernel where page->pivage == page is used for some
>> reason.
>>
>> Checking for PG_reserved wont work?
>
> I had the same thought and suggest setting it to the memory section block,
> since that is a uniquie value (unlike PG_reserved),

You mean setting pg->private to mem_section address?
I hope I understand your point.

Actually, KAMEZAWA tried it at first version but I changed it.
That's because I want to support this mechanism to ARM FLATMEM.(It
doesn't have mem_section)


>
>> > static inline int pfn_valid(unsigned long pfn)
>> > {
>> > + struct mem_section *ms;
>> > if (pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS)
>> > return 0;
>> > - return valid_section(__nr_to_section(pfn_to_section_nr(pfn)));
>> > + ms = __nr_to_section(pfn_to_section_nr(pfn));
>> > + return valid_section(ms) && memmap_valid(pfn);
>
> .. and we already have computed it when we use it so we could pass it as
> a parameter (to both _valid and mark_valid).

I hope this can support FALTMEM which have holes(ex, ARM).

>
> milton
>



--
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: KAMEZAWA Hiroyuki on 27 Jul 2010 04:20
On Tue, 27 Jul 2010 03:12:38 -0500
Milton Miller <miltonm(a)bga.com> wrote:

> > > > > +/*
> > > > > + * Fill pg->private on valid mem_map with page itself.
> > > > > + * pfn_valid() will check this later. (see include/linux/mmzone.h)
> > > > > + * Every arch for supporting hole of mem_map should call
> > > > > + * mark_valid_memmap(start, end). please see usage in ARM.
> > > > > + */
> > > > > +void mark_valid_memmap(unsigned long start, unsigned long end)
> > > > > +{
> > > > > + struct mem_section *ms;
> > > > > + unsigned long pos, next;
> > > > > + struct page *pg;
> > > > > + void *memmap, *mapend;
> > > > > +
> > > > > + for (pos = start; pos < end; pos = next) {
> > > > > + next = (pos + PAGES_PER_SECTION) & PAGE_SECTION_MASK;
> > > > > + ms = __pfn_to_section(pos);
> > > > > + if (!valid_section(ms))
> > > > > + continue;
> > > > > +
> > > > > + for (memmap = (void*)pfn_to_page(pos),
> > > > > + /* The last page in section */
> > > > > + mapend = pfn_to_page(next-1);
> > > > > + memmap < mapend; memmap += PAGE_SIZE) {
> > > > > + pg = virt_to_page(memmap);
> > > > > + set_page_private(pg, (unsigned long)pg);
> > > > > + }
> > > > > + }
> > > > > +}
>
> Hmm, this loop would need to change for sections. And sizeof(struct
> page) % PAGE_SIZE may not be 0, so we want a global symbol for sparsemem
> too.
IIUC, sizeof(struct page) % PAGE_SIZE is not a probelm.


> Perhaps the mem_section array. Using a symbol that is part of
> the model pre-checks can remove a global symbol lookup and has the side
> effect of making sure our pfn_valid is for the right model.
>

But yes, maybe it's good to make use of a fixed-(magic)-value.


Tanks,
-Kame



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 |  Next  |  Last
Pages: 1 2 3 4 5 6 7
Prev: dlm: use genl_register_family_with_ops()
Next: Add yield hypercall for KVM guests