All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Weiner <hannes@cmpxchg.org>
To: Michal Hocko <mhocko@suse.cz>
Cc: Vladimir Davydov <vdavydov@parallels.com>,
	linux-mm@kvack.org, Greg Thelen <gthelen@google.com>,
	Dave Hansen <dave@sr71.net>,
	cgroups@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [patch] mm: memcontrol: lockless page counters
Date: Wed, 24 Sep 2014 12:51:28 -0400	[thread overview]
Message-ID: <20140924165128.GA9968@cmpxchg.org> (raw)
In-Reply-To: <20140924133316.GA4558@dhcp22.suse.cz>

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

WARNING: multiple messages have this Message-ID (diff)
From: Johannes Weiner <hannes@cmpxchg.org>
To: Michal Hocko <mhocko@suse.cz>
Cc: Vladimir Davydov <vdavydov@parallels.com>,
	linux-mm@kvack.org, Greg Thelen <gthelen@google.com>,
	Dave Hansen <dave@sr71.net>,
	cgroups@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [patch] mm: memcontrol: lockless page counters
Date: Wed, 24 Sep 2014 12:51:28 -0400	[thread overview]
Message-ID: <20140924165128.GA9968@cmpxchg.org> (raw)
In-Reply-To: <20140924133316.GA4558@dhcp22.suse.cz>

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

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

  reply	other threads:[~2014-09-24 16:51 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-19 13:22 [patch] mm: memcontrol: lockless page counters Johannes Weiner
2014-09-19 13:22 ` Johannes Weiner
2014-09-19 13:29 ` Johannes Weiner
2014-09-19 13:29   ` Johannes Weiner
2014-09-22 14:41 ` Vladimir Davydov
2014-09-22 14:41   ` Vladimir Davydov
2014-09-22 18:57   ` Johannes Weiner
2014-09-22 18:57     ` Johannes Weiner
2014-09-22 18:57     ` Johannes Weiner
2014-09-23 11:06     ` Vladimir Davydov
2014-09-23 11:06       ` Vladimir Davydov
2014-09-23 11:06       ` Vladimir Davydov
2014-09-23 13:28       ` Johannes Weiner
2014-09-23 13:28         ` Johannes Weiner
2014-09-23 15:21         ` Vladimir Davydov
2014-09-23 15:21           ` Vladimir Davydov
2014-09-23 15:21           ` Vladimir Davydov
2014-09-23 17:05           ` Johannes Weiner
2014-09-23 17:05             ` Johannes Weiner
2014-09-23 17:05             ` Johannes Weiner
2014-09-24  8:02             ` Vladimir Davydov
2014-09-24  8:02               ` Vladimir Davydov
2014-09-24 13:33             ` Michal Hocko
2014-09-24 13:33               ` Michal Hocko
2014-09-24 13:33               ` Michal Hocko
2014-09-24 16:51               ` Johannes Weiner [this message]
2014-09-24 16:51                 ` Johannes Weiner
2014-09-24 14:16             ` Michal Hocko
2014-09-24 14:16               ` Michal Hocko
2014-09-24 17:00               ` Johannes Weiner
2014-09-24 17:00                 ` Johannes Weiner
2014-09-25 13:07                 ` Michal Hocko
2014-09-25 13:07                   ` Michal Hocko
2014-09-22 14:44 ` Michal Hocko
2014-09-22 14:44   ` Michal Hocko
2014-09-22 15:50   ` Johannes Weiner
2014-09-22 15:50     ` Johannes Weiner
2014-09-22 17:28     ` Michal Hocko
2014-09-22 17:28       ` Michal Hocko
2014-09-22 19:58       ` Johannes Weiner
2014-09-22 19:58         ` Johannes Weiner
2014-09-23 13:25         ` Michal Hocko
2014-09-23 13:25           ` Michal Hocko
2014-09-23 13:25           ` Michal Hocko
2014-09-23 14:05           ` Johannes Weiner
2014-09-23 14:05             ` Johannes Weiner
2014-09-23 14:28             ` Michal Hocko
2014-09-23 14:28               ` Michal Hocko
2014-09-23 14:28               ` Michal Hocko
2014-09-23 22:33               ` David Rientjes
2014-09-23 22:33                 ` David Rientjes
2014-09-23  7:46 ` Kamezawa Hiroyuki
2014-09-23  7:46   ` Kamezawa Hiroyuki

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=20140924165128.GA9968@cmpxchg.org \
    --to=hannes@cmpxchg.org \
    --cc=cgroups@vger.kernel.org \
    --cc=dave@sr71.net \
    --cc=gthelen@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.cz \
    --cc=vdavydov@parallels.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.