All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin LaHaise <bcrl@kvack.org>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Jeff Moyer <jmoyer@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Joonsoo Kim <js1304@gmail.com>,
	Fengguang Wu <fengguang.wu@intel.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	linux-next@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm-move-mremap-from-file_operations-to-vm_operations_struct-fix
Date: Mon, 20 Jul 2015 13:51:45 -0400	[thread overview]
Message-ID: <20150720175145.GH21558@kvack.org> (raw)
In-Reply-To: <20150720173311.GA4379@redhat.com>

On Mon, Jul 20, 2015 at 07:33:11PM +0200, Oleg Nesterov wrote:
> Hi Jeff,
> 
> On 07/20, Jeff Moyer wrote:
> >
> > Hi, Oleg,
> >
> > Oleg Nesterov <oleg@redhat.com> writes:
> >
> > > Shouldn't we account aio events/pages somehow, say per-user, or in
> > > mm->pinned_vm ?
> >
> > Ages ago I wrote a patch to account the completion ring to a process'
> > memlock limit:
> >   "[patch] aio: remove aio-max-nr and instead use the memlock rlimit to
> >    limit the number of pages pinned for the aio completion ring"
> >   http://marc.info/?l=linux-aio&m=123661380807041&w=2
> >
> > The problem with that patch is that it modifies the user/kernel
> > interface.  It could be done over time, as Andrew outlined in that
> > thread, but I've been reluctant to take that on.
> 
> See also the usage of mm->pinned_vm and user->locked_vm in perf_mmap(),
> perhaps aio can do the same...
> 
> > If you just mean we should account the memory so that the right process
> > can be killed, that sounds like a good idea to me.
> 
> Not sure we actually need this. I only meant that this looks confusing
> because this memory is actually locked but the kernel doesn't know this.
> 
> And btw, I forgot to mention that I triggered OOM on the testing machine
> with only 512mb ram, and aio-max-nr was huge. So, once again, while this
> all doesn't look right to me, I do not think this is the real problem.
> 
> Except the fact that an unpriviliged user can steal all aio-max-nr events.
> This probably worth fixing in any case.
> 
> 
> 
> And if we accept the fact this memory is locked and if we properly account
> it, then may be we can just kill aio_migratepage(), aio_private_file(), and
> change aio_setup_ring() to simply use install_special_mapping(). This will
> greatly simplify the code. But let me remind that I know nothing about aio,
> so please don't take my thoughts seriously.

No, you can't get rid of that code.  The page migration is required when 
CPUs/memory is offlined and data needs to be moved to another node.  
Similarly, support for mremap() is also required for container migration / 
restoration.

As for accounting locked memory, we don't do that for memory pinned by 
O_DIRECT either.  Given how small the amount of memory aio can pin is 
compared to O_DIRECT or mlock(), it is unlikely that the accounting of 
how much aio has pinned will make any real difference in the big picture.  
A single O_DIRECT i/o can pin megabytes of memory.

		-ben

> Oleg.

-- 
"Thought is the essence of where you are now."

  reply	other threads:[~2015-07-20 17:51 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-16 23:14 [PATCH] mm-move-mremap-from-file_operations-to-vm_operations_struct-fix Oleg Nesterov
2015-07-16 23:22 ` Stephen Rothwell
2015-07-16 23:24 ` Andrew Morton
2015-07-16 23:52   ` Oleg Nesterov
2015-07-17 14:06     ` Benjamin LaHaise
2015-07-17 17:27       ` Oleg Nesterov
2015-07-17 17:37         ` Benjamin LaHaise
2015-07-17 17:55           ` Oleg Nesterov
2015-07-17 18:12             ` Austin S Hemmelgarn
2015-07-17 18:19               ` Oleg Nesterov
2015-07-17 18:39                 ` Austin S Hemmelgarn
2015-07-17 18:54                   ` Oleg Nesterov
2015-07-17 19:09                     ` Austin S Hemmelgarn
2015-07-17 22:56             ` Oleg Nesterov
2015-07-17 22:31       ` Oleg Nesterov
2015-07-20 14:22         ` Jeff Moyer
2015-07-20 17:33           ` Oleg Nesterov
2015-07-20 17:51             ` Benjamin LaHaise [this message]
2015-07-20 18:30               ` Jeff Moyer
2015-07-20 18:31               ` Oleg Nesterov
2015-07-20 19:24                 ` Oleg Nesterov
2015-07-20 19:39                   ` Benjamin LaHaise
2015-07-20 20:03                     ` Oleg Nesterov
2015-07-21 15:29 ` [PATCH v2] mm-move-mremap-from-file_operations-to-vm_operations_struct-fix Oleg Nesterov
2015-07-21 15:38   ` Benjamin LaHaise
2015-07-21 16:18     ` Oleg Nesterov
2015-07-21 16:20 ` [PATCH v3] mm-move-mremap-from-file_operations-to-vm_operations_struct-fix Oleg Nesterov

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=20150720175145.GH21558@kvack.org \
    --to=bcrl@kvack.org \
    --cc=akpm@linux-foundation.org \
    --cc=fengguang.wu@intel.com \
    --cc=hannes@cmpxchg.org \
    --cc=jmoyer@redhat.com \
    --cc=js1304@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-next@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=sfr@canb.auug.org.au \
    /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.