On Tue, Jan 24, 2012 at 12:43 AM, Michal Hocko [off-list ref] wrote:

On Mon 23-01-12 14:05:33, Ying Han wrote:

quoted

On Wed, Jan 18, 2012 at 3:53 PM, KAMEZAWA Hiroyuki
[off-list ref] wrote:

quoted

On Wed, 18 Jan 2012 11:47:03 +0100
Michal Hocko [off-list ref] wrote:

quoted

On Wed 18-01-12 09:12:26, KAMEZAWA Hiroyuki wrote:

quoted

On Tue, 17 Jan 2012 17:46:05 +0100
Michal Hocko [off-list ref] wrote:

quoted

On Fri 13-01-12 17:40:19, KAMEZAWA Hiroyuki wrote:

[...]

quoted

quoted

quoted

This patch removes PCG_MOVE_LOCK and add hashed rwlock array
instead of it. This works well enough. Even when we need to
take the lock,

Hmmm, rwlocks are not popular these days very much.
Anyway, can we rather make it (source) memcg (bit)spinlock instead. We
would reduce false sharing this way and would penalize only pages from
the moving group.

per-memcg spinlock ?

Yes

quoted

The reason I used rwlock() is to avoid disabling IRQ.  This routine
will be called by IRQ context (for dirty ratio support).  So, IRQ
disable will be required if we use spinlock.

OK, I have missed the comment about disabling IRQs. It's true that we do
not have to be afraid about deadlocks if the lock is held only for
reading from the irq context but does the spinlock makes a performance
bottleneck? We are talking about the slowpath.
I could see the reason for the read lock when doing hashed locks because
they are global but if we make the lock per memcg then we shouldn't
interfere with other updates which are not blocked by the move.

Hm, ok. In the next version, I'll use per-memcg spinlock (with hash if necessary)

Just want to make sure I understand it, even we make the lock
per-memcg, there is still a false sharing of pc within one memcg.

Yes that is true. I have missed that we might fault in several pages at
once but this would happen only during task move, right? And that is not
a hot path anyway. Or?

I was thinking of page-statistics update which is hot path. If the
moving task and non-moving task share the same per-memcg lock, any
page-statistic update from the non-moving task will be blocked? Sorry
If i missed something here :)

quoted

Do we need to demonstrate the effect ?

Also, I don't get the point of why spinlock instead of rwlock in this case?

spinlock provides a fairness while with rwlocks might lead to
starvation.

that is true.

--Ying

--
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic