linux-kernel - Re: [PATCH RFC] rcu/tree: Refactor object allocation and try harder for array allocation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 24 Apr 2020 00:16:16 -0400
From:   Joel Fernandes <joel@...lfernandes.org>
To:     Uladzislau Rezki <urezki@...il.com>
Cc:     "Paul E. McKenney" <paulmck@...nel.org>,
        Johannes Weiner <hannes@...xchg.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Josh Triplett <josh@...htriplett.org>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        rcu <rcu@...r.kernel.org>, Steven Rostedt <rostedt@...dmis.org>
Subject: Re: [PATCH RFC] rcu/tree: Refactor object allocation and try harder
 for array allocation

Hi Vlad,
I'm mostly on the same page, some more comments below:

On Thu, Apr 23, 2020 at 4:00 PM Uladzislau Rezki <urezki@...il.com> wrote:
[snip]
> a) Single argument(headless)
> In this case we can make use an allocator with sleepable flags,
> because we document that headleass variant must follow might_sleep()
> annotation. For example __GFP_NORETRY | __GFP_NOWARN. __GFP_NORETRY
> can do some light direct reclaim, thus the caller can call schedule().
> To do such allocation we just drop our local spinlock.
> If an allocation gets failed, we directly fall into synchronize_rcu()
> i.e. inline freeing.
>
> I also call it sleepable case, that is (a).
>
> b) Double argument(with rcu_head)
> This case we consider as it gets called from atomic context even though
> it can be not. Why we consider such case as atomic: we just assume that.
> The reason is to keep it simple, because it is not possible to detect whether
> a current context is attomic or not(for all type of kernels), i mean the one
> that calls kfree_rcu().
>
> In this case we do not have synchronize_rcu() option. Instead we have an
> object with rcu_head inside. If an allocation gets failed we just make
> use of rcu_head inside the object, regular queuing.
>
> In this case we do not need to hard in order to obtain memory. Therefore
> my question was to Johannes what is best way here. Since we decided to
> minimize reclaiming, whereas GFP_NOWAIT wakes up kswapd if no memory.
> GFP_ATOMIC also is not good, because for (b) we do not need to waste
> it.

I think Johannes said that waking up kswapd is Ok. OTOH, I did not see
the drawback in waking up kswapd to do background reclaim since it
does not happen synchronously right? I think Johannes said we can do
better than just waking kswapd by also doing light direct reclaim
using __GFP_NORETRY but let me know if I missed something.

> > Upon memory-allocation failure, the single-argument kfree_rcu() can leak
> > the memory (it has presumably already splatted) and the double-argument
> > kfree_rcu() can make use of the rcu_head structure that was provided.
> >
> For single argument we inline freeing into current context after
> synchronize_rcu() because it follows might_sleep() annotation.

Yes.

Also, with the additional caching being planned, we could avoid the
chances of hitting the synchronize_rcu inlining.

Thanks,

 - Joel