Balbir Singh wrote:

Nick Piggin wrote:

quoted

Balbir Singh wrote:

quoted

Nick Piggin wrote:

quoted

Very good review comment. Here's what we see

1. Page comes in through page cache, we increment the reference ount
2. Page comes into rmap, we increment the refcount again
3. We race in page_add.*rmap(), the problem we have is that for
  the same page, for rmap(), step 2 would have taken place more than
  once
4. That's why we uncharge

I think I need to add a big fat comment in the ref_cnt member. ref_cnt
is held once for the page in the page cache and once for the page mapped
anywhere in the page tables.

reference counting helps us correctly determine when to take the page
of the LRU (it moves from page tables to swap cache, but it's still
on the LRU).

Still don't understand. You increment the refcount once when you put
the page in the pagecache, then again when the first process maps the
page, then again while subsequent processes map the page but you soon
drop it afterwards. That's fine, I don't pretend to understand why
you're doing it, but presumably the controller has a good reason for
that.

But my point is, why should the VM know or care about that? You should
handle all those details in your controller. If, in order to do that,
you need to differentiate between when a process puts a page in
pagecache and when it maps a page, that's fine, just use different
hooks for those events.

The situation now is that your one hook is not actually a "this page
was mapped" hook, or a "this page was added to pagecache", or "we are
about to map this page". These are easy for VM maintainers to maintain
because they're simple VM concepts.

But your hook is "increment ref_cnt and do some other stuff". So now
the VM needs to know about when and why your container implementation
needs to increment and decrement this ref_cnt. I don't know this, and
I don't want to know this ;)

My hook really is -- there was a race, there is no rmap lock to prevent
several independent processes from mapping the same page into their
page tables. I want to increment the reference count just once (apart from
it being accounted in the page cache), since we account the page once.

I'll revisit this hook to see if it can be made cleaner

If you just have a different hook for mapping a page into the page
tables, your controller can take care of any races, no?

-- 
SUSE Labs, Novell Inc.

--
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>