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@xxxxxxxxx> 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
