* Re: [PATCH 0/8] Use correctly the Xen memory terminologies in Linux
From: Julien Grall @ 2015-07-29 11:02 UTC (permalink / raw)
To: xen-devel
Cc: ian.campbell, stefano.stabellini, linux-kernel, Boris Ostrovsky,
David Vrabel, Dmitry Torokhov, Greg Kroah-Hartman, H. Peter Anvin,
Ingo Molnar, James E.J. Bottomley,
Jean-Christophe Plagniol-Villard, Jiri Slaby, Juergen Gross,
Konrad Rzeszutek Wilk, linux-api, linux-arm-kernel, linux-fbdev,
linux-input, linuxppc-dev, linux-scsi, netdev,
Roger Pau Monné, Russell King
In-Reply-To: <1438095769-2560-1-git-send-email-julien.grall@citrix.com>
On 28/07/15 16:02, Julien Grall wrote:
> Hi all,
>
> This patch series aims to use the memory terminologies described in
> include/linux/mm.h [1] for Linux xen code.
I mistakenly wrote the wrong include here. It should be include/xen/mm.h
from the Xen tree:
http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=e758ed14f390342513405dd766e874934573e6cb
Regards,
--
Julien Grall
^ permalink raw reply
* Re: [PATCH v7 4/6] block: loop: prepare for supporing direct IO
From: Ming Lei @ 2015-07-29 11:21 UTC (permalink / raw)
To: Dave Chinner
Cc: Christoph Hellwig, Jens Axboe, Linux Kernel Mailing List,
Justin M. Forbes, Jeff Moyer, Tejun Heo,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150729084102.GB16638@dastard>
On Wed, Jul 29, 2015 at 4:41 AM, Dave Chinner <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org> wrote:
> On Wed, Jul 29, 2015 at 03:33:52AM -0400, Ming Lei wrote:
>> On Mon, Jul 27, 2015 at 1:33 PM, Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
>> > On Mon, Jul 27, 2015 at 05:53:33AM -0400, Ming Lei wrote:
>> >> Because size has to be 4k aligned too.
>> >
>> > Yes. But again I don't see any reason to limit us to a hardcoded 512
>> > byte block size here, especially considering the patches to finally
>>
>> From loop block's view, the request size can be any count of 512-byte
>> sectors, then the transfer size to backing device can't guarantee to be
>> 4k aligned always.
>
> In theory, yes. In practise, doesn't happen very often.
>
>> > allow enabling other block sizes from userspace.
>>
>> I have some questions about the patchset, and looks the author doesn't
>> reply it yet.
>>
>> On Mon, Jul 27, 2015 at 6:06 PM, Dave Chinner <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org> wrote:
>> >> Because size has to be 4k aligned too.
>> >
>> > So check that, too. Any >= 4k block size filesystem should be doing
>> > mostly 4k aligned and sized IO...
>>
>> I guess you mean we only use direct IO for the 4k aligned and sized IO?
>> If so, that won't be efficient because the page cache has to be flushed
>> during the switch.
>
> It will be extremely rare for a 4k block size filesystem to do
> anything other than 4k aligned and sized IO. Think about it for a
> minute: what does the page cache do to unaligned IO patterns (i.e.
> buffered IO)? It does IO in page sizes, and so if the application
> if doing badly aligned or sized IO with buffered IO, then the
> underlying device will only ever size page sized and aligned IO.
>
> Hence sector aligned IO will only come from applications doing
> direct IO. If the application is doing direct IO and it's not
> properly aligned, then it already is going to get sucky performance
> because most filesystem serialise sub-block size direct IO because
> concurrent sub-block IOs to the same block usually leads to data
> corruption.
The blocksize of filesysten over loop can be 512, 1024, 2048, and
suppose sector size of backing device is 4096, then filesystem
can see aligned direct IO when IO size/offset from application is aligned
with fs block size, but loop still can't do direct IO for all this
kind of requests
against backing file.
Another case is that application may access loop block directly, such
as 'dd if=/dev/loopN', but it may not be common, and maybe it needn't
to consider.
Thanks,
>
> So, really, sector aligned/sized direct IO is a sucky performance
> path before we even get to the loop device, so we don't really need
> to care how fast the loop device handles this case. The loop device
> just needs to ensure that it doesn't corrupt data when badly aligned
> IOs come in... ;)
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org
^ permalink raw reply
* Re: [RFC PATCH 13/14] kthread_worker: Add set_kthread_worker_user_nice()
From: Petr Mladek @ 2015-07-29 11:23 UTC (permalink / raw)
To: Tejun Heo
Cc: Andrew Morton, Oleg Nesterov, Ingo Molnar, Peter Zijlstra,
Steven Rostedt, Paul E. McKenney, Josh Triplett, Thomas Gleixner,
Linus Torvalds, Jiri Kosina, Borislav Petkov, Michal Hocko,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Vlastimil Babka,
live-patching-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150728174058.GF5322-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
On Tue 2015-07-28 13:40:58, Tejun Heo wrote:
> On Tue, Jul 28, 2015 at 04:39:30PM +0200, Petr Mladek wrote:
> ...
> > +/*
> > + * set_kthread_worker_user_nice - set scheduling priority for the kthread worker
> > + * @worker: target kthread_worker
> > + * @nice: niceness value
> > + */
> > +void set_kthread_worker_user_nice(struct kthread_worker *worker, long nice)
> > +{
> > + struct task_struct *task = worker->task;
> > +
> > + WARN_ON(!task);
> > + set_user_nice(task, nice);
> > +}
> > +EXPORT_SYMBOL(set_kthread_worker_user_nice);
>
> kthread_worker is explcitly associated with a single kthread. Why do
> we want to create explicit wrappers for kthread operations? This is
> encapsulation for encapsulation's sake. It doesn't buy us anything at
> all. Just let the user access the associated kthread and operate on
> it.
My plan is to make the API cleaner and hide struct kthread_worker
definition into kthread.c. It would prevent anyone doing any hacks
with it. BTW, we do the same with struct workqueue_struct.
Another possibility would be to add helper function to get the
associated task struct but this might cause inconsistencies when
the worker is restarted.
Best Regards,
Petr
^ permalink raw reply
* Re: [RFC PATCH 07/14] mm/huge_page: Convert khugepaged() into kthread worker API
From: Petr Mladek @ 2015-07-29 11:32 UTC (permalink / raw)
To: Tejun Heo
Cc: Andrew Morton, Oleg Nesterov, Ingo Molnar, Peter Zijlstra,
Steven Rostedt, Paul E. McKenney, Josh Triplett, Thomas Gleixner,
Linus Torvalds, Jiri Kosina, Borislav Petkov, Michal Hocko,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Vlastimil Babka,
live-patching-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150728173635.GD5322-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
On Tue 2015-07-28 13:36:35, Tejun Heo wrote:
> Hello,
>
> On Tue, Jul 28, 2015 at 04:39:24PM +0200, Petr Mladek wrote:
> > -static void khugepaged_wait_work(void)
> > +static void khugepaged_wait_func(struct kthread_work *dummy)
> > {
> > if (khugepaged_has_work()) {
> > if (!khugepaged_scan_sleep_millisecs)
> > - return;
> > + goto out;
> >
> > wait_event_freezable_timeout(khugepaged_wait,
> > - kthread_should_stop(),
> > + !khugepaged_enabled(),
> > msecs_to_jiffies(khugepaged_scan_sleep_millisecs));
> > - return;
> > + goto out;
> > }
> >
> > if (khugepaged_enabled())
> > wait_event_freezable(khugepaged_wait, khugepaged_wait_event());
> > +
> > +out:
> > + if (khugepaged_enabled())
> > + queue_kthread_work(&khugepaged_worker,
> > + &khugepaged_do_scan_work);
> > }
>
> There gotta be a better way to do this. It's outright weird to
> convert it over to work item based interface and then handle idle
> periods by injecting wait work items. If there's an external event
> which wakes up the worker, convert that to a queueing event. If it's
> a timed event, implement a delayed work and queue that with delay.
I am going to give it a try.
Best Regards,
Petr
^ permalink raw reply
* Re: Next round: revised futex(2) man page for review
From: Thomas Gleixner @ 2015-07-29 12:00 UTC (permalink / raw)
To: Darren Hart
Cc: Michael Kerrisk (man-pages), Torvald Riegel, Carlos O'Donell,
Ingo Molnar, Jakub Jelinek, linux-man, lkml, Davidlohr Bueso,
Arnd Bergmann, Steven Rostedt, Peter Zijlstra, Linux API,
Roland McGrath, Anton Blanchard, Eric Dumazet, bill o gallmeister,
Jan Kiszka, Daniel Wagner, Rich Felker, Andy Lutomirski,
bert hubert, Rusty Russell, Heinrich Schuchardt
In-Reply-To: <20150729042141.GA62059@vmdeb7>
On Tue, 28 Jul 2015, Darren Hart wrote:
> Found it on libc-alpha, here it is for reference:
>
> From: Rich Felker <dalias-8zAoT0mYgF4@public.gmane.org>
> Date: Wed, 29 Oct 2014 22:43:17 -0400
> To: Darren Hart <dvhart-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
> Cc: Carlos O'Donell <carlos-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>, Roland McGrath <roland-/Z5OmTQCD9xF6kxbq+BtvQ@public.gmane.org>,
> Torvald Riegel <triegel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>, GLIBC Devel <libc-alpha-9JcytcrH/bA+uJoB2kUjGw@public.gmane.org>,
> Michael Kerrisk <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Subject: Re: Add futex wrapper to glibc?
>
> On Wed, Oct 29, 2014 at 06:59:15PM -0700, Darren Hart wrote:
> > > We are IMO at the stage where futex is stable, few things are
> > > changing, and with documentation in place, I would consider adding a
> > > futex wrapper.
> >
> > Yes, at least for the defined OP codes. New OPs may be added of
> > course, but that isn't a concern for supporting what exists today, and
> > doesn't break compatibility.
> >
> > I wonder though... can we not wrap FUTEX_REQUEUE? It's fundamentally
> > broken. FUTEX_CMP_REQUEUE should *always* be used instead. The glibc
> > wrapper is one way to encourage developers to do the right thing
> > (don't expose the bad op in the header).
>
> You're mistaken here. There are plenty of valid ways to use
> FUTEX_REQUEUE - for example if the calling thread is requeuing the
> target(s) to a lock that the calling thread owns. Just because it
> doesn't meet the needs of the way glibc was using it internally
> doesn't mean it's useless for other applications.
>
> In any case, I don't think there's a proposal to intercept/modify the
> commands to futex, just to pass them through (and possibly do a
> cancellable syscall for some of them).
Fair enough. Did not think about the requeue to futex held by the
caller case. In that case FUTEX_REQUEUE works as advertised.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH 0/4] enable migration of driver pages
From: Mel Gorman @ 2015-07-29 12:16 UTC (permalink / raw)
To: Gioh Kim, jlayton, bfields, vbabka, iamjoonsoo.kim, viro, mst,
koct9i, minchan, aquini, linux-fsdevel, virtualization,
linux-kernel, linux-api, linux-mm, dri-devel, akpm, Gioh Kim
In-Reply-To: <20150729105554.GU16722@phenom.ffwll.local>
On Wed, Jul 29, 2015 at 12:55:54PM +0200, Daniel Vetter wrote:
> On Wed, Jul 29, 2015 at 11:49:45AM +0100, Mel Gorman wrote:
> > On Mon, Jul 13, 2015 at 05:35:15PM +0900, Gioh Kim wrote:
> > > My ARM-based platform occured severe fragmentation problem after long-term
> > > (several days) test. Sometimes even order-3 page allocation failed. It has
> > > memory size 512MB ~ 1024MB. 30% ~ 40% memory is consumed for graphic processing
> > > and 20~30 memory is reserved for zram.
> > >
> >
> > The primary motivation of this series is to reduce fragmentation by allowing
> > more kernel pages to be moved. Conceptually that is a worthwhile goal but
> > there should be at least one major in-kernel user and while balloon
> > pages were a good starting point, I think we really need to see what the
> > zram changes look like at the same time.
>
> I think gpu drivers really would be the perfect candidate for compacting
> kernel page allocations. And this also seems the primary motivation for
> this patch series, so I think that's really what we should use to judge
> these patches.
>
> Of course then there's the seemingly eternal chicken/egg problem of
> upstream gpu drivers for SoCs :(
I recognised that the driver he had modified was not an in-tree user so
it did not really help the review or the design. I did not think it was
very fair to ask that an in-tree GPU driver be converted when it would not
help the embedded platform of interest. Converting zram is both a useful
illustration of the aops requirements and is expected to be beneficial on
the embedded platform. Now, if a GPU driver author was willing to convert
theirs as an example then that would be useful!
--
Mel Gorman
SUSE Labs
--
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>
^ permalink raw reply
* Re: [PATCH -mm v9 0/8] idle memory tracking
From: Michal Hocko @ 2015-07-29 12:36 UTC (permalink / raw)
To: Vladimir Davydov
Cc: Andrew Morton, Andres Lagar-Cavilla, Minchan Kim, Raghavendra K T,
Johannes Weiner, Greg Thelen, Michel Lespinasse, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet, linux-api,
linux-doc, linux-mm, cgroups, linux-kernel
In-Reply-To: <cover.1437303956.git.vdavydov@parallels.com>
On Sun 19-07-15 15:31:09, Vladimir Davydov wrote:
[...]
> ---- USER API ----
>
> The user API consists of two new proc files:
I was thinking about this for a while. I dislike the interface. It is
quite awkward to use - e.g. you have to read the full memory to check a
single memcg idleness. This might turn out being a problem especially on
large machines. It also provides a very low level information (per-pfn
idleness) which is inherently racy. Does anybody really require this
level of detail?
I would assume that most users are interested only in a single number
which tells the idleness of the system/memcg. Well, you have mentioned
a per-process reclaim but I am quite skeptical about this.
I guess the primary reason to rely on the pfn rather than the LRU walk,
which would be more targeted (especially for memcg cases), is that we
cannot hold lru lock for the whole LRU walk and we cannot continue
walking after the lock is dropped. Maybe we can try to address that
instead? I do not think this is easy to achieve but have you considered
that as an option?
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH 0/4] enable migration of driver pages
From: Daniel Vetter @ 2015-07-29 12:46 UTC (permalink / raw)
To: Mel Gorman
Cc: aquini, mst, linux-api, linux-kernel, dri-devel, virtualization,
bfields, minchan, Gioh Kim, linux-mm, viro, akpm, Gioh Kim,
linux-fsdevel, jlayton, koct9i, iamjoonsoo.kim, vbabka
In-Reply-To: <20150729121614.GA19352@techsingularity.net>
On Wed, Jul 29, 2015 at 01:16:14PM +0100, Mel Gorman wrote:
> On Wed, Jul 29, 2015 at 12:55:54PM +0200, Daniel Vetter wrote:
> > On Wed, Jul 29, 2015 at 11:49:45AM +0100, Mel Gorman wrote:
> > > On Mon, Jul 13, 2015 at 05:35:15PM +0900, Gioh Kim wrote:
> > > > My ARM-based platform occured severe fragmentation problem after long-term
> > > > (several days) test. Sometimes even order-3 page allocation failed. It has
> > > > memory size 512MB ~ 1024MB. 30% ~ 40% memory is consumed for graphic processing
> > > > and 20~30 memory is reserved for zram.
> > > >
> > >
> > > The primary motivation of this series is to reduce fragmentation by allowing
> > > more kernel pages to be moved. Conceptually that is a worthwhile goal but
> > > there should be at least one major in-kernel user and while balloon
> > > pages were a good starting point, I think we really need to see what the
> > > zram changes look like at the same time.
> >
> > I think gpu drivers really would be the perfect candidate for compacting
> > kernel page allocations. And this also seems the primary motivation for
> > this patch series, so I think that's really what we should use to judge
> > these patches.
> >
> > Of course then there's the seemingly eternal chicken/egg problem of
> > upstream gpu drivers for SoCs :(
>
> I recognised that the driver he had modified was not an in-tree user so
> it did not really help the review or the design. I did not think it was
> very fair to ask that an in-tree GPU driver be converted when it would not
> help the embedded platform of interest. Converting zram is both a useful
> illustration of the aops requirements and is expected to be beneficial on
> the embedded platform. Now, if a GPU driver author was willing to convert
> theirs as an example then that would be useful!
Well my concern is more with merging infrastructure to upstream for
drivers which aren't upstream and with no plan to make that happen anytime
soon. Seems like just offload a bit to me ... but in the end core mm isn't
my thing so not my decision.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH v3 01/11] stm class: Introduce an abstraction for System Trace Module devices
From: Alexander Shishkin @ 2015-07-29 13:25 UTC (permalink / raw)
To: Chunyan Zhang
Cc: Greg Kroah-Hartman, linux-kernel@vger.kernel.org, Mathieu Poirier,
peter.lachner-ral2JQCrhuEAvxtiuMwx3w,
norbert.schulz-ral2JQCrhuEAvxtiuMwx3w,
keven.boell-ral2JQCrhuEAvxtiuMwx3w,
yann.fouassier-ral2JQCrhuEAvxtiuMwx3w,
laurent.fert-ral2JQCrhuEAvxtiuMwx3w, linux-api@vger.kernel.org,
Chunyan Zhang, Mark Brown
In-Reply-To: <CAAfSe-vOgnMXCy6fb8yuQ0O_5x1L1QtCmGjNjH+OtGAg6KJHrg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Chunyan Zhang <zhang.lyra-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>> +/**
>> + * stm_source_register_device() - register an stm_source device
>> + * @parent: parent device
>> + * @data: device description structure
>> + *
>> + * This will create a device of stm_source class that can write
>> + * data to an stm device once linked.
>> + *
>> + * Return: 0 on success, -errno otherwise.
>> + */
>> +int stm_source_register_device(struct device *parent,
>> + struct stm_source_data *data)
>> +{
>> + struct stm_source_device *src;
>> + int err;
>> +
>> + if (!stm_core_up)
>> + return -EPROBE_DEFER;
>> +
>
> I tried to update Coresight-stm driver[1] based on your this version
> patch, but the Coresight-stm driver probe() failed.
> the reason was:
> In the end of Coresight stm_probe(), we called this function, but
> "stm_core_up" was zero then, so the error returned value
> "-EPROBE_DEFER" was received.
Yes, that is the intended behavior if stm core is not initialized yet.
> In fact, "stm_core_up" would increase itself until "stm_core_init" be
> called - it's the root of this problem, I'll explain this where the
> function "stm_core_init" defined.
I'm sorry, I didn't understand this, can you rephrase?
> And redoing Coresight stm_probe() will incur a WARN_ON() like below:
>
> [ 1.075746] coresight-stm 10006000.stm: stm_register_device failed
> [ 1.082118] ------------[ cut here ]------------
> [ 1.086819] WARNING: CPU: 1 PID: 1 at drivers/clk/clk.c:657
> clk_core_disable+0x138/0x13c()
> [ 1.095353] Modules linked in:
> [ 1.098487] CPU: 1 PID: 1 Comm: swapper/0 Tainted: G S
> 4.2.0-rc1+ #107
> [ 1.106398] Hardware name: Spreadtrum SC9836 Openphone Board (DT)
> [ 1.112678] Call trace:
> [ 1.115194] [<ffffffc00008a5b4>] dump_backtrace+0x0/0x138
> [ 1.120761] [<ffffffc00008a708>] show_stack+0x1c/0x28
> [ 1.125972] [<ffffffc0003320e0>] dump_stack+0x84/0xc8
> [ 1.131179] [<ffffffc00009b580>] warn_slowpath_common+0xa4/0xdc
> [ 1.137285] [<ffffffc00009b700>] warn_slowpath_null+0x34/0x44
> [ 1.143213] [<ffffffc000321eb4>] clk_core_disable+0x134/0x13c
Well, like I said in the offline thread, this has to do with cleaning up
in the error path of stm_probe(). What happens if stm_probe() fails for
any other reason? I'm guessing the same warning.
>> +static int __init stm_core_init(void)
>> +{
>> + int err;
>> +
>> + err = class_register(&stm_class);
>> + if (err)
>> + return err;
>> +
>> + err = class_register(&stm_source_class);
>> + if (err)
>> + goto err_stm;
>> +
>> + err = stp_configfs_init();
>> + if (err)
>> + goto err_src;
>> +
>> + init_srcu_struct(&stm_source_srcu);
>> +
>> + stm_core_up++;
>> +
>> + return 0;
>> +
>> +err_src:
>> + class_unregister(&stm_source_class);
>> +err_stm:
>> + class_unregister(&stm_class);
>> +
>> + return err;
>> +}
>> +
>> +module_init(stm_core_init);
>
> Since you are using module_init() instead of postcore_initcall() which
> was in the last version patch, as such, this function would be
> executed after Coresight "stm_probe" finished.
Yes, iirc on arm the initcall order somehow forced postcore
stm_core_init() before configfs, which it relies on, causing a
crash. Now I see that somebody hacked configfs to start at core_initcall
(f5b697700c8) instead.
There has to be a way to defer stm_probe(), although a quick look at
amba code suggests it's not implemented.
> So, we think there a few optional solutions:
> 1) Remove the "stm_register_device" out from Coresight "stm_probe",
> but we have to save another global variable:
>
> struct device *stm_dev;
>
> in the process of Coresight "stm_probe".
Sorry, didn't understand this one.
Except for I can say that having a global variable like that is a bad
idea, but that's not relevant to the problem at hand.
> 2) Change module_init() to other XYX_init() which would run prior to
> "amba_probe()" (i.e. the caller of Coresight stm_probe), this may be a
> better one.
I'm really not a big fan of the initcall games, to be honest, it will
always be a problem on some architecture or other. Having said that, if
stm_core_init() runs at postcore_initcall level, does that solve your
problem?
> 3) stm_core_init() could be turned into a library call where
> initialisation of the internals is done when first called.
Well, it's not that simple: stm is used by both stm and stm_source
devices, in this case we'll need to make sure that the first call to
either of the {stm,stm_source}_register_device() results in the actual
initialization of the stm core. I think it's a cleaner solution than the
initcall games, though.
Regards,
--
Alex
^ permalink raw reply
* Re: [PATCH v3 01/11] stm class: Introduce an abstraction for System Trace Module devices
From: Mark Brown @ 2015-07-29 13:35 UTC (permalink / raw)
To: Alexander Shishkin
Cc: Chunyan Zhang, Greg Kroah-Hartman,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Mathieu Poirier, peter.lachner-ral2JQCrhuEAvxtiuMwx3w,
norbert.schulz-ral2JQCrhuEAvxtiuMwx3w,
keven.boell-ral2JQCrhuEAvxtiuMwx3w,
yann.fouassier-ral2JQCrhuEAvxtiuMwx3w,
laurent.fert-ral2JQCrhuEAvxtiuMwx3w,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Chunyan Zhang
In-Reply-To: <877fpjkseh.fsf-qxRn5AmX6ZD9BXuAQUXR0fooFf0ArEBIu+b9c/7xato@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 337 bytes --]
On Wed, Jul 29, 2015 at 04:25:10PM +0300, Alexander Shishkin wrote:
> There has to be a way to defer stm_probe(), although a quick look at
> amba code suggests it's not implemented.
What makes you say this? Probe deferral is implemented in the driver
core rather than individual buses, the buses don't need to know anything
about it.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCH v3 01/11] stm class: Introduce an abstraction for System Trace Module devices
From: Alexander Shishkin @ 2015-07-29 13:46 UTC (permalink / raw)
To: Mark Brown
Cc: Chunyan Zhang, Greg Kroah-Hartman, linux-kernel@vger.kernel.org,
Mathieu Poirier, peter.lachner-ral2JQCrhuEAvxtiuMwx3w,
norbert.schulz-ral2JQCrhuEAvxtiuMwx3w,
keven.boell-ral2JQCrhuEAvxtiuMwx3w,
yann.fouassier-ral2JQCrhuEAvxtiuMwx3w,
laurent.fert-ral2JQCrhuEAvxtiuMwx3w, linux-api@vger.kernel.org,
Chunyan Zhang
In-Reply-To: <20150729133529.GI20130-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> writes:
> On Wed, Jul 29, 2015 at 04:25:10PM +0300, Alexander Shishkin wrote:
>
>> There has to be a way to defer stm_probe(), although a quick look at
>> amba code suggests it's not implemented.
>
> What makes you say this? Probe deferral is implemented in the driver
> core rather than individual buses, the buses don't need to know anything
> about it.
I stand corrected, it indeed is.
So returning EPROBE_DEFER from stm_probe() should Just Work (provided
stm_probe() handles its error paths correctly).
Regards,
--
Alex
^ permalink raw reply
* Re: [PATCH -mm v9 0/8] idle memory tracking
From: Vladimir Davydov @ 2015-07-29 13:59 UTC (permalink / raw)
To: Michal Hocko
Cc: Andrew Morton, Andres Lagar-Cavilla, Minchan Kim, Raghavendra K T,
Johannes Weiner, Greg Thelen, Michel Lespinasse, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150729123629.GI15801-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org>
On Wed, Jul 29, 2015 at 02:36:30PM +0200, Michal Hocko wrote:
> On Sun 19-07-15 15:31:09, Vladimir Davydov wrote:
> [...]
> > ---- USER API ----
> >
> > The user API consists of two new proc files:
>
> I was thinking about this for a while. I dislike the interface. It is
> quite awkward to use - e.g. you have to read the full memory to check a
> single memcg idleness. This might turn out being a problem especially on
> large machines.
Yes, with this API estimating the wss of a single memory cgroup will
cost almost as much as doing this for the whole system.
Come to think of it, does anyone really need to estimate idleness of one
particular cgroup? If we are doing this for finding an optimal memcg
limits configuration or while considering a load move within a cluster
(which I think are the primary use cases for the feature), we must do it
system-wide to see the whole picture.
> It also provides a very low level information (per-pfn idleness) which
> is inherently racy. Does anybody really require this level of detail?
Well, one might want to do it per-process, obtaining PFNs from
/proc/pid/pagemap.
>
> I would assume that most users are interested only in a single number
> which tells the idleness of the system/memcg.
Yes, that's what I need it for - estimating containers' wss for setting
their limits accordingly.
> Well, you have mentioned a per-process reclaim but I am quite
> skeptical about this.
This is what Minchan mentioned initially. Personally, I'm not going to
use it per-process, but I wouldn't rule out this use case either.
>
> I guess the primary reason to rely on the pfn rather than the LRU walk,
> which would be more targeted (especially for memcg cases), is that we
> cannot hold lru lock for the whole LRU walk and we cannot continue
> walking after the lock is dropped. Maybe we can try to address that
> instead? I do not think this is easy to achieve but have you considered
> that as an option?
Yes, I have, and I've come to a conclusion it's not doable, because LRU
lists can be constantly rotating at an arbitrary rate. If you have an
idea in mind how this could be done, please share.
Speaking of LRU-vs-PFN walk, iterating over PFNs has its own advantages:
- You can distribute a walk in time to avoid CPU bursts.
- You are free to parallelize the scanner as you wish to decrease the
scan time.
Thanks,
Vladimir
^ permalink raw reply
* Re: [PATCH -mm v9 0/8] idle memory tracking
From: Michel Lespinasse @ 2015-07-29 14:12 UTC (permalink / raw)
To: Vladimir Davydov
Cc: Michal Hocko, Andrew Morton, Andres Lagar-Cavilla, Minchan Kim,
Raghavendra K T, Johannes Weiner, Greg Thelen, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet, linux-api,
linux-doc, linux-mm, cgroups, linux-kernel
In-Reply-To: <20150729135907.GT8100@esperanza>
[-- Attachment #1: Type: text/plain, Size: 1843 bytes --]
On Wed, Jul 29, 2015 at 6:59 AM, Vladimir Davydov <vdavydov@parallels.com>
wrote:
>> I guess the primary reason to rely on the pfn rather than the LRU walk,
>> which would be more targeted (especially for memcg cases), is that we
>> cannot hold lru lock for the whole LRU walk and we cannot continue
>> walking after the lock is dropped. Maybe we can try to address that
>> instead? I do not think this is easy to achieve but have you considered
>> that as an option?
>
> Yes, I have, and I've come to a conclusion it's not doable, because LRU
> lists can be constantly rotating at an arbitrary rate. If you have an
> idea in mind how this could be done, please share.
>
> Speaking of LRU-vs-PFN walk, iterating over PFNs has its own advantages:
> - You can distribute a walk in time to avoid CPU bursts.
> - You are free to parallelize the scanner as you wish to decrease the
> scan time.
There is a third way: one could go through every MM in the system and scan
their page tables. Doing things that way turns out to be generally faster
than scanning by physical address, because you don't have to go through
RMAP for every page. But, you end up needing to take the mmap_sem lock of
every MM (in turn) while scanning them, and that degrades quickly under
memory load, which is exactly when you most need this feature. So, scan by
address is still what we use here.
My only concern about the interface is that it exposes the fact that the
scan is done by address - if the interface only showed per-memcg totals, it
would make it possible to change the implementation underneath if we
somehow figure out how to work around the mmap_sem issue in the future. I
don't think that is necessarily a blocker but this is something to keep in
mind IMO.
--
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
[-- Attachment #2: Type: text/html, Size: 2060 bytes --]
^ permalink raw reply
* Re: [PATCH -mm v9 0/8] idle memory tracking
From: Michel Lespinasse @ 2015-07-29 14:13 UTC (permalink / raw)
To: Vladimir Davydov
Cc: Michal Hocko, Andrew Morton, Andres Lagar-Cavilla, Minchan Kim,
Raghavendra K T, Johannes Weiner, Greg Thelen, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CANN689HJX2ZL891uOd8TW9ct4PNH9d5odQZm86WMxkpkCWhA-w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
(resending as text, sorry for previous post which didn't make it to the ML)
On Wed, Jul 29, 2015 at 7:12 AM, Michel Lespinasse <walken-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
>
> On Wed, Jul 29, 2015 at 6:59 AM, Vladimir Davydov <vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> wrote:
> >> I guess the primary reason to rely on the pfn rather than the LRU walk,
> >> which would be more targeted (especially for memcg cases), is that we
> >> cannot hold lru lock for the whole LRU walk and we cannot continue
> >> walking after the lock is dropped. Maybe we can try to address that
> >> instead? I do not think this is easy to achieve but have you considered
> >> that as an option?
> >
> > Yes, I have, and I've come to a conclusion it's not doable, because LRU
> > lists can be constantly rotating at an arbitrary rate. If you have an
> > idea in mind how this could be done, please share.
> >
> > Speaking of LRU-vs-PFN walk, iterating over PFNs has its own advantages:
> > - You can distribute a walk in time to avoid CPU bursts.
> > - You are free to parallelize the scanner as you wish to decrease the
> > scan time.
>
> There is a third way: one could go through every MM in the system and scan their page tables. Doing things that way turns out to be generally faster than scanning by physical address, because you don't have to go through RMAP for every page. But, you end up needing to take the mmap_sem lock of every MM (in turn) while scanning them, and that degrades quickly under memory load, which is exactly when you most need this feature. So, scan by address is still what we use here.
>
> My only concern about the interface is that it exposes the fact that the scan is done by address - if the interface only showed per-memcg totals, it would make it possible to change the implementation underneath if we somehow figure out how to work around the mmap_sem issue in the future. I don't think that is necessarily a blocker but this is something to keep in mind IMO.
--
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
^ permalink raw reply
* Re: [PATCH -mm v9 0/8] idle memory tracking
From: Michal Hocko @ 2015-07-29 14:26 UTC (permalink / raw)
To: Vladimir Davydov
Cc: Andrew Morton, Andres Lagar-Cavilla, Minchan Kim, Raghavendra K T,
Johannes Weiner, Greg Thelen, Michel Lespinasse, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet, linux-api,
linux-doc, linux-mm, cgroups, linux-kernel
In-Reply-To: <20150729135907.GT8100@esperanza>
On Wed 29-07-15 16:59:07, Vladimir Davydov wrote:
> On Wed, Jul 29, 2015 at 02:36:30PM +0200, Michal Hocko wrote:
> > On Sun 19-07-15 15:31:09, Vladimir Davydov wrote:
> > [...]
> > > ---- USER API ----
> > >
> > > The user API consists of two new proc files:
> >
> > I was thinking about this for a while. I dislike the interface. It is
> > quite awkward to use - e.g. you have to read the full memory to check a
> > single memcg idleness. This might turn out being a problem especially on
> > large machines.
>
> Yes, with this API estimating the wss of a single memory cgroup will
> cost almost as much as doing this for the whole system.
>
> Come to think of it, does anyone really need to estimate idleness of one
> particular cgroup?
It is certainly interesting for setting the low limit.
> If we are doing this for finding an optimal memcg
> limits configuration or while considering a load move within a cluster
> (which I think are the primary use cases for the feature), we must do it
> system-wide to see the whole picture.
>
> > It also provides a very low level information (per-pfn idleness) which
> > is inherently racy. Does anybody really require this level of detail?
>
> Well, one might want to do it per-process, obtaining PFNs from
> /proc/pid/pagemap.
Sure once the interface is exported you can do whatever ;) But my
question is whether any real usecase _requires_ it.
> > I would assume that most users are interested only in a single number
> > which tells the idleness of the system/memcg.
>
> Yes, that's what I need it for - estimating containers' wss for setting
> their limits accordingly.
So why don't we export the single per memcg and global knobs then?
This would have few advantages. First of all it would be much easier to
use, you wouldn't have to export memcg ids and finally the implementation
could be changed without any user visible changes (e.g. lru vs. pfn walks),
potential caching and who knows what. In other words. Michel had a
single number interface AFAIR, what was the primary reason to move away
from that API?
> > Well, you have mentioned a per-process reclaim but I am quite
> > skeptical about this.
>
> This is what Minchan mentioned initially. Personally, I'm not going to
> use it per-process, but I wouldn't rule out this use case either.
Considering how many times we have been bitten by too broad interfaces I
would rather be conservative.
> > I guess the primary reason to rely on the pfn rather than the LRU walk,
> > which would be more targeted (especially for memcg cases), is that we
> > cannot hold lru lock for the whole LRU walk and we cannot continue
> > walking after the lock is dropped. Maybe we can try to address that
> > instead? I do not think this is easy to achieve but have you considered
> > that as an option?
>
> Yes, I have, and I've come to a conclusion it's not doable, because LRU
> lists can be constantly rotating at an arbitrary rate. If you have an
> idea in mind how this could be done, please share.
Yes this is really tricky with the current LRU implementation. I
was playing with some ideas (do some checkpoints on the way) but
none of them was really working out on a busy systems. But the LRU
implementation might change in the future. I didn't mean this as a hard
requirement it just sounds that the current implementation restrictions
shape the user visible API which is a good sign to think twice about it.
> Speaking of LRU-vs-PFN walk, iterating over PFNs has its own advantages:
> - You can distribute a walk in time to avoid CPU bursts.
This would make the information even more volatile. I am not sure how
helpful it would be in the end.
> - You are free to parallelize the scanner as you wish to decrease the
> scan time.
This is true but you could argue similar with per-node/lru threads if this
was implemented in the kernel and really needed. I am not sure it would
be really needed though. I would expect this would be a low priority
thing.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH -mm v9 0/8] idle memory tracking
From: Vladimir Davydov @ 2015-07-29 14:45 UTC (permalink / raw)
To: Michel Lespinasse
Cc: Michal Hocko, Andrew Morton, Andres Lagar-Cavilla, Minchan Kim,
Raghavendra K T, Johannes Weiner, Greg Thelen, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CANN689HJX2ZL891uOd8TW9ct4PNH9d5odQZm86WMxkpkCWhA-w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Jul 29, 2015 at 07:12:13AM -0700, Michel Lespinasse wrote:
> On Wed, Jul 29, 2015 at 6:59 AM, Vladimir Davydov <vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> wrote:
> >> I guess the primary reason to rely on the pfn rather than the LRU walk,
> >> which would be more targeted (especially for memcg cases), is that we
> >> cannot hold lru lock for the whole LRU walk and we cannot continue
> >> walking after the lock is dropped. Maybe we can try to address that
> >> instead? I do not think this is easy to achieve but have you considered
> >> that as an option?
> >
> > Yes, I have, and I've come to a conclusion it's not doable, because LRU
> > lists can be constantly rotating at an arbitrary rate. If you have an
> > idea in mind how this could be done, please share.
> >
> > Speaking of LRU-vs-PFN walk, iterating over PFNs has its own advantages:
> > - You can distribute a walk in time to avoid CPU bursts.
> > - You are free to parallelize the scanner as you wish to decrease the
> > scan time.
>
> There is a third way: one could go through every MM in the system and scan
> their page tables. Doing things that way turns out to be generally faster
> than scanning by physical address, because you don't have to go through
> RMAP for every page. But, you end up needing to take the mmap_sem lock of
> every MM (in turn) while scanning them, and that degrades quickly under
> memory load, which is exactly when you most need this feature. So, scan by
> address is still what we use here.
Page table scan approach has the inherent problem - it ignores unmapped
page cache. If a workload does a lot of read/write or map-access-unmap
operations, we won't be able to even roughly estimate its wss.
Thanks,
Vladimir
^ permalink raw reply
* Re: [RFC PATCH 03/14] kthread: Add drain_kthread_worker()
From: Tejun Heo @ 2015-07-29 15:03 UTC (permalink / raw)
To: Petr Mladek
Cc: Andrew Morton, Oleg Nesterov, Ingo Molnar, Peter Zijlstra,
Steven Rostedt, Paul E. McKenney, Josh Triplett, Thomas Gleixner,
Linus Torvalds, Jiri Kosina, Borislav Petkov, Michal Hocko,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Vlastimil Babka,
live-patching-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150729100457.GI2673-KsEp0d+Q8qECVLCxKZUutA@public.gmane.org>
Hello, Petr.
On Wed, Jul 29, 2015 at 12:04:57PM +0200, Petr Mladek wrote:
> > I'm not sure full-on chained work detection is necessary here.
> > kthread worker's usages tend to be significantly simpler and draining
> > is only gonna be used for destruction.
>
> I think that it might be useful to detect bugs when someone
> depends on the worker when it is being destroyed. For example,
> I tried to convert "khubd" kthread and there was not easy to
> double check that this worked as expected.
>
> I actually think about replacing
>
> WARN_ON_ONCE(!is_chained_work(worker)))
>
> with
>
> WARN_ON(!is_chained_work(worker)))
>
> in queue_kthread_work, so that we get the warning for all misused
> workers.
This is a partial soluation no matter what you do especially for
destruction path as there's nothing which prevents draining and
destruction winning the race and then external queueing coming in
afterwards. For use-after-free, slab debug should work pretty well.
I really don't think we need anything special here.
> > > + while (!list_empty(&worker->work_list)) {
> > > + /*
> > > + * Unlock, so we could move forward. Note that queuing
> > > + * is limited by @nr_drainers > 0.
> > > + */
> > > + spin_unlock_irq(&worker->lock);
> > > +
> > > + flush_kthread_worker(worker);
> > > +
> > > + if (++flush_cnt == 10 ||
> > > + (flush_cnt % 100 == 0 && flush_cnt <= 1000))
> > > + pr_warn("kthread worker %s: drain_kthread_worker() isn't complete after %u tries\n",
> > > + worker->task->comm, flush_cnt);
> > > +
> > > + spin_lock_irq(&worker->lock);
> > > + }
> >
> > I'd just do something like WARN_ONCE(flush_cnt++ > 10, "kthread worker: ...").
>
> This would print the warning only for one broken worker. But I do not
> have strong opinion about it.
I really think that'd be a good enough protection here. It's
indicative an outright kernel bug and things tend to go awry and/or
badly reported after the initial failure anyway.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH -mm v9 0/8] idle memory tracking
From: Michel Lespinasse @ 2015-07-29 15:08 UTC (permalink / raw)
To: Vladimir Davydov
Cc: Michal Hocko, Andrew Morton, Andres Lagar-Cavilla, Minchan Kim,
Raghavendra K T, Johannes Weiner, Greg Thelen, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet, linux-api,
linux-doc, linux-mm, cgroups, linux-kernel
In-Reply-To: <20150729144539.GU8100@esperanza>
[-- Attachment #1: Type: text/plain, Size: 452 bytes --]
On Wed, Jul 29, 2015 at 7:45 AM, Vladimir Davydov <vdavydov@parallels.com>
wrote:
> Page table scan approach has the inherent problem - it ignores unmapped
> page cache. If a workload does a lot of read/write or map-access-unmap
> operations, we won't be able to even roughly estimate its wss.
You can catch that in mark_page_accessed on those paths, though.
--
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
[-- Attachment #2: Type: text/html, Size: 557 bytes --]
^ permalink raw reply
* Re: [PATCH -mm v9 0/8] idle memory tracking
From: Michal Hocko @ 2015-07-29 15:08 UTC (permalink / raw)
To: Vladimir Davydov
Cc: Michel Lespinasse, Andrew Morton, Andres Lagar-Cavilla,
Minchan Kim, Raghavendra K T, Johannes Weiner, Greg Thelen,
David Rientjes, Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet,
linux-api, linux-doc, linux-mm, cgroups, linux-kernel
In-Reply-To: <20150729144539.GU8100@esperanza>
On Wed 29-07-15 17:45:39, Vladimir Davydov wrote:
> On Wed, Jul 29, 2015 at 07:12:13AM -0700, Michel Lespinasse wrote:
> > On Wed, Jul 29, 2015 at 6:59 AM, Vladimir Davydov <vdavydov@parallels.com>
> > wrote:
> > >> I guess the primary reason to rely on the pfn rather than the LRU walk,
> > >> which would be more targeted (especially for memcg cases), is that we
> > >> cannot hold lru lock for the whole LRU walk and we cannot continue
> > >> walking after the lock is dropped. Maybe we can try to address that
> > >> instead? I do not think this is easy to achieve but have you considered
> > >> that as an option?
> > >
> > > Yes, I have, and I've come to a conclusion it's not doable, because LRU
> > > lists can be constantly rotating at an arbitrary rate. If you have an
> > > idea in mind how this could be done, please share.
> > >
> > > Speaking of LRU-vs-PFN walk, iterating over PFNs has its own advantages:
> > > - You can distribute a walk in time to avoid CPU bursts.
> > > - You are free to parallelize the scanner as you wish to decrease the
> > > scan time.
> >
> > There is a third way: one could go through every MM in the system and scan
> > their page tables. Doing things that way turns out to be generally faster
> > than scanning by physical address, because you don't have to go through
> > RMAP for every page. But, you end up needing to take the mmap_sem lock of
> > every MM (in turn) while scanning them, and that degrades quickly under
> > memory load, which is exactly when you most need this feature. So, scan by
> > address is still what we use here.
>
> Page table scan approach has the inherent problem - it ignores unmapped
> page cache. If a workload does a lot of read/write or map-access-unmap
> operations, we won't be able to even roughly estimate its wss.
That page cache is trivially reclaimable if it is clean. If it needs
writeback then it is non-idle only until the next writeback. So why does
it matter for the estimation?
--
Michal Hocko
SUSE Labs
--
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>
^ permalink raw reply
* Re: [RFC PATCH 13/14] kthread_worker: Add set_kthread_worker_user_nice()
From: Tejun Heo @ 2015-07-29 15:12 UTC (permalink / raw)
To: Petr Mladek
Cc: Andrew Morton, Oleg Nesterov, Ingo Molnar, Peter Zijlstra,
Steven Rostedt, Paul E. McKenney, Josh Triplett, Thomas Gleixner,
Linus Torvalds, Jiri Kosina, Borislav Petkov, Michal Hocko,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Vlastimil Babka,
live-patching-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150729112354.GK2673-KsEp0d+Q8qECVLCxKZUutA@public.gmane.org>
Hello,
On Wed, Jul 29, 2015 at 01:23:54PM +0200, Petr Mladek wrote:
> My plan is to make the API cleaner and hide struct kthread_worker
> definition into kthread.c. It would prevent anyone doing any hacks
> with it. BTW, we do the same with struct workqueue_struct.
I think obsessive attachment to cleanliness tends to worse code in
general like simple several liner wrappers which don't do anything
other than increasing interface surface and obscuring what's going on.
Let's please take a reasonable trade-off. It shouldn't be nasty but
we don't want to be paying unnecessary complexity for perfect purity
either.
> Another possibility would be to add helper function to get the
> associated task struct but this might cause inconsistencies when
> the worker is restarted.
A kthread_worker would be instantiated on the create call and released
on destroy and the caller is natrually expected to synchronize
creation and destruction against all other operations. Nothing seems
complicated or subtle to me.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH -mm v9 0/8] idle memory tracking
From: Vladimir Davydov @ 2015-07-29 15:28 UTC (permalink / raw)
To: Michal Hocko
Cc: Andrew Morton, Andres Lagar-Cavilla, Minchan Kim, Raghavendra K T,
Johannes Weiner, Greg Thelen, Michel Lespinasse, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet, linux-api,
linux-doc, linux-mm, cgroups, linux-kernel
In-Reply-To: <20150729142618.GJ15801@dhcp22.suse.cz>
On Wed, Jul 29, 2015 at 04:26:19PM +0200, Michal Hocko wrote:
> On Wed 29-07-15 16:59:07, Vladimir Davydov wrote:
> > On Wed, Jul 29, 2015 at 02:36:30PM +0200, Michal Hocko wrote:
> > > On Sun 19-07-15 15:31:09, Vladimir Davydov wrote:
> > > [...]
> > > > ---- USER API ----
> > > >
> > > > The user API consists of two new proc files:
> > >
> > > I was thinking about this for a while. I dislike the interface. It is
> > > quite awkward to use - e.g. you have to read the full memory to check a
> > > single memcg idleness. This might turn out being a problem especially on
> > > large machines.
> >
> > Yes, with this API estimating the wss of a single memory cgroup will
> > cost almost as much as doing this for the whole system.
> >
> > Come to think of it, does anyone really need to estimate idleness of one
> > particular cgroup?
>
> It is certainly interesting for setting the low limit.
Yes, but IMO there is no point in setting the low limit for one
particular cgroup w/o considering what's going on with the rest of the
system.
>
> > If we are doing this for finding an optimal memcg
> > limits configuration or while considering a load move within a cluster
> > (which I think are the primary use cases for the feature), we must do it
> > system-wide to see the whole picture.
> >
> > > It also provides a very low level information (per-pfn idleness) which
> > > is inherently racy. Does anybody really require this level of detail?
> >
> > Well, one might want to do it per-process, obtaining PFNs from
> > /proc/pid/pagemap.
>
> Sure once the interface is exported you can do whatever ;) But my
> question is whether any real usecase _requires_ it.
I only know/care about my use case, which is memcg configuration, but I
want to make the API as reusable as possible.
>
> > > I would assume that most users are interested only in a single number
> > > which tells the idleness of the system/memcg.
> >
> > Yes, that's what I need it for - estimating containers' wss for setting
> > their limits accordingly.
>
> So why don't we export the single per memcg and global knobs then?
> This would have few advantages. First of all it would be much easier to
> use, you wouldn't have to export memcg ids and finally the implementation
> could be changed without any user visible changes (e.g. lru vs. pfn walks),
> potential caching and who knows what. In other words. Michel had a
> single number interface AFAIR, what was the primary reason to move away
> from that API?
Because there is too much to be taken care of in the kernel with such an
approach and chances are high that it won't satisfy everyone. What
should the scan period be equal too? Knob. How many kthreads do we want?
Knob. I want to keep history for last N intervals (this was a part of
Michel's implementation), what should N be equal to? Knob. I want to be
able to choose between an instant scan and a scan distributed in time.
Knob. I want to see stats for anon/locked/file/dirty memory separately,
please add them to the API. You see the scale of the problem with doing
it in the kernel?
The API this patch set introduces is simple and fair. It only defines
what "idle" flag mean and gives you a way to flip it. That's it. You
wanna history? DIY. You wanna periodic scans? DIY. Etc.
>
> > > Well, you have mentioned a per-process reclaim but I am quite
> > > skeptical about this.
> >
> > This is what Minchan mentioned initially. Personally, I'm not going to
> > use it per-process, but I wouldn't rule out this use case either.
>
> Considering how many times we have been bitten by too broad interfaces I
> would rather be conservative.
I consider an API "broad" when it tries to do a lot of different things.
sys_prctl is a good example of a broad API.
/proc/kpageidle is not broad, because it does just one thing (I hope it
does it good :). If we attempted to implement the scanner in the kernel
with all those tunables I mentioned above, then we would get a broad API
IMO.
>
> > > I guess the primary reason to rely on the pfn rather than the LRU walk,
> > > which would be more targeted (especially for memcg cases), is that we
> > > cannot hold lru lock for the whole LRU walk and we cannot continue
> > > walking after the lock is dropped. Maybe we can try to address that
> > > instead? I do not think this is easy to achieve but have you considered
> > > that as an option?
> >
> > Yes, I have, and I've come to a conclusion it's not doable, because LRU
> > lists can be constantly rotating at an arbitrary rate. If you have an
> > idea in mind how this could be done, please share.
>
> Yes this is really tricky with the current LRU implementation. I
> was playing with some ideas (do some checkpoints on the way) but
> none of them was really working out on a busy systems. But the LRU
> implementation might change in the future.
It might. Then we could come up with a new /proc or /sys file which
would do the same as /proc/kpageidle, but on per LRU^w whatever-it-is
basis, and give people a choice which one to use.
> I didn't mean this as a hard requirement it just sounds that the
> current implementation restrictions shape the user visible API which
> is a good sign to think twice about it.
Agree. That's why we are discussing it now :-)
>
> > Speaking of LRU-vs-PFN walk, iterating over PFNs has its own advantages:
> > - You can distribute a walk in time to avoid CPU bursts.
>
> This would make the information even more volatile. I am not sure how
> helpful it would be in the end.
If you do it periodically, it is quite accurate.
>
> > - You are free to parallelize the scanner as you wish to decrease the
> > scan time.
>
> This is true but you could argue similar with per-node/lru threads if this
> was implemented in the kernel and really needed. I am not sure it would
> be really needed though. I would expect this would be a low priority
> thing.
But if you needed it one day, you'd have to extend the kernel API. With
/proc/kpageidle, you just go and fix your program.
Thanks,
Vladimir
--
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>
^ permalink raw reply
* Re: [PATCH -mm v9 0/8] idle memory tracking
From: Vladimir Davydov @ 2015-07-29 15:31 UTC (permalink / raw)
To: Michel Lespinasse
Cc: Michal Hocko, Andrew Morton, Andres Lagar-Cavilla, Minchan Kim,
Raghavendra K T, Johannes Weiner, Greg Thelen, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CANN689Euq3Y-CHQo8q88vzFAYZX4S6rK+rZRfbuSKfS74u=gcg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Jul 29, 2015 at 08:08:22AM -0700, Michel Lespinasse wrote:
> On Wed, Jul 29, 2015 at 7:45 AM, Vladimir Davydov <vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> wrote:
> > Page table scan approach has the inherent problem - it ignores unmapped
> > page cache. If a workload does a lot of read/write or map-access-unmap
> > operations, we won't be able to even roughly estimate its wss.
>
> You can catch that in mark_page_accessed on those paths, though.
Actually, the problem here is how to find an unmapped page cache page
*to mark it idle*, not to mark it accessed.
Thanks,
Vladimir
^ permalink raw reply
* Re: [PATCH -mm v9 0/8] idle memory tracking
From: Michel Lespinasse @ 2015-07-29 15:34 UTC (permalink / raw)
To: Vladimir Davydov
Cc: Michal Hocko, Andrew Morton, Andres Lagar-Cavilla, Minchan Kim,
Raghavendra K T, Johannes Weiner, Greg Thelen, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet, linux-api,
linux-doc, linux-mm, cgroups, linux-kernel
In-Reply-To: <20150729153135.GW8100@esperanza>
[-- Attachment #1: Type: text/plain, Size: 956 bytes --]
On Wed, Jul 29, 2015 at 8:31 AM, Vladimir Davydov <vdavydov@parallels.com>
wrote:
> On Wed, Jul 29, 2015 at 08:08:22AM -0700, Michel Lespinasse wrote:
> > On Wed, Jul 29, 2015 at 7:45 AM, Vladimir Davydov <
> vdavydov@parallels.com>
> > wrote:
> > > Page table scan approach has the inherent problem - it ignores unmapped
> > > page cache. If a workload does a lot of read/write or map-access-unmap
> > > operations, we won't be able to even roughly estimate its wss.
> >
> > You can catch that in mark_page_accessed on those paths, though.
>
> Actually, the problem here is how to find an unmapped page cache page
> *to mark it idle*, not to mark it accessed.
>
Ah, yes.
When I tried that I was still scanning memory by address at the end just to
compute such totals - but I did not have to do rmap at that point anymore.
It did look incredibly lame, though.
--
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
[-- Attachment #2: Type: text/html, Size: 1422 bytes --]
^ permalink raw reply
* Re: [PATCH -mm v9 0/8] idle memory tracking
From: Vladimir Davydov @ 2015-07-29 15:36 UTC (permalink / raw)
To: Michal Hocko
Cc: Michel Lespinasse, Andrew Morton, Andres Lagar-Cavilla,
Minchan Kim, Raghavendra K T, Johannes Weiner, Greg Thelen,
David Rientjes, Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150729150855.GM15801-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org>
On Wed, Jul 29, 2015 at 05:08:55PM +0200, Michal Hocko wrote:
> On Wed 29-07-15 17:45:39, Vladimir Davydov wrote:
> > On Wed, Jul 29, 2015 at 07:12:13AM -0700, Michel Lespinasse wrote:
> > > On Wed, Jul 29, 2015 at 6:59 AM, Vladimir Davydov <vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> > > wrote:
> > > >> I guess the primary reason to rely on the pfn rather than the LRU walk,
> > > >> which would be more targeted (especially for memcg cases), is that we
> > > >> cannot hold lru lock for the whole LRU walk and we cannot continue
> > > >> walking after the lock is dropped. Maybe we can try to address that
> > > >> instead? I do not think this is easy to achieve but have you considered
> > > >> that as an option?
> > > >
> > > > Yes, I have, and I've come to a conclusion it's not doable, because LRU
> > > > lists can be constantly rotating at an arbitrary rate. If you have an
> > > > idea in mind how this could be done, please share.
> > > >
> > > > Speaking of LRU-vs-PFN walk, iterating over PFNs has its own advantages:
> > > > - You can distribute a walk in time to avoid CPU bursts.
> > > > - You are free to parallelize the scanner as you wish to decrease the
> > > > scan time.
> > >
> > > There is a third way: one could go through every MM in the system and scan
> > > their page tables. Doing things that way turns out to be generally faster
> > > than scanning by physical address, because you don't have to go through
> > > RMAP for every page. But, you end up needing to take the mmap_sem lock of
> > > every MM (in turn) while scanning them, and that degrades quickly under
> > > memory load, which is exactly when you most need this feature. So, scan by
> > > address is still what we use here.
> >
> > Page table scan approach has the inherent problem - it ignores unmapped
> > page cache. If a workload does a lot of read/write or map-access-unmap
> > operations, we won't be able to even roughly estimate its wss.
>
> That page cache is trivially reclaimable if it is clean. If it needs
> writeback then it is non-idle only until the next writeback. So why does
> it matter for the estimation?
Because it might be a part of a workload's working set, in which case
evicting it will make the workload lag.
Thanks,
Vladimir
^ permalink raw reply
* [PATCH V6 0/6] Allow user to request memory to be locked on page fault
From: Eric B Munson @ 2015-07-29 15:42 UTC (permalink / raw)
To: Andrew Morton
Cc: Eric B Munson, Shuah Khan, Michal Hocko, Michael Kerrisk,
Vlastimil Babka, Jonathan Corbet, Ralf Baechle, linux-alpha,
linux-kernel, linux-mips, linux-parisc, linuxppc-dev, sparclinux,
linux-xtensa, linux-mm, linux-arch, linux-api
mlock() allows a user to control page out of program memory, but this
comes at the cost of faulting in the entire mapping when it is
allocated. For large mappings where the entire area is not necessary
this is not ideal. Instead of forcing all locked pages to be present
when they are allocated, this set creates a middle ground. Pages are
marked to be placed on the unevictable LRU (locked) when they are first
used, but they are not faulted in by the mlock call.
This series introduces a new mlock() system call that takes a flags
argument along with the start address and size. This flags argument
gives the caller the ability to request memory be locked in the
traditional way, or to be locked after the page is faulted in. A new
MCL flag is added to mirror the lock on fault behavior from mlock() in
mlockall().
There are two main use cases that this set covers. The first is the
security focussed mlock case. A buffer is needed that cannot be written
to swap. The maximum size is known, but on average the memory used is
significantly less than this maximum. With lock on fault, the buffer
is guaranteed to never be paged out without consuming the maximum size
every time such a buffer is created.
The second use case is focussed on performance. Portions of a large
file are needed and we want to keep the used portions in memory once
accessed. This is the case for large graphical models where the path
through the graph is not known until run time. The entire graph is
unlikely to be used in a given invocation, but once a node has been
used it needs to stay resident for further processing. Given these
constraints we have a number of options. We can potentially waste a
large amount of memory by mlocking the entire region (this can also
cause a significant stall at startup as the entire file is read in).
We can mlock every page as we access them without tracking if the page
is already resident but this introduces large overhead for each access.
The third option is mapping the entire region with PROT_NONE and using
a signal handler for SIGSEGV to mprotect(PROT_READ) and mlock() the
needed page. Doing this page at a time adds a significant performance
penalty. Batching can be used to mitigate this overhead, but in order
to safely avoid trying to mprotect pages outside of the mapping, the
boundaries of each mapping to be used in this way must be tracked and
available to the signal handler. This is precisely what the mm system
in the kernel should already be doing.
For mlock(MLOCK_ONFAULT) the user is charged against RLIMIT_MEMLOCK as
if mlock(MLOCK_LOCKED) or mmap(MAP_LOCKED) was used, so when the VMA is
created not when the pages are faulted in. For mlockall(MCL_ONFAULT)
the user is charged as if MCL_FUTURE was used. This decision was made
to keep the accounting checks out of the page fault path.
To illustrate the benefit of this set I wrote a test program that mmaps
a 5 GB file filled with random data and then makes 15,000,000 accesses
to random addresses in that mapping. The test program was run 20 times
for each setup. Results are reported for two program portions, setup
and execution. The setup phase is calling mmap and optionally mlock on
the entire region. For most experiments this is trivial, but it
highlights the cost of faulting in the entire region. Results are
averages across the 20 runs in milliseconds.
mmap with mlock(MLOCK_LOCKED) on entire range:
Setup avg: 8228.666
Processing avg: 8274.257
mmap with mlock(MLOCK_LOCKED) before each access:
Setup avg: 0.113
Processing avg: 90993.552
mmap with PROT_NONE and signal handler and batch size of 1 page:
With the default value in max_map_count, this gets ENOMEM as I attempt
to change the permissions, after upping the sysctl significantly I get:
Setup avg: 0.058
Processing avg: 69488.073
mmap with PROT_NONE and signal handler and batch size of 8 pages:
Setup avg: 0.068
Processing avg: 38204.116
mmap with PROT_NONE and signal handler and batch size of 16 pages:
Setup avg: 0.044
Processing avg: 29671.180
mmap with mlock(MLOCK_ONFAULT) on entire range:
Setup avg: 0.189
Processing avg: 17904.899
The signal handler in the batch cases faulted in memory in two steps to
avoid having to know the start and end of the faulting mapping. The
first step covers the page that caused the fault as we know that it will
be possible to lock. The second step speculatively tries to mlock and
mprotect the batch size - 1 pages that follow. There may be a clever
way to avoid this without having the program track each mapping to be
covered by this handeler in a globally accessible structure, but I could
not find it. It should be noted that with a large enough batch size
this two step fault handler can still cause the program to crash if it
reaches far beyond the end of the mapping.
These results show that if the developer knows that a majority of the
mapping will be used, it is better to try and fault it in at once,
otherwise mlock(MLOCK_ONFAULT) is significantly faster.
The performance cost of these patches are minimal on the two benchmarks
I have tested (stream and kernbench). The following are the average
values across 20 runs of stream and 10 runs of kernbench after a warmup
run whose results were discarded.
Avg throughput in MB/s from stream using 1000000 element arrays
Test 4.2-rc1 4.2-rc1+lock-on-fault
Copy: 10,566.5 10,421
Scale: 10,685 10,503.5
Add: 12,044.1 11,814.2
Triad: 12,064.8 11,846.3
Kernbench optimal load
4.2-rc1 4.2-rc1+lock-on-fault
Elapsed Time 78.453 78.991
User Time 64.2395 65.2355
System Time 9.7335 9.7085
Context Switches 22211.5 22412.1
Sleeps 14965.3 14956.1
---
Changes from V5:
Drop MLOCK_LOCKED flag
* MLOCK_ONFAULT and MCL_ONFAULT are treated as a modifier to other locking
operations, mirroring the relationship between VM_LOCKED and
VM_LOCKONFAULT
* Drop mmap flag and related tests
* Fix clearing of MCL_CURRENT when mlockall is called with MCL_FUTURE,
mlockall behavoir now matches the old behavior WRT to ordering
Changes from V4:
Drop all architectures for new sys call entries except x86[_64] and MIPS
Drop munlock2 and munlockall2
Make VM_LOCKONFAULT a modifier to VM_LOCKED only to simplify book keeping
Adjust tests to match
Changes from V3:
Ensure that pages present when mlock2(MLOCK_ONFAULT) is called are locked
Ensure that VM_LOCKONFAULT is handled in cases that used to only check VM_LOCKED
Add tests for new system calls
Add missing syscall entries, fix NR_syscalls on multiple arch's
Add missing MAP_LOCKONFAULT for tile
Changes from V2:
Added new system calls for mlock, munlock, and munlockall with added
flags arguments for controlling how memory is locked or unlocked.
Eric B Munson (6):
mm: mlock: Refactor mlock, munlock, and munlockall code
mm: mlock: Add new mlock system call
mm: Introduce VM_LOCKONFAULT
mm: mlock: Add mlock flags to enable VM_LOCKONFAULT usage
selftests: vm: Add tests for lock on fault
mips: Add entry for new mlock2 syscall
arch/alpha/include/uapi/asm/mman.h | 3 +
arch/mips/include/uapi/asm/mman.h | 6 +
arch/mips/include/uapi/asm/unistd.h | 15 +-
arch/mips/kernel/scall32-o32.S | 1 +
arch/mips/kernel/scall64-64.S | 1 +
arch/mips/kernel/scall64-n32.S | 1 +
arch/mips/kernel/scall64-o32.S | 1 +
arch/parisc/include/uapi/asm/mman.h | 3 +
arch/powerpc/include/uapi/asm/mman.h | 1 +
arch/sparc/include/uapi/asm/mman.h | 1 +
arch/tile/include/uapi/asm/mman.h | 1 +
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
arch/xtensa/include/uapi/asm/mman.h | 6 +
drivers/gpu/drm/drm_vm.c | 8 +-
fs/proc/task_mmu.c | 1 +
include/linux/mm.h | 2 +
include/linux/syscalls.h | 2 +
include/uapi/asm-generic/mman-common.h | 5 +
include/uapi/asm-generic/mman.h | 1 +
include/uapi/asm-generic/unistd.h | 4 +-
kernel/fork.c | 2 +-
kernel/sys_ni.c | 1 +
mm/debug.c | 1 +
mm/gup.c | 10 +-
mm/huge_memory.c | 2 +-
mm/hugetlb.c | 4 +-
mm/mlock.c | 91 +++-
mm/mmap.c | 2 +-
mm/rmap.c | 4 +-
tools/testing/selftests/vm/Makefile | 2 +
tools/testing/selftests/vm/mlock2-tests.c | 661 ++++++++++++++++++++++++++++
tools/testing/selftests/vm/on-fault-limit.c | 47 ++
tools/testing/selftests/vm/run_vmtests | 22 +
34 files changed, 873 insertions(+), 41 deletions(-)
create mode 100644 tools/testing/selftests/vm/mlock2-tests.c
create mode 100644 tools/testing/selftests/vm/on-fault-limit.c
Cc: Shuah Khan <shuahkh@osg.samsung.com>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-alpha@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-mips@linux-mips.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: sparclinux@vger.kernel.org
Cc: linux-xtensa@linux-xtensa.org
Cc: linux-mm@kvack.org
Cc: linux-arch@vger.kernel.org
Cc: linux-api@vger.kernel.org
--
1.9.1
--
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>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox