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 E2212C76196 for ; Tue, 28 Mar 2023 19:34:55 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 816016B0071; Tue, 28 Mar 2023 15:34:55 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 79D986B0072; Tue, 28 Mar 2023 15:34:55 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6653A6B0074; Tue, 28 Mar 2023 15:34:55 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 534936B0071 for ; Tue, 28 Mar 2023 15:34:55 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 0BE47140BEA for ; Tue, 28 Mar 2023 19:34:55 +0000 (UTC) X-FDA: 80619309750.26.C12B4D7 Received: from mail-ed1-f51.google.com (mail-ed1-f51.google.com [209.85.208.51]) by imf19.hostedemail.com (Postfix) with ESMTP id 326B01A0019 for ; Tue, 28 Mar 2023 19:34:52 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=dHoOF8rN; spf=pass (imf19.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.51 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1680032093; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=PCcvIywQQCX7FP21Q6hk+wcOp0VhtxDKF+8JhJGEAfQ=; b=OGXTUhDx5iydqRGSXdZHkW8ovT5VgGtF7q5ECqGTK6BvagKzLpuqjUpaD8nrSRlIyFHp2K WHvTltMUFxhk0JWVIUhFkmrlQcuWyZKQINdHgfUXIo5FZ2M9tNN168AEzZUaKnxmj5h4Ov LwcWMBrOekVs0OTF1QddgzJfjn2Icv4= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=dHoOF8rN; spf=pass (imf19.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.51 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1680032093; a=rsa-sha256; cv=none; b=vJ5zcPwnQIzHBsbtTcjV2ADFhAXwj9ymNqiiJ/IAo9XrlgdR5+uXehr6Jhgciv+2YRISeQ GPKYSuiIJHLRfedbedW+jEpQT1HYpTXXgex2gPVdYTI2mJk7+6Jg/bEQEUL1tgTrEONkYd HBi/BvDuPWHS43Au79/M6To6SBkGMOM= Received: by mail-ed1-f51.google.com with SMTP id cn12so54184531edb.4 for ; Tue, 28 Mar 2023 12:34:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1680032092; 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=PCcvIywQQCX7FP21Q6hk+wcOp0VhtxDKF+8JhJGEAfQ=; b=dHoOF8rNOSkdjd1rnnf8XC6IGb6FKwmM7Ivdvjk2GQi/EbVyThzjBYyHQXopuJULIg 3p1bzuC4gKKNl6Fqhda2du3Rr+s6FdBcg4aXKrD1ycu2zktIM3FTOAoGmv4pZHnjujsT rvqc9SbA4NjhcACriIJnLeYfMpWyuhW0j5mUqMiEJuGYX3I6Vm1bge9vyTRqSfTQ+kFD hcNKe+61X/fdB+69ZT/PWtpn27JcWOnhGjSUdRlOVudULYhr4IE9KPo4WrZdFpVOufcb x3G5vlvhcJZuXWJ1jNnDtE5NmkzFKju+dq8flQe22Sd56znLfGyB2CeL5tXa6i93FNYi J7Xg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680032092; 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=PCcvIywQQCX7FP21Q6hk+wcOp0VhtxDKF+8JhJGEAfQ=; b=Ny/WOgunyWYzLyWXENnaRQqJYdabHFVE9/hqXnuLSlAdujcM5C65uQqHjZh4WiDk8A DgiwuVt1hYJTgBhB9eZ517CU8EZIvjMJ1Bqa1e5XTUCWQ8NEQCqq5j47bZiA+FmkoVN9 zyUuxaKqKz///wp6CH431A6s1ItVDqcpIKC0wiObVa15AB6eP3+RQmj04uo1CAWmwnxD 0+miyn16A2K+zEN6i7YuedaFGk9F7LClhLhHBZ63AaxbSiJ/esbPXbM9MxOycW8bfWto uMxR1ya6H8JlHh1fpJfnpfYZDZKOrQy0i4t59iDcKKJeGOkrXi2KOsBmaO4NMLj85Keg 8eyw== X-Gm-Message-State: AAQBX9fkXK+DjjYrRRIS1I33dltqXjo4HDg1kiB1bWBhy84g2NY0FAOI ss5fyuVqxQPhJX27KszwaSZ/GiWSTjHZ7rzILBbGbA== X-Google-Smtp-Source: AKy350YOd+MIycwAtui+juLEs+/XeA1BayeLMmg6/SLyZm36OoLT9BnRDKlngVlZ55Qf5nONmzBspfNUTrbsKzLJWs4= X-Received: by 2002:a50:d581:0:b0:502:1d1c:7d37 with SMTP id v1-20020a50d581000000b005021d1c7d37mr8497378edi.8.1680032091630; Tue, 28 Mar 2023 12:34:51 -0700 (PDT) MIME-Version: 1.0 References: <20230328061638.203420-1-yosryahmed@google.com> <20230328061638.203420-6-yosryahmed@google.com> <20230328141523.txyhl7wt7wtvssea@google.com> In-Reply-To: From: Yosry Ahmed Date: Tue, 28 Mar 2023 12:34:15 -0700 Message-ID: Subject: Re: [PATCH v1 5/9] memcg: replace stats_flush_lock with an atomic To: Shakeel Butt Cc: Tejun Heo , Josef Bacik , Jens Axboe , Zefan Li , Johannes Weiner , Michal Hocko , Roman Gushchin , Muchun Song , Andrew Morton , =?UTF-8?Q?Michal_Koutn=C3=BD?= , 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 X-Stat-Signature: kuhtcjjdwmth36sap8oqs6o5oz15r9j9 X-Rspam-User: X-Rspamd-Queue-Id: 326B01A0019 X-Rspamd-Server: rspam06 X-HE-Tag: 1680032092-68712 X-HE-Meta: U2FsdGVkX18xtYP5e02YOvf0lLXU964SPLGcNrLiKyMIraq0urfpd6aU9O2Gy2OzwEihnn+6HiZ+2s34iKsKm6JeS/bqnFgTKMFEIveF691jpdpZYlB/BTkqJpOcYHLFz4BtGA08hhUdo+w5l6WuiFEY2BygOvJHan+zoiiQQdQx5ZEM3HZMZFSoBWhCzG7+UnKbyCTu5SM9C0tLsKcJAjKLduZ1QZNJxuRhsejfhf/gzTAV/7gVYJ9OgSQ4/WcAHkkMk77A9knro92grfVH7xhyKrLC7SSvgz6jMYaVCKH60brJVibEs0olrZ+4hiy5Wvoih7oCrJQ3/GDpEp/gns/P3APGr2DAIQkDax2HTx+3dZvY055UtO0A8AzKjP2iT7ot7RW4bHH1SOl0b41hlbO50ax7sqWTA34mKpf5bmew/6W16M8VqAsBTI+nOIEhdeAoeBlWUOD+HsScrh99CmpDQaXN7WMw58o9wVZu/VbRs5kmIINDGrcyxRB96wvU8Ivd3hQnhTX8bHZjjGFtNwnIFTYqXsp+7+X7iKOBQ7WD+kXcdBeRCk2yAco+7CfCgVZA5vrvaQPtL5/bG9vDI2YntkE3A8O8hYkOPP0wq9FLmQDPlUucSCG22Z0w+jyk7I2OOekPGSdVh4tQIIPGOUpxacNk1ANdrSYEehZOwVwsAqf4YKsKF+iz1hDc2dzoAE3ZQ4dDH7fl3qhh23WzTxar0v/GQsgYe/3w3Qvz2coSD8kv04OVPI0yR1GlC+1L7rRimMqpa4y1lkDg096lTnEFjofXVob24ZJUPT7FPcPL7oCs2F54opxu/e8vXovlCVFUsfiznVs29xTYcgVwDdVMt7XJaxO4jFuqf3OPC1CvnXsBOEz2XwlpbXCv7umBiSM1L81RBiW/Y6BeX2HYGrRHh828u6k8dXXcBc5x5ooAFQnmovt7QvAolXfB8sIndXmBqk1+/d0AquCwehV bcvcYjgh GZiJ5iyX4rsRU6+wRPwy3UoQih90uWvXbx8EXLngbVGvguWIrQKHL9hi5hBtI3qsImdpmVNwtfhPQnuzVIs2h0Dm3kBtOpzeMwfCW6yo3bEEYoQNMx3bTgoXtnm81K79N2sGpb+L9NGEv0s0bTr2ul8FjeGZTSO/mL+tm3GtXys2fnnt6AWVKDZ7ApHU9CmcXBvz0zeEm81fsEZxFi/h4XqaVHTBG4Yh9oiDJcUCXX3ECF3+79WLAZdRVmIoZM9thni8cn92PAAAGwlqIFA3zUbQreOwLsq5B7p0x2BTckjxsStIrGMXkOYnjlg== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000006, 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 12:28=E2=80=AFPM Shakeel Butt = wrote: > > On Tue, Mar 28, 2023 at 11:53=E2=80=AFAM Yosry Ahmed wrote: > > > [...] > > > > + if (atomic_xchg(&stats_flush_ongoing, 1)) > > > > > > Have you profiled this? I wonder if we should replace the above with > > > > > > if (atomic_read(&stats_flush_ongoing) || atomic_xchg(&stats_f= lush_ongoing, 1)) > > > > I profiled the entire series with perf and I haven't noticed a notable > > difference between before and after the patch series -- but maybe some > > specific access patterns cause a regression, not sure. > > > > Does an atomic_cmpxchg() satisfy the same purpose? it's easier to read > > / more concise I guess. > > > > Something like > > > > if (atomic_cmpxchg(&stats_flush_ongoing, 0, 1)) > > > > WDYT? > > > > No, I don't think cmpxchg will be any different from xchg(). On x86, > the cmpxchg will always write to stats_flush_ongoing and depending on > the comparison result, it will either be 0 or 1 here. > > If you see the implementation of queued_spin_trylock(), it does the > same as well. Interesting. I thought cmpxchg by definition will compare first and only do the write if stats_flush_ongoing =3D=3D 0 in this case. I thought queued_spin_trylock() was doing an atomic_read() first to avoid the LOCK instruction unnecessarily the lock is held by someone else.