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 59DFCC6FD18 for ; Tue, 28 Mar 2023 17:53:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C315F6B0074; Tue, 28 Mar 2023 13:53:28 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BE1AE6B0075; Tue, 28 Mar 2023 13:53:28 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A83B2900002; Tue, 28 Mar 2023 13:53:28 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 98F266B0074 for ; Tue, 28 Mar 2023 13:53:28 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 7478C140B23 for ; Tue, 28 Mar 2023 17:53:28 +0000 (UTC) X-FDA: 80619054096.28.54052FA Received: from mail-ed1-f45.google.com (mail-ed1-f45.google.com [209.85.208.45]) by imf04.hostedemail.com (Postfix) with ESMTP id 902464000E for ; Tue, 28 Mar 2023 17:53:26 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=cmpxchg-org.20210112.gappssmtp.com header.s=20210112 header.b=WyJBv5Pq; spf=pass (imf04.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.208.45 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1680026006; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=GBBAU6hZLECWp0UudInEHiiA2FkG0Dhl57EhHD71/2w=; b=hJkgoh0C1leGa2pv1epaCxjA4Mq+eT664vMGDxvkvB/w1i+iOsYtcwz+f9JSrr5sBj3AqA 7FNq/Q2bHVso+LqfWiiFhXIRBlMSMMK/R9KdfXbrihlGUjjwAgK1h01wLoa6Qm2AF3vb8D x4wraKNYaugPq33hqXFUVPmalAQo7fY= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=cmpxchg-org.20210112.gappssmtp.com header.s=20210112 header.b=WyJBv5Pq; spf=pass (imf04.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.208.45 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1680026006; a=rsa-sha256; cv=none; b=T6VZlrGvI5wvXL6GXDKSajfUAmbg5g/HKkOlZLNtnAK4fmJoqO3XM0ZhSARpvo6WuI7wCC OQkPYX/GomDgzyxuD7f/72jhyfYrgdJNmNws4Ni607/jYSyD3bct+scT5tA7VmDdIwLBvJ U4/PGr9XaJMTPG5Z7H+3Fk5VtJPfXV0= Received: by mail-ed1-f45.google.com with SMTP id r11so53079149edd.5 for ; Tue, 28 Mar 2023 10:53:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20210112.gappssmtp.com; s=20210112; t=1680026004; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=GBBAU6hZLECWp0UudInEHiiA2FkG0Dhl57EhHD71/2w=; b=WyJBv5Pqllid3pNI6/ikTPvxnNXwwYvm5BeSsZHlodYCQOrWeIKez1PCCxvY3x1D8c 3+AZOlaWzwu1aKR+OAeUfaGWTxiIfflV7gGlohUIYiEqEskdjBV9QYa+PD/HDQeJZU3Y pFQhYKU+arSbJemfnq9SR0DQ6bDuZPsw4fp/k2p3LlpGJi35X785hbTi0EoGjexbJdKT /zo4sSXmA7NblVWxBBey8ZK0PCmLNdDA177py1CZsWHspE6EGmptD4//mQHui9cQkTEc kPDtXGZPdalt51dVpXNabobGHyskSSeC52qpMMTruKTKj6Vgxy77Pb7DYdWEsqx2UcyW ZVrA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680026004; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=GBBAU6hZLECWp0UudInEHiiA2FkG0Dhl57EhHD71/2w=; b=jjtpUvm0RGnIoTiPzP0LFsK0fWMYeZ3EVc/TEAnQaq6Exqhlwtd9xXvWUTpxnV2c3K TR155xX2TK34vHU7Yq5kFHz/reLjz5+folkkiyweg80YUmK8pFDzZqHWGQNeZrPEL5Ay 7gxJuS8af5TaD/rhG7xscBjELKiihO2Y4Ul/rWXMABD9cKTiWDFWANUsFi6rzAjC4t2I 6q8bhQu3DkTlubIdrFY2VPxBLNv5p7rhjyfqm+lxzQ9+bzoFMxo3/y1U9Oz7/soiEqjX HjEZPbxdF7US1/TwC5JHVKHXdtXpfRrc3acL4IrdCYuE40sQcalK+/FG3h+eDWQGM1uy Hyng== X-Gm-Message-State: AAQBX9cCrICeEMJRn1B3fIeYpQU8sjeniQbblQwl5VBAe4KVWDEWutTc 4aPznDxlsVV7Q6WTwvYm+Zg0LQ== X-Google-Smtp-Source: AKy350ZpnwLB5iKCMGCbwwvWk9Vdlstn38oUzOZza2h97UWxvDmZVbQkVawtzBlghbIBM/+YOMAKfg== X-Received: by 2002:a05:6402:716:b0:502:2494:b8fc with SMTP id w22-20020a056402071600b005022494b8fcmr15014577edx.7.1680026003884; Tue, 28 Mar 2023 10:53:23 -0700 (PDT) Received: from localhost ([2a02:8070:6387:ab20:5139:4abd:1194:8f0e]) by smtp.gmail.com with ESMTPSA id t27-20020a50ab5b000000b004c0c5864cc5sm16157912edc.25.2023.03.28.10.53.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Mar 2023 10:53:23 -0700 (PDT) Date: Tue, 28 Mar 2023 13:53:22 -0400 From: Johannes Weiner To: Yosry Ahmed Cc: Tejun Heo , Josef Bacik , Jens Axboe , Zefan Li , Michal Hocko , 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 v1 5/9] memcg: replace stats_flush_lock with an atomic Message-ID: References: <20230328061638.203420-1-yosryahmed@google.com> <20230328061638.203420-6-yosryahmed@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230328061638.203420-6-yosryahmed@google.com> X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 902464000E X-Stat-Signature: 6x5no9sgnzaa4t4agty8etoenguy3rti X-HE-Tag: 1680026006-461128 X-HE-Meta: U2FsdGVkX18FPBgXtI4suPBi+b/zvu169pmI8tdB/+6xecaxci2OJdmSV8WZ89zJ5+QZYE83z44IIPesw26oia+iop+jHRxPXxbjQr1otPsuNTGzPwOIno+6AogXTfp+0tfR8gChvaxl9snbZaXBLsyzpep4MpoIXk+LSFqYWd5OhEmSjhQC0oHjZUOSwyVvwELn9+OV/VvrV2uc87xSBNJfC57jDNyy7A6wtDEKE9Rc4pUPNdWcJ4q59+lNBeWDPW3ITDSSDPrO3T0wgOOvoquMjHRVZFC6xg/3cuhgKHVEtRetoQau+cq89vIevXr6zn0vhlaARmf7MYrDe1a0Dp0Oo8c418XukyLq/b8DFBrcHU/CxlryfDbkGTOvbOHyZQe4PpYSks9W2JeWp5oICamfVP8Lu5d7piKscR+7Z02b7dG3GDrwS8iK2IN0hVXtL2ntvowE0c8NQ2vcakbbUN/nF7lNKYOcvcQmaShA3gGBY9LhIIOqvCgQums03w2nyMLctoumUoSjPFz86wc8mw3A/z84YW9OrUTtK4Yes2L6QdhwPSzT5hqWXAZ97IH6pRh1bms+nFSclF5K9DwU6onhV4JD0ZNNVMCPNTMImRZIW4LdPhur89jtO96i7n/EStqsuAvAs9bQemvO9/roVMjFo2SL5lFBWOaCwDK+5Rg4s3yeIM+Cnj3ZmWvhnOD+17wpRuw124t+N/yrhcwRBesxzO3+jTzaVCgKHNgOlIqQnp+yOcbtBitKm5K2KGJW3uqZSWbJiTbQ3wrABVx/MFwhPOMZ7l4XJOVY4xpeUVMeVfetpWM0H1cgnux8otI0t90l4C30JNPItw3LTWspVBG1n+6uAdGlCPHCCRUBwFMkobmDO96Qnqz4kzlmkDYuBH0rs8EwxqbA1SIYVTHkDrLbLP4D9kqmxax54BoUPNd6RWH14YmEZfZCE1VYt7lju8QDnuXNLykjGzCw6nB metp6uyh Qfil9sG4ffczeGsXu/6LnXuhCS4RE9PIfnKwgag3reoe5W/5SvUmCkp+XURpy10aLszOq1DDPB81v0bCSv2UGhkdK/WIFlP5+O8FxrqvT7FAyVOAGWzsiBidy2qCo5OQTupn6qlCu4TXbQzl7knDE8KICnpLF7wb0CDTCFBl1tIQPVAkqybQQuvvB8/Ql5Ap5RGmFUqgDKiM6vWIYlsIDx8DJ50kdp0hK8I/urbZU2ZxLGLic617RLX1mMJG4tw9eiZFHGDFsFmV79fVlqU0U0EJHvLCLLGNmagRPFkTyPygKdyNsggJQdMm3QaUQ5miyu3ynyFNXh2QQTsMQemiP+oidjLZAkmgtfIscT6AzmlbIln5IQ7KF8Dc0ivKWf95/GirhCHgSd9WxIEHD3rNlQ+SGiXLUmPaVdmwR7+c79eAXsFEJvUni31GksHXIOF0ghNGYu1wP+kWp5AYYPyPQlXtCtEEBbBIMLOX/c4EvVMHlspYNi1LVyXn+Jh1iKBPr505b/odpWlVq4o2ugLccfl5OGaNjvpNJXW+XUhSamEqPdDv92CWoJ9IF+JamkDtA7OPvghIOPcLIRqDHioVsWeRtnUvhSZdbR7+B1oeXWvtgO4z/2t2wFyr0gw== 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 Tue, Mar 28, 2023 at 06:16:34AM +0000, Yosry Ahmed wrote: > As Johannes notes in [1], stats_flush_lock is currently used to: > (a) Protect updated to stats_flush_threshold. > (b) Protect updates to flush_next_time. > (c) Serializes calls to cgroup_rstat_flush() based on those ratelimits. > > However: > > 1. stats_flush_threshold is already an atomic > > 2. flush_next_time is not atomic. The writer is locked, but the reader > is lockless. If the reader races with a flush, you could see this: > > if (time_after(jiffies, flush_next_time)) > spin_trylock() > flush_next_time = now + delay > flush() > spin_unlock() > spin_trylock() > flush_next_time = now + delay > flush() > spin_unlock() > > which means we already can get flushes at a higher frequency than > FLUSH_TIME during races. But it isn't really a problem. > > The reader could also see garbled partial updates, so it needs at > least READ_ONCE and WRITE_ONCE protection. > > 3. Serializing cgroup_rstat_flush() calls against the ratelimit > factors is currently broken because of the race in 2. But the race > is actually harmless, all we might get is the occasional earlier > flush. If there is no delta, the flush won't do much. And if there > is, the flush is justified. > > So the lock can be removed all together. However, the lock also served > the purpose of preventing a thundering herd problem for concurrent > flushers, see [2]. Use an atomic instead to serve the purpose of > unifying concurrent flushers. > > [1]https://lore.kernel.org/lkml/20230323172732.GE739026@cmpxchg.org/ > [2]https://lore.kernel.org/lkml/20210716212137.1391164-2-shakeelb@google.com/ > > Signed-off-by: Yosry Ahmed With Shakeel's suggestion: Acked-by: Johannes Weiner