From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.9 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9A410C2BA1A for ; Tue, 7 Apr 2020 01:47:08 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 3F97E206C0 for ; Tue, 7 Apr 2020 01:47:08 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=joelfernandes.org header.i=@joelfernandes.org header.b="acA8eNjU" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3F97E206C0 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=joelfernandes.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id D510A8E0005; Mon, 6 Apr 2020 21:47:07 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D018A8E0001; Mon, 6 Apr 2020 21:47:07 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C17018E0005; Mon, 6 Apr 2020 21:47:07 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0091.hostedemail.com [216.40.44.91]) by kanga.kvack.org (Postfix) with ESMTP id A9BFE8E0001 for ; Mon, 6 Apr 2020 21:47:07 -0400 (EDT) Received: from smtpin13.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 5BD28180AD807 for ; Tue, 7 Apr 2020 01:47:07 +0000 (UTC) X-FDA: 76679370894.13.fight45_3efc2bf15563 X-HE-Tag: fight45_3efc2bf15563 X-Filterd-Recvd-Size: 10273 Received: from mail-qk1-f193.google.com (mail-qk1-f193.google.com [209.85.222.193]) by imf08.hostedemail.com (Postfix) with ESMTP for ; Tue, 7 Apr 2020 01:47:06 +0000 (UTC) Received: by mail-qk1-f193.google.com with SMTP id o18so130036qko.12 for ; Mon, 06 Apr 2020 18:47:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=joelfernandes.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=it7Sl4XJV8sr3uWUaSpf5IpDHLo3zFcDYRyOXrto1zg=; b=acA8eNjUgGW+WNIrrMA1PmZ9NXs+M/Z8oHZgW7gH0gPEtuSObad7rAi+AysuXk85gg jsCQ6bSELmnxE9E2w+st3YU4/fLQ5PqVrPu6dIONNExe2NwMwWsEEYZUUIFd5tRieZW0 T2jtmWrJScRRJkKv4rP4bNU8v972WUsMldh5o= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=it7Sl4XJV8sr3uWUaSpf5IpDHLo3zFcDYRyOXrto1zg=; b=qFFuw3N6/z9hBv/ycpTRS2sf14lkYVJ8xeVTw2lBF/sWrAK76JCuAW/7XkWV5ou+ju PDp77Potn5slFWU5pdf+P91b8SjP/ZyCyyiLNRjAoni43h81LIlqCUgY9L5+xB4F2fMR k1ScVwgXPx+IZQlvmrjFz7nPksz48B28vRIGPeTQgu1eYcZG5LQsmS0Jfm+NhD4i/Bva kUt+SFT9mfNo+A8aHaSWm4/hJPUxG6HW2CCk7h+yVuKhdohOUn7lUR0rshn/e/b4g08Q aOvTGKrbMBnRJ9+n6rKLQnQ/qCgNoIUsiGHExcyA2JyGQU/vl0KA8wTTN33zSGxMwdCl rO5g== X-Gm-Message-State: AGi0Pua2m9AT2JvMEoRBjivJrTzXCul/GKsIkDY8JD2yEByv/W8Vbr5k jxd8CrSZy03ok1vIAx6yNmcriQ== X-Google-Smtp-Source: APiQypI8N5S0mC8WSQKuIn5uRNOLIXiPoFJgFirmckJmZSLBdAPlSimlMsFFfJ/7rdG427LN5MyWPQ== X-Received: by 2002:a05:620a:81c:: with SMTP id s28mr19266022qks.147.1586224026227; Mon, 06 Apr 2020 18:47:06 -0700 (PDT) Received: from localhost ([2620:15c:6:12:9c46:e0da:efbf:69cc]) by smtp.gmail.com with ESMTPSA id r34sm16701941qtk.89.2020.04.06.18.47.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 Apr 2020 18:47:05 -0700 (PDT) Date: Mon, 6 Apr 2020 21:47:05 -0400 From: Joel Fernandes To: Uladzislau Rezki Cc: "Paul E . McKenney" , LKML , RCU , linux-mm@kvack.org, Andrew Morton , Steven Rostedt , Oleksiy Avramchenko Subject: Re: [PATCH 1/1] rcu/tree: add emergency pool for headless case Message-ID: <20200407014705.GC11326@google.com> References: <20200403173051.4081-1-urezki@gmail.com> <20200404195129.GA83565@google.com> <20200405172105.GA7539@pc636> <20200405233028.GC83565@google.com> <20200406125640.GA23256@pc636> <20200406151851.GF83565@google.com> <20200406161708.GA3919@pc636> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200406161708.GA3919@pc636> X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Apr 06, 2020 at 06:17:08PM +0200, Uladzislau Rezki wrote: > On Mon, Apr 06, 2020 at 11:18:51AM -0400, Joel Fernandes wrote: > > Hi Vlad, > > > > On Mon, Apr 06, 2020 at 02:56:40PM +0200, Uladzislau Rezki wrote: > > > Hello, Joel. > > > > > > > > > > > > > > > Hi Vlad, > > > > > > > > > > > > One concern I have is this moves the problem a bit further down. My belief is > > > > > > we should avoid the likelihood of even needing an rcu_head allocated for the > > > > > > headless case, to begin with - than trying to do damage-control when it does > > > > > > happen. The only way we would end up needing an rcu_head is if we could not > > > > > > allocate an array. > > > > > > > > > > > Let me share my view on all such caching. I think that now it becomes less as > > > > > the issue, because of we have now https://lkml.org/lkml/2020/4/2/383 patch. > > > > > I see that it does help a lot. I tried to simulate low memory condition and > > > > > apply high memory pressure with that. I did not manage to trigger the > > > > > "synchronize rcu" path at all. It is because of using much more permissive > > > > > parameters when we request a memory from the SLAB(direct reclaim, etc...). > > > > > > > > That's a good sign that we don't hit this path in your tests. > > > > > > > Just one request, of course if you have a time :) > > > Could you please double check on your test environment to stress the system > > > to check if you also can not hit it? > > > > Sure, I am planning to do so and happy to spend time on it :) One question I > > had about the below test: > > > > > How i test it. Please apply below patch: > > > > > > t a/kernel/rcu/tree.c b/kernel/rcu/tree.c > > > index 5e26145e9ead..25f7ac8583e1 100644 > > > --- a/kernel/rcu/tree.c > > > +++ b/kernel/rcu/tree.c > > > @@ -3203,6 +3203,7 @@ void kvfree_call_rcu(struct rcu_head *head, rcu_callback_t func) > > > > > > if (head) { > > > ptr = (void *) head - (unsigned long) func; > > > + head = NULL; > > > } else { > > > /* > > > * Please note there is a limitation for the head-less > > > @@ -3233,16 +3234,18 @@ void kvfree_call_rcu(struct rcu_head *head, rcu_callback_t func) > > > * Under high memory pressure GFP_NOWAIT can fail, > > > * in that case the emergency path is maintained. > > > */ > > > - success = kvfree_call_rcu_add_ptr_to_bulk(krcp, ptr); > > > - if (!success) { > > > + /* success = kvfree_call_rcu_add_ptr_to_bulk(krcp, ptr); */ > > > > If you drop this, then it is not realistic right? I mean it changes behavior > > of the code completely. We need to try to allocate array and then try to > > allocate the head. > > > That just bypasses an allocation for the array, to make it more simple > and move forward toward the path we would like to test. Also head is > set to NULL to simulated headless freeing. Makes sense, I know you are forcing code to invoke the bad case more often but I was concerned the hack would change dynamics of code enough to make it an unrealistic situation. But I see your point. > > > + /* if (!success) { */ > > > /* Is headless object? */ > > > if (head == NULL) { > > > /* Drop the lock. */ > > > krc_this_cpu_unlock(krcp, flags); > > > > > > head = attach_rcu_head_to_object(ptr); > > > - if (head == NULL) > > > + if (head == NULL) { > > > + success = false; > > > goto inline_return; > > > + } > > > > > > /* Take it back. */ > > > krcp = krc_this_cpu_lock(&flags); > > > @@ -3267,7 +3270,7 @@ void kvfree_call_rcu(struct rcu_head *head, rcu_callback_t func) > > > */ > > > expedited_drain = true; > > > success = true; > > > - } > > > + /* } */ > > > > > > WRITE_ONCE(krcp->count, krcp->count + 1); > > > > > > @@ -3297,7 +3300,9 @@ void kvfree_call_rcu(struct rcu_head *head, rcu_callback_t func) > > > if (!rcu_kfree_nowarn) > > > WARN_ON_ONCE(1); > > > debug_rcu_head_unqueue(ptr); > > > - synchronize_rcu(); > > > + /* synchronize_rcu(); */ > > > + printk(KERN_ERR "-> hit synchronize_rcu() path.\n"); > > > + trace_printk("-> hit synchronize_rcu() path.\n"); > > > kvfree(ptr); > > > } > > > } > > > > > > > > > lower the memory size and run kfree rcu tests. It would be appreciated. > > > > I am happy to try out the diff if I can understand how the above diff is > > close enough with current code's behavior, if we are not using the array. One > > other issue with current kfree rcu tests is, the test is itself the reason > > for the pressure -- I believe we should also have some testing that shows > > that the memory pressure is caused else where (such as a real user workload > > causing OOM), and then we see how RCU behaves under OOM -- if we have too > > many synchronous latencies, does the additional caching remove such latenies > > under OOM? etc. I also want to look into your vmalloc tests. > > > Of course to have real tests would be good. Agreed. > > > > I guess also, with your latest patch on releasing the lock to be in a > > > > non-atomic context, and then doing the allocation, it became even more > > > > permissive? If you drop that patch and tried, do you still not hit the > > > > synchronous path more often? > > > > > > > Yep. If i drop the patch, i can hit it. > > > > Ah, cool. So basically the direct-reclaim path does the synchronous waiting, > > instead of synchronize_rcu(). Either way, we wait synchronously. How to chose > > which way is better though? If direct reclaim improves the memory situation, > > then we should enter that path. But if direct reclaim takes too much time > > (thus hurting the kfree_rcu() latency), then perhaps it is better for > > kfree_rcu() to just do the synchronize_rcu() and let someone else enter the > > direct-reclaim path. We should probably quantify and see which approach works > > better. > > > I see at it like, headless variant has to be called from the sleeping context, > therefore it can sleep. What is better to call synchronize_rcu() or doing direct > reclaim depends on how many CPUs in a system we have. I suspect that doing > direct reclaim is better, at least it will free some memory for us. We > could also extend that patch and make it a bit different, for example do > NOWAIT then try ATOMIC and as a last step do GFP_KERNEL alloc. Yes, that's a good idea. That way perhaps we reduce chance that kfree_rcu() enters into direct-reclaim. Let us do it that way. At least I don't see any drawbacks in such approach. thanks, - Joel