From: Anton Vorontsov <anton@enomsg.org>
To: Michal Hocko <mhocko@suse.cz>
Cc: Hyunhee Kim <hyunhee.kim@samsung.com>,
linux-mm@kvack.org, 'Kyungmin Park' <kyungmin.park@samsung.com>
Subject: Re: [PATCH] memcg: event control at vmpressure.
Date: Mon, 10 Jun 2013 17:17:47 -0700 [thread overview]
Message-ID: <20130611001747.GA16971@teo> (raw)
In-Reply-To: <20130610151258.GA14295@dhcp22.suse.cz>
On Mon, Jun 10, 2013 at 05:12:58PM +0200, Michal Hocko wrote:
> > + if (level >= ev->level && level != vmpr->current_level) {
> > eventfd_signal(ev->efd, 1);
> > signalled = true;
> > + vmpr->current_level = level;
>
> This would mean that you send a signal for, say, VMPRESSURE_LOW, then
> the reclaim finishes and two days later when you hit the reclaim again
> you would simply miss the event, right?
>
> So, unless I am missing something, then this is plain wrong.
Yup, in it current version, it is not acceptable. For example, sometimes
we do want to see all the _LOW events, since _LOW level shows not just the
level itself, but the activity (i.e. reclaiming process).
There are a few ways to make both parties happy, though.
If the app wants to implement the time-based throttling, then just close
the fd and sleep for needed amount of time (or do not read from the
eventfd -- kernel then will just increment the eventfd counter, so there
won't be context switches at the least). Doing the time-based throttling
in the kernel won't buy us much, I believe.
Or, if you still want the "one-shot"/"edge-triggered" events (which might
make perfect sense for medium and critical levels), then I'd propose to
add some additional flag when you register the event, so that the old
behaviour would be still available for those who need it. This approach I
think is the best one.
Thanks!
Anton
--
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:[~2013-06-11 0:17 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-10 11:14 Hyunhee Kim
2013-06-10 14:09 ` Luiz Capitulino
2013-06-10 15:12 ` Michal Hocko
2013-06-11 0:17 ` Anton Vorontsov [this message]
2013-06-11 1:01 ` Kyungmin Park
2013-06-11 6:21 ` Michal Hocko
2013-06-11 8:49 ` [PATCH v2] " Hyunhee Kim
2013-06-11 12:59 ` Michal Hocko
2013-06-12 5:42 ` Hyunhee Kim
2013-06-12 13:09 ` Michal Hocko
2013-06-11 13:10 ` [PATCH] " Luiz Capitulino
2013-06-11 13:13 ` Pekka Enberg
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=20130611001747.GA16971@teo \
--to=anton@enomsg.org \
--cc=hyunhee.kim@samsung.com \
--cc=kyungmin.park@samsung.com \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.cz \
/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