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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 62FC5C6FD18 for ; Wed, 29 Mar 2023 11:22:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229694AbjC2LWQ (ORCPT ); Wed, 29 Mar 2023 07:22:16 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53446 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229477AbjC2LWQ (ORCPT ); Wed, 29 Mar 2023 07:22:16 -0400 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D9718133; Wed, 29 Mar 2023 04:22:14 -0700 (PDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 8D56F1F7AB; Wed, 29 Mar 2023 11:22:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1680088933; 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=gebdMLUhhGELLLVSbQbSg6v4/rJOoLd9eRIXmlgXsio=; b=vXmcD7WVZBRtIF0QTHnyV0Rs4Utot99+8SQdPpEeQgdw9Nt9PPFDXEaDiGG07RKvkX9o4l lGXnaYncs8HwoSJ4MiI1vkPA5f/VJ3GJlKnBIUWk10zdR5ivx8u3vsRJi+RTJsPs0Xg3tS /x3S3AGVNCHDf2wdcfpAeE+BafyyrcY= Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 6EBF7138FF; Wed, 29 Mar 2023 11:22:13 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id QbCbGWUfJGSgSQAAMHmgww (envelope-from ); Wed, 29 Mar 2023 11:22:13 +0000 Date: Wed, 29 Mar 2023 13:22:12 +0200 From: Michal Hocko To: Yosry Ahmed Cc: Tejun Heo , Josef Bacik , Jens Axboe , Zefan Li , Johannes Weiner , Roman Gushchin , Shakeel Butt , Muchun Song , Andrew Morton , Michal =?iso-8859-1?Q?Koutn=FD?= , Vasily Averin , cgroups@vger.kernel.org, linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, bpf@vger.kernel.org Subject: Re: [PATCH v2 4/9] cgroup: rstat: add WARN_ON_ONCE() if flushing outside task context Message-ID: References: <20230328221644.803272-1-yosryahmed@google.com> <20230328221644.803272-5-yosryahmed@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230328221644.803272-5-yosryahmed@google.com> Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org On Tue 28-03-23 22:16:39, Yosry Ahmed wrote: > rstat flushing is too expensive to perform in irq context. > The previous patch removed the only context that may invoke an rstat > flush from irq context, add a WARN_ON_ONCE() to detect future > violations, or those that we are not aware of. > > Ideally, we wouldn't flush with irqs disabled either, but we have one > context today that does so in mem_cgroup_usage(). Forbid callers from > irq context for now, and hopefully we can also forbid callers with irqs > disabled in the future when we can get rid of this callsite. I am sorry to be late to the discussion. I wanted to follow up on Johannes reply in the previous version but you are too fast ;) I do agree that this looks rather arbitrary. You do not explain how the warning actually helps. Is the intention to be really verbose to the kernel log when somebody uses this interface from the IRQ context and get bug reports? What about configurations with panic on warn? Do we really want to crash their systems for something like that? > Signed-off-by: Yosry Ahmed > Reviewed-by: Shakeel Butt > --- > kernel/cgroup/rstat.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c > index d3252b0416b6..c2571939139f 100644 > --- a/kernel/cgroup/rstat.c > +++ b/kernel/cgroup/rstat.c > @@ -176,6 +176,8 @@ static void cgroup_rstat_flush_locked(struct cgroup *cgrp, bool may_sleep) > { > int cpu; > > + /* rstat flushing is too expensive for irq context */ > + WARN_ON_ONCE(!in_task()); > lockdep_assert_held(&cgroup_rstat_lock); > > for_each_possible_cpu(cpu) { > -- > 2.40.0.348.gf938b09366-goog -- Michal Hocko SUSE Labs