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 56C16C76196 for ; Thu, 23 Mar 2023 15:46:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231918AbjCWPqt (ORCPT ); Thu, 23 Mar 2023 11:46:49 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55866 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231944AbjCWPqr (ORCPT ); Thu, 23 Mar 2023 11:46:47 -0400 Received: from mail-yw1-x1131.google.com (mail-yw1-x1131.google.com [IPv6:2607:f8b0:4864:20::1131]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AA5E224BCC for ; Thu, 23 Mar 2023 08:46:46 -0700 (PDT) Received: by mail-yw1-x1131.google.com with SMTP id 00721157ae682-53d277c1834so401925497b3.10 for ; Thu, 23 Mar 2023 08:46:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1679586406; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=YBdASoORqRtNKDZe+JhohFUINcVjU1T9Cmqk53zCz4U=; b=ckebmJKNAMJvXNZunzlqH+ez60Rc/ONMAZsXIP/WI2OPr8Ugs6CAw5uIB5GQSzBF5t nZbEU4B2yYUqjm8x0bs1lBP/PqQ0xb8ZVEQCXkc+Z42xiibHqGVTX9wXrcoRFQ64iCs9 PotN+b/x3H8Xl2/rGsaQprbltIO3MbPyh+lr9YykF5ZkAaF01xGyi2kItgkAxOx3o1P0 N2yxvqIw6ycEiEWVwg5xka5yKOaO6E/6dCS3vsfKQlk7bduove72mFlBiob15+9a7S5w vsZQvTBTbmtQn+MZurcYgLRf23og+qiN+y+xr7p6VdhlSbMdUanDPNka6vaZTlo8dYu+ 7HUA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679586406; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=YBdASoORqRtNKDZe+JhohFUINcVjU1T9Cmqk53zCz4U=; b=NgD/sgvQ9fy5us2dI7yoUvrz89ZqUS7mWmnr2nuihzeG4nu4LtJHWofLWF86kEuc8d bF8H1DffBIuUqjibrnck6BNFlBUbzukR5M2PrvbV5su6oLUxv+GeEfUIJ7OUvwX6DpwW gCkZrr5DbKwTAS7RfuARsalPJqoIfJtmP9mjbw103y/DNLH55tlJsIPOvwTSQjpxI+Q7 kCc0XHzHt2buuUuvzmUlWSQ9hofw4AWhcKG1sFss+RBC0Pm+9JSa1X2izREhNF+lonLy 4GMs1kFNwthM6koZWlyDswEJ9DG+9Ry14v2j4wl4j4nb68uW5+lEfQ4qCMfyyWwTyHZR yhkQ== X-Gm-Message-State: AAQBX9dc4d1lAQWG4/nGuXgHRi9YiiFr2zJ6RD1kKKZkO3+dV42WqxX8 Jaapf/PDyGeQccXWywd3q2npTAwccsnkT7p4vA1QyA== X-Google-Smtp-Source: AKy350Z3s4eLHH6fqjOcmIDzeeoM06xCh16V/YBEFB3KPT5mVXp0NAv4et3n+6L/J74+4q9pBbx4DJK1zZT8HJCaoMU= X-Received: by 2002:a81:ef08:0:b0:545:883a:542c with SMTP id o8-20020a81ef08000000b00545883a542cmr1297377ywm.0.1679586405658; Thu, 23 Mar 2023 08:46:45 -0700 (PDT) MIME-Version: 1.0 References: <20230323040037.2389095-1-yosryahmed@google.com> <20230323040037.2389095-2-yosryahmed@google.com> In-Reply-To: From: Shakeel Butt Date: Thu, 23 Mar 2023 08:46:34 -0700 Message-ID: Subject: Re: [RFC PATCH 1/7] cgroup: rstat: only disable interrupts for the percpu lock To: Yosry Ahmed Cc: Tejun Heo , Josef Bacik , Jens Axboe , Zefan Li , Johannes Weiner , Michal Hocko , Roman Gushchin , Muchun Song , Andrew Morton , Vasily Averin , cgroups@vger.kernel.org, linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, bpf@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org On Thu, Mar 23, 2023 at 8:43=E2=80=AFAM Yosry Ahmed = wrote: > > On Thu, Mar 23, 2023 at 8:40=E2=80=AFAM Shakeel Butt wrote: > > > > On Thu, Mar 23, 2023 at 6:36=E2=80=AFAM Yosry Ahmed wrote: > > > > > [...] > > > > > > > > > > > 2. Are we really calling rstat flush in irq context? > > > > > > > > > > I think it is possible through the charge/uncharge path: > > > > > memcg_check_events()->mem_cgroup_threshold()->mem_cgroup_usage().= I > > > > > added the protection against flushing in an interrupt context for > > > > > future callers as well, as it may cause a deadlock if we don't di= sable > > > > > interrupts when acquiring cgroup_rstat_lock. > > > > > > > > > > > 3. The mem_cgroup_flush_stats() call in mem_cgroup_usage() is o= nly > > > > > > done for root memcg. Why is mem_cgroup_threshold() interested i= n root > > > > > > memcg usage? Why not ignore root memcg in mem_cgroup_threshold(= ) ? > > > > > > > > > > I am not sure, but the code looks like event notifications may be= set > > > > > up on root memcg, which is why we need to check thresholds. > > > > > > > > This is something we should deprecate as root memcg's usage is ill = defined. > > > > > > Right, but I think this would be orthogonal to this patch series. > > > > > > > I don't think we can make cgroup_rstat_lock a non-irq-disabling lock > > without either breaking a link between mem_cgroup_threshold and > > cgroup_rstat_lock or make mem_cgroup_threshold work without disabling > > irqs. > > > > So, this patch can not be applied before either of those two tasks are > > done (and we may find more such scenarios). > > > Could you elaborate why? > > My understanding is that with an in_task() check to make sure we only > acquire cgroup_rstat_lock from non-irq context it should be fine to > acquire cgroup_rstat_lock without disabling interrupts. >From mem_cgroup_threshold() code path, cgroup_rstat_lock will be taken with irq disabled while other code paths will take cgroup_rstat_lock with irq enabled. This is a potential deadlock hazard unless cgroup_rstat_lock is always taken with irq disabled.