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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 58779C433F5 for ; Mon, 21 Feb 2022 16:24:45 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AACA28D0002; Mon, 21 Feb 2022 11:24:44 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id A342B8D0001; Mon, 21 Feb 2022 11:24:44 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8AE748D0002; Mon, 21 Feb 2022 11:24:44 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0186.hostedemail.com [216.40.44.186]) by kanga.kvack.org (Postfix) with ESMTP id 7D2008D0001 for ; Mon, 21 Feb 2022 11:24:44 -0500 (EST) Received: from smtpin13.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 30F3718195410 for ; Mon, 21 Feb 2022 16:24:44 +0000 (UTC) X-FDA: 79167310488.13.D516BDF Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by imf14.hostedemail.com (Postfix) with ESMTP id 736AB100002 for ; Mon, 21 Feb 2022 16:24:43 +0000 (UTC) Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 4DDD8210F2; Mon, 21 Feb 2022 16:24:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1645460682; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=KOF2rQUXfZ/DXE3meXJOJcIpB1gBHmL1wlFbk8LNnTA=; b=iS05qPB7xKkib6dNzf9XLX8Q4SS0/PC0Oq4MogwC79iYVHThplnxAQ/Kz/D5+4fNG1Bgrj fgPXkcsPIiXRC3/Ljc8Tqs1IB07deaLGPdZa3z6HBvTlQsNhC5vdamz85W2MkbzqC8uFnp UxLSgvgBsy7PO3dnsDnW6UPQyJatO48= Received: from suse.cz (unknown [10.100.201.86]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id C585BA3B89; Mon, 21 Feb 2022 16:24:41 +0000 (UTC) Date: Mon, 21 Feb 2022 17:24:41 +0100 From: Michal Hocko To: Sebastian Andrzej Siewior Cc: cgroups@vger.kernel.org, linux-mm@kvack.org, Andrew Morton , Johannes Weiner , Michal =?iso-8859-1?Q?Koutn=FD?= , Peter Zijlstra , Thomas Gleixner , Vladimir Davydov , Waiman Long , kernel test robot Subject: Re: [PATCH v3 5/5] mm/memcg: Protect memcg_stock with a local_lock_t Message-ID: References: <20220217094802.3644569-1-bigeasy@linutronix.de> <20220217094802.3644569-6-bigeasy@linutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 736AB100002 X-Stat-Signature: 6o33kpieojufbw6b6uihiy7ea4w8it3b Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=iS05qPB7; spf=pass (imf14.hostedemail.com: domain of mhocko@suse.com designates 195.135.220.28 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com X-Rspam-User: X-HE-Tag: 1645460683-984483 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 21-02-22 16:19:52, Sebastian Andrzej Siewior wrote: > On 2022-02-21 15:46:30 [+0100], Michal Hocko wrote: > > On Thu 17-02-22 10:48:02, Sebastian Andrzej Siewior wrote: > > [...] > > > @@ -2266,7 +2273,6 @@ static void drain_all_stock(struct mem_cgroup *root_memcg) > > > * as well as workers from this path always operate on the local > > > * per-cpu data. CPU up doesn't touch memcg_stock at all. > > > */ > > > - curcpu = get_cpu(); > > > > Could you make this a separate patch? > > Sure. > > > > for_each_online_cpu(cpu) { > > > struct memcg_stock_pcp *stock = &per_cpu(memcg_stock, cpu); > > > struct mem_cgroup *memcg; > > > @@ -2282,14 +2288,9 @@ static void drain_all_stock(struct mem_cgroup *root_memcg) > > > rcu_read_unlock(); > > > > > > if (flush && > > > - !test_and_set_bit(FLUSHING_CACHED_CHARGE, &stock->flags)) { > > > - if (cpu == curcpu) > > > - drain_local_stock(&stock->work); > > > - else > > > - schedule_work_on(cpu, &stock->work); > > > - } > > > + !test_and_set_bit(FLUSHING_CACHED_CHARGE, &stock->flags)) > > > + schedule_work_on(cpu, &stock->work); > > > > Maybe I am missing but on !PREEMPT kernels there is nothing really > > guaranteeing that the worker runs so there should be cond_resched after > > the mutex is unlocked. I do not think we want to rely on callers to be > > aware of this subtlety. > > There is no guarantee on PREEMPT kernels, too. The worker will be made > running and will be put on the CPU when the scheduler sees it fit and > there could be other worker which take precedence (queued earlier). > But I was not aware that the worker _needs_ to run before we return. A lack of draining will not be a correctness problem (sorry I should have made that clear). It is more about subtlety than anything. E.g. the charging path could be forced to memory reclaim because of the cached charges which are still waiting for their draining. Not really something to lose sleep over from the runtime perspective. I was just wondering that this makes things more complex than necessary. > We > might get migrated after put_cpu() so I wasn't aware that this is > important. Should we attempt best effort and wait for the worker on the > current CPU? > > An alternative would be to split out __drain_local_stock which doesn't > > do local_lock. > > but isn't the section in drain_local_stock() unprotected then? local_lock instead of {get,put}_cpu would handle that right? -- Michal Hocko SUSE Labs