From: "Jared Hulbert" <jaredeh@gmail.com>
To: Nick Piggin <nickpiggin@yahoo.com.au>
Cc: benh@kernel.crashing.org,
Linux Memory Management List <linux-mm@kvack.org>
Subject: Re: [RFC] Changing VM_PFNMAP assumptions and rules
Date: Mon, 12 Nov 2007 14:03:01 -0800 [thread overview]
Message-ID: <6934efce0711121403h2623958cq49490077c586924f@mail.gmail.com> (raw)
In-Reply-To: <200711111109.34562.nickpiggin@yahoo.com.au>
On 11/10/07, Nick Piggin <nickpiggin@yahoo.com.au> wrote:
> On Saturday 10 November 2007 06:15, Jared Hulbert wrote:
> > Per conversations regarding XIP from the vm/fs mini-summit a couple
> > months back I've got a patch to air out.
> >
> > The basic problem is that the assumptions about PFN mappings stemming
> > from the rules of remap_pfn_range() aren't always valid. For example:
> > what stops one from using vm_insert_pfn() to map PFN's into a vma in
> > an arbitrary order? Nothing. Yet those PFN's cause problems in two
> > ways.
> >
> > First, vm_normal_page() won't return NULL.
>
> They will, because it isn't allowed to be a COW mapping, and hence it
> fails the vm_normal_page() test.
No. It doesn't work. If I have a mapping that doesn't abide by the
pfn_of_page == vma->vm_pgoff + ((addr - vma->vm_start) >> PAGE_SHIFT)
rule, which is easy to do with vm_insert_pfn(), it won't get to NULL at
if (pfn == vma->vm_pgoff + off)
and because we don't expect to get this far with a PFN sometimes I do
have a is_cow_mapping() that is just a PFN. Which means I fail the
second test and get to the print_bad_pte(). At least that's what I
have captured in the past. Is that bad?
I'm still not sure you quite grasp what I am doing. You assume the
map only contains PFN and COW'ed in core pages. I'm mixing it all up.
A given page in a file for AXFS is either backed by a uncompressed
XIP'able page or a compressed page that needs to be uncompressed to
RAM to be used (think SquashFS, CramFS, etc.) So I would have raw
PFN's that are !pfn_valid() by nature, COW'ed pages that are from the
raw PFN, and in RAM pages that are backed by a compressed chunk on
Flash. What more the raw PFN's are definately not in
remap_pfn_range() order.
> > My answer to this is to
> > simply check if pfn_valid() if it isn't then we've got a proper PFN
> > that can only be a PFN. If you do have a valid PFN then you are (A) a
> > 'cow'ed' PFN that is now a real page or (B) you are a real page
> > pretending to be a PFN only. The thing that makes me nervous is that
> > my hack doesn't let that page pretend to be a PFN. I can't figure out
> > why a page would need/want to pretend to be a PFN so I don't see
> > anything wrong with this, but maybe somebody does.
> >
> > Second, there are a few random BUG_ON() that don't seem to serve any
> > purpose other than to punish the PFN's that don't abide by
> > remap_pfn_range() rules. I just get rid of them. The problem is I
> > don't really understand why they are there in the first place so for
> > all I know I'm horribly breaking spufs or something.
>
> They are perhaps slightly undercommented, but they are definitely
> required. And it is to ensure that everything works correctly.
Help me understand this. It seems to work fine if we remove these.
> > Okay so I haven't tried this out on 2.6.24-rc1 yet, but the same basic
> > idea worked on 2.6.23 and older. I just wanted to get feedback on
> > this approach. I don't know the vm all that well so I want to make
> > sure I'm not doing something really stupid that breaks a bunch of code
> > paths I don't use.
>
> You actually can't just use pfn_valid, because there are cases where
> you actually *cannot* touch the underlying struct page's mapcount,
> flags, etc. I think the only real user is /dev/mem.
Okay, I don't get why, but that's okay.
> So my suggestion to you, if you want to support COW pfnmaps, is to
> create a new VM_FLAG type (VM_INVALIDPFNMAP? ;)), which has the
> pfn_valid() == COW semantics that you want.
I __don't__ want pfn_valid() == COW. I want pfn_valid() ==
is_real_RAM_page(). That real RAM page is not necessarily COW'ed yet.
Remember I want a mapping that contains some Flash back pages and
some RAM backed pages.
> Keep the streamlined fastpath in vm_normal_page()...
>
> if (unlikely(vma->vm_flags & (VM_PFNMAP|VM_JAREDMAP))) {
> if (vma->vm_flags & VM_JAREDMAP) {
> if (!pfn_valid(pfn))
> return NULL;
> } else {
> unsigned long off = (addr - vma->vm_start) >> PAGE_SHIFT;
> if (pfn == vma->vm_pgoff + off)
> return NULL;
> if (!is_cow_mapping(vma->vm_flags))
> return NULL;
> }
> }
Got it.
> The tests in vm_insert_pfn would just be complementary to your new
> scheme..
> BUG_ON(!(vma->vm_flags & (VM_PFNMAP|VM_JAREDMAP)));
> BUG_ON((vma->vm_flags & VM_PFNMAP) && is_cow_mapping(vma->vm_flags));
Right.
> BUG_ON((vma->vm_flags & VM_JAREDMAP) && pfn_valid(pfn));
Okay, maybe. I got to look at this more carefully.
> May not work out so easy, but AFAIKS it will work. See how much milage
> that gets you.
>
> The other thing you might like is to allow pfn_valid(pfn) pfns to go
> into these mappings, and you know it is fine to twiddle with the
> struct page (eg. if you want to switch between different pfns, which
> I know the spufs guys want to). That's not too hard: just take out some
> of the assertions. You might have to do a little bit of setup work too,
> like increment the page count and mapcount etc. but just so long as you
> put that in a mm/memory.c helper rather than your own code, it should
> be clean enough.
Okay.... I don't understand how to do that. These PFN's are from an
MTD partition. They don't have a page structs. So I don't mind
having real page structs backing the Flash pages being used here. It
would make it unnecessary to tweak the filemap_xip.c stuff, eventually
it will be useful for doing read/write XIP stuff. However, I just
really don't get how to even start that.
I have a page that is at a hardware level read-only. What kind of
rules can that page live under? More importantly these PFN's get
mapped in with a call to ioremap() in the mtd drivers. So once I
figure out how to SPARSE_MEM, hotplug these pages in I've got to hack
the MTD to work with real pages. Or something like that. I'm not
ready to take that on yet, I just don't understand it all enough yet.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2007-11-12 22:03 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-11-09 19:15 Jared Hulbert
2007-11-11 0:09 ` Nick Piggin
2007-11-12 22:03 ` Jared Hulbert [this message]
2007-11-12 22:29 ` Benjamin Herrenschmidt
2007-11-12 23:53 ` Jared Hulbert
2007-11-13 0:24 ` Benjamin Herrenschmidt
2007-11-13 12:08 ` Nick Piggin
2007-11-14 1:29 ` Jared Hulbert
2007-11-13 17:26 ` Nick Piggin
2007-11-14 18:52 ` Jared Hulbert
2007-11-16 23:42 ` Jared Hulbert
2007-11-19 0:17 ` Nick Piggin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6934efce0711121403h2623958cq49490077c586924f@mail.gmail.com \
--to=jaredeh@gmail.com \
--cc=benh@kernel.crashing.org \
--cc=linux-mm@kvack.org \
--cc=nickpiggin@yahoo.com.au \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox