public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Anton Vorontsov <anton@enomsg.org>
To: Luiz Capitulino <lcapitulino@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Minchan Kim <minchan@kernel.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org, mhocko@suse.cz,
	kmpark@infradead.org, hyunhee.kim@samsung.com
Subject: Re: [PATCH v2] vmpressure: implement strict mode
Date: Fri, 28 Jun 2013 17:56:37 -0700	[thread overview]
Message-ID: <20130629005637.GA16068@teo> (raw)
In-Reply-To: <20130628154402.4035f2fa@redhat.com>

On Fri, Jun 28, 2013 at 03:44:02PM -0400, Luiz Capitulino wrote:
> > Why can't you use poll() and demultiplex the events? Check if there is an
> > event in the crit fd, and if there is, then just ignore all the rest.
> 
> This may be a valid workaround for current kernels, but application
> behavior will be different among kernels with a different number of
> events.

This is not a workaround, this is how poll works, and this is kinda
expected... But not that I had this plan in mind when I was designing the
current scheme... :)

> Say, we events on top of critical. Then crit fd will now be
> notified for cases where it didn't use to on older kernels.

I'm not sure I am following here... but thinking about it more, I guess
the extra read() will be needed anyway (to reset the counter).

> > > However, it *is* possible to make non-strict work on strict if we make
> > > strict default _and_ make reads on memory.pressure_level return
> > > available events. Just do this on app initialization:
> > > 
> > > for each event in memory.pressure_level; do
> > > 	/* register eventfd to be notified on "event" */
> > > done
> > 
> > This scheme registers "all" events.
> 
> Yes, because I thought that's the user-case that matters for activity
> manager :)

Some activity managers use only low levels (Android), some might use only
medium levels (simple load-balancing).

Being able to register only "all" does not make sense to me.

> > Here is more complicated case:
> > 
> > Old kernels, pressure_level reads:
> > 
> >   low, med, crit
> > 
> > The app just wants to listen for med level.
> > 
> > New kernels, pressure_level reads:
> > 
> >   low, FOO, med, BAR, crit
> > 
> > How would application decide which of FOO and BAR are ex-med levels?
> 
> What you meant by ex-med?

The scale is continuous and non-overlapping. If you add some other level,
you effectively "shrinking" other levels, so the ex-med in the list above
might correspond to "FOO, med" or "med, BAR" or "FOO, med, BAR", and that
is exactly the problem.

> Let's not over-design. I agree that allowing the API to be extended
> is a good thing, but we shouldn't give complex meaning to events,
> otherwise we're better with a numeric scale instead.

I am not over-desiging at all. Again, you did not provide any solution for
the case if we are going to add a new level. Thing is, I don't know if we
are going to add more levels, but the three-levels scheme is not something
scientifically proven, it is just an arbitrary thing we made up. We may
end up with four, or five... or not.

Thanks,

Anton

  reply	other threads:[~2013-06-29  0:56 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-27  3:17 [PATCH v2] vmpressure: implement strict mode Luiz Capitulino
2013-06-27  9:26 ` Michal Hocko
2013-06-27 13:34   ` Luiz Capitulino
2013-06-27 14:59     ` Michal Hocko
2013-06-27 15:53   ` Minchan Kim
2013-06-27 17:42     ` Michal Hocko
2013-06-27 15:44 ` Minchan Kim
2013-06-27 22:02 ` Andrew Morton
2013-06-28  0:02   ` Minchan Kim
2013-06-28  0:34     ` Andrew Morton
2013-06-28  0:58       ` Anton Vorontsov
2013-06-28  1:13         ` Andrew Morton
2013-06-28  4:34           ` Anton Vorontsov
2013-06-28  5:07             ` Anton Vorontsov
2013-06-28 14:00               ` Luiz Capitulino
2013-06-28 16:57                 ` Anton Vorontsov
2013-06-28 17:09                   ` Anton Vorontsov
2013-06-28 18:25                     ` Luiz Capitulino
2013-06-28 18:45                       ` Anton Vorontsov
2013-06-28 18:58                         ` Luiz Capitulino
2013-06-28 18:45                     ` Luiz Capitulino
2013-06-28 18:55                       ` Anton Vorontsov
2013-06-28 19:44                         ` Luiz Capitulino
2013-06-29  0:56                           ` Anton Vorontsov [this message]
2013-07-01  8:22                             ` Hyunhee Kim
2013-07-02  4:32                               ` Anton Vorontsov
2013-07-02  8:29                                 ` Hyunhee Kim
2013-07-02 13:29                             ` Michal Hocko
2013-07-02 14:59                             ` Luiz Capitulino
2013-07-02 17:24                               ` Anton Vorontsov
2013-07-02 18:38                                 ` Luiz Capitulino
2013-06-28  5:24             ` Andrew Morton
2013-06-28 13:43             ` Luiz Capitulino
2013-06-28  9:04           ` Minchan Kim

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=20130629005637.GA16068@teo \
    --to=anton@enomsg.org \
    --cc=akpm@linux-foundation.org \
    --cc=hyunhee.kim@samsung.com \
    --cc=kmpark@infradead.org \
    --cc=lcapitulino@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.cz \
    --cc=minchan@kernel.org \
    /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