Re: [patch] mm: memcontrol: lockless page counters

Johannes Weiner
2014-09-24T16:51:28Z

On Wed, Sep 24, 2014 at 03:33:16PM +0200, Michal Hocko wrote:
> On Tue 23-09-14 13:05:25, Johannes Weiner wrote:
> [...]
> > How about the following update? Don't be thrown by the
> > page_counter_cancel(), I went back to it until we find something more
> > suitable. But as long as it's documented and has only 1.5 callsites,
> > it shouldn't matter all that much TBH.
> >
> > Thanks for your invaluable feedback so far, and sorry if the original
> > patch was hard to review. I'll try to break it up, to me it's usually
> > easier to verify new functions by looking at the callers in the same
> > patch, but I can probably remove the res_counter in a follow-up patch.
>
> The original patch was really huge and rather hard to review. Having
> res_counter removal in a separate patch would be definitely helpful.

Sorry, I just saw your follow-up after sending out v2. Yes, I split
out the res_counter removal, so the patch is a lot smaller.

> I would even lobby to have the new page_counter in a separate patch with
> the detailed description of the semantic and expected usage. Lockless
> schemes are always tricky and hard to review.

I was considering that (before someone explicitely asked for it) but
ended up thinking it's better to have the API go in along with the
main user, which often helps understanding. The users of the API are
unchanged, except for requiring callers to serialize limit updates.
And I commented all race conditions inside the counter code, hopefully
that helps, but let me know if things are unclear in v2.

Thanks