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 DA7E4C3DA49 for ; Tue, 30 Jul 2024 20:15:26 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4B2266B0083; Tue, 30 Jul 2024 16:15:26 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 462566B0085; Tue, 30 Jul 2024 16:15:26 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 350986B0089; Tue, 30 Jul 2024 16:15:26 -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 17CB56B0083 for ; Tue, 30 Jul 2024 16:15:26 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id B9B6FA4C39 for ; Tue, 30 Jul 2024 20:15:25 +0000 (UTC) X-FDA: 82397523810.13.3145DA8 Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by imf17.hostedemail.com (Postfix) with ESMTP id 80F2840032 for ; Tue, 30 Jul 2024 20:15:23 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="WQOaYe/v"; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf17.hostedemail.com: domain of hawk@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=hawk@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722370483; a=rsa-sha256; cv=none; b=Z8d1mA/toeLD0K2J5VvlTP0E7Ab1TAshTAYkoeU+/9EFlzkha5n2F0PCaqpo5ezUr0T3ld xx//Yc7UbIVsqv10kioJnenriKSyqpyrexYyxrH8OqLBP/e/CfDjujuxCy4JhL5oWmRWvJ BtoGWEIh99Jjs9rNyrDtclX0etup1V4= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="WQOaYe/v"; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf17.hostedemail.com: domain of hawk@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=hawk@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1722370483; 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=Gsi+xEP+Gv9H0projqmS21UgEUytcga+s7Vq8js6SA8=; b=zAx8p2X2m2IN4KgglfrAFPrXU/iPtGykQCLgOtcQCmdDOqXGXIdVHAzqIzQdFSdRK2e5hE gthwL02L9NNoLCh8vPAeQeRLMkq/X0SQO+yE7QQ9454emsuN0McXQMIrSfwY3QrkKldhQq qBgU3A+1cIPqJwHTCY7SOzGrElZJtaA= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id A0D5ECE119B; Tue, 30 Jul 2024 20:15:20 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id F3108C4AF0B; Tue, 30 Jul 2024 20:15:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1722370520; bh=ay6hgAnQAQnA6PKOJXLf2wJGdfKc4x05GCOcouY6KOk=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=WQOaYe/voS7e7yPBBf26uuqMHv4nTNggvLSEufYWjnigpEbzGc7bbqHfMaCwbs7tR c/bL9Tm68bYx+kYM2YpcoWn2tjjLxkc540s+EWmz1FDh8pnMStVOKnb+kfpytFly3S vLkrnkDMn0VxwamptGIDXZdI8FF5X68dgkFhBEEZb+/jKmJ0PaGUaPH/XE9uj3eArW AinIQP/guYZ6Y9Lnfq8K1O9kbffbQW2lQ4Ob0zFoo/AXGrpEB7vxxBbDdbFVJqMZKI GT2I6sqQrrrlvwIRv7EYpwdhnzsflu2ytP4QO9CmFO23dyN5xin51/nE2ZjRNUD83i phxECXErqFHMA== Message-ID: <3fd88047-db3f-4165-9b58-fdeb5413c1a6@kernel.org> Date: Tue, 30 Jul 2024 22:15:16 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH V8 1/2] cgroup/rstat: Avoid flushing if there is an ongoing overlapping flush To: Yosry Ahmed Cc: tj@kernel.org, cgroups@vger.kernel.org, shakeel.butt@linux.dev, hannes@cmpxchg.org, lizefan.x@bytedance.com, longman@redhat.com, kernel-team@cloudflare.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <172139415725.3084888.13770938453137383953.stgit@firesoul> Content-Language: en-US From: Jesper Dangaard Brouer In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 80F2840032 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: biky88s19mst14cws8fwiyn1wbidowg5 X-HE-Tag: 1722370523-481404 X-HE-Meta: U2FsdGVkX19tswxSrcuRbY1J0DT08Qr91KUzGlj2CrUCTiwTckJoUs4LZDi15uGlv0hbR6wiwjRDEgoDl6qF00qI0SoJyw3Y+nxPnCcDgZieE/wlzEjbkBX1UgpDeRk4xmKZ4z3d0av/jcFLUn5ZvjrCHCmZJ2Gs9imr+lu6WM3w1Ksdos+lKhkqaqT2EzWPhyxX4GSazqMY2YNWUgxVnvUj6HdL4VvPtSJEX7kltmXsXNIGh9nEQOlymqCCScZQpUhMbE98EM7Rr5nR2CpkaCm3SqDHzHhDeYxL6WsP3UJypnDwyKAlw89iORe6XlpQM4rLDVR3AKA3F+V3TARXtkBoEEHsJYN4hg5xvfJIIykYwBgKydOv3hp5sYumsG9+dY2oh2j7PY9ZTADKhs9ZjdggI59w4Xdb6AFTOQsGxBDEdnJ8aUbvpF7X+1ypSgl9742dw2TUw/4uUglCsBtRp6P/MRwDncdIykQaomO8zmjHgfm/IuBwFYNTnWBj3DPlpkoUDNZ2KRmXsU+q+JZTDVk87MG3dRjlHo02vd6TwbPleeh0MWezVc3cx0AERonxs4fu0s+Pn+V6nPfWZ7GdP3S6oLax30LLcIP+jxdgtXNmKs3cOWABUcYicZEipcsJi04yWna2fpazaFc8IGefwqQdpXsNTN/Hjf9z6qJATvXwUwqCBrG7BHUdDxlH9WjRvLYkt5ATlXy9gabe0WX+PJfXkfTBDact1v5eF4RPKSeI+Bpu8fLEpPxWa+LhDm2aEo6OIMw9jsq3NSLuEQAxlx3ncALv8jo3Tl1SyqqVVkyh0IbI2Olg+eQJxa7epdExOLLM8YXpUlJ4vvCgxVAvAAISqmbZU82wx/BXLDOx5LC9xkcvQRaAMlZpz1NZUi/26k/6MXk53vHZvZMGoR5Fwf6IvEe4hBRfWZBQkmYi0r9cZi20+lc4kV+5bRCBoEewxc7L96/vwOVArJ6vuzJ VSdPcSdr 5icM/enEfr5dvO1OouBzXRSE1Q2fCdvsAiSxNuvsRu5ZPTZjEsc2aZJOd+HWh7bk8ouoW7yZpRA+O6MXf58exxbwdnWg87nGUk4sUaBVY7e6snJ8vxVY6wXutFFAv+ccV3igMZcQngiInfQOfMa6vVmuF4s7xgr7K+serwLieIvSDRTOF8a7IGZf6XA6Jyx8JiP+2uNQcuToQFYNMe+2Piu5EzojWm0upW6vkJ/P6BA01lqBLYnHYa6pt7Q== X-Bogosity: Ham, tests=bogofilter, spamicity=0.152458, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 30/07/2024 20.54, Yosry Ahmed wrote: > [..] >>>> +static inline void __cgroup_rstat_lock(struct cgroup *cgrp, int cpu_in_loop, >>>> + bool already_contended) >>>> __acquires(&cgroup_rstat_lock) >>>> { >>>> - bool contended; >>>> + bool locked = false; >>>> >>>> - contended = !spin_trylock_irq(&cgroup_rstat_lock); >>>> - if (contended) { >>>> - trace_cgroup_rstat_lock_contended(cgrp, cpu_in_loop, contended); >>>> + if (already_contended) /* Skip trylock if already contended */ >>>> + locked = __cgroup_rstat_trylock(cgrp, cpu_in_loop); >>> Should this be the other way around? >>> >> I think it is correct, but I used it wrong in once place, in >> cgroup_rstat_flush_hold(), as cgroup_rstat_trylock_flusher() returning >> false doesn't mean it was already_contended, but that ongoing flusher >> "skipped" (and waited for) a flush. I need to correct this. > > Something isn't adding up here as well. The comment says skip trylock > if already contended, then if already_contended is true we do a > trylock. Am I confusing myself here? 🙂 Your are correct. Thanks you for spelling it out for me! I will send a V9 tomorrow, then deploy it to my prod experiment hosts, and retest as I think my mistake here affects the prod results, as some of the tracepoints gets skipped due to this. Again thanks for catching this!!! --Jesper