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 X-Spam-Level: X-Spam-Status: No, score=-12.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3EEA1C433B4 for ; Mon, 19 Apr 2021 07:00:24 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id C53E66101C for ; Mon, 19 Apr 2021 07:00:23 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C53E66101C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 596D66B0036; Mon, 19 Apr 2021 03:00:23 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 51F576B006E; Mon, 19 Apr 2021 03:00:23 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 371946B0070; Mon, 19 Apr 2021 03:00:23 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0143.hostedemail.com [216.40.44.143]) by kanga.kvack.org (Postfix) with ESMTP id 11FCB6B0036 for ; Mon, 19 Apr 2021 03:00:23 -0400 (EDT) Received: from smtpin37.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id BA306181AEF39 for ; Mon, 19 Apr 2021 07:00:22 +0000 (UTC) X-FDA: 78048217884.37.77654C2 Received: from mail-pf1-f179.google.com (mail-pf1-f179.google.com [209.85.210.179]) by imf17.hostedemail.com (Postfix) with ESMTP id AFBF0407F8F3 for ; Mon, 19 Apr 2021 07:00:19 +0000 (UTC) Received: by mail-pf1-f179.google.com with SMTP id h15so4863048pfv.2 for ; Mon, 19 Apr 2021 00:00:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=Dk2/9Hs4gCeLuOk2z2sMzsyg1GMNTelWF08Mrd5lMC0=; b=d9obWs3erdjaKB6VgVfN7hy1egp8tivwn9bXkq7q5F9jJtVaDnpFTJBSxhhLTA2Pg/ H3p+JIDvhuA8XTUFOEH3XjXsalKtje4qi1sWp1KUxwrNWGXEZIgp/rKJdVlfLg6CpOK0 DeDP/CDWmNG28XBcCkoK5hBnHhYphWZmpB/evs5MEj6WQ5A78FjgBMP2XResZ/rQCn1f eUKbq7JMHCBCpq5BSs7GVjBZnzb5zED/TYDhb9OIJnbnI6wx00uPnLoaDZYGjA4ShELz kDu7y5cZZgMXdXKRKfwDMW8yy2njA6WgI14Dmc6warh3kGmqgoBx6rE2A06olC2TOekZ pwEw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=Dk2/9Hs4gCeLuOk2z2sMzsyg1GMNTelWF08Mrd5lMC0=; b=CQWAspKPWyF8Tlm0ly1NtUtfuzcwrFzhqjXV0LLGfZpQ2Ty/Vk1/ba6ZO8UVbx0UtH SWqBgWYqp5swp4TQ9fb1aUTFQLtFs+vhBf6Jusw1upJzoF7kavCeQnsJ2cgE+b2Z4PDo oo2sqjCU4L8Gy3NNakUBOSiEjxBigcdmYtISt7j62J5OBXdhN7wx063L7mMmWpNPIE2W 6qfs9vWDEvUV14Qa1977athBAA+Trozz4Ui0Gl1YkVtvfC9+2a/noMgvF5edRDoPHpy9 VUgQdlMd02Smqacfp9Y5vXJylKf8paNi21KealUOYFEjYdo4vNpRfdyjrPlK3lCx8XNf lkqA== X-Gm-Message-State: AOAM532V7GfjyWh+GnRZkznKogrDpN61drYF0KNj9SufiDUPtsgtrLCy ro1dilFBYukvsMoBftyLUjE= X-Google-Smtp-Source: ABdhPJxJgy8WLzl6Rfn3UohRDC8bRZXk1wE5VqtWIe0usLH93JizsWl45G9idOtRf9D3ibdrbUPLJw== X-Received: by 2002:a05:6a00:856:b029:208:f11c:231f with SMTP id q22-20020a056a000856b0290208f11c231fmr18143818pfk.25.1618815621154; Mon, 19 Apr 2021 00:00:21 -0700 (PDT) Received: from localhost (121-45-173-48.tpgi.com.au. [121.45.173.48]) by smtp.gmail.com with ESMTPSA id y24sm10449927pjp.26.2021.04.19.00.00.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 19 Apr 2021 00:00:20 -0700 (PDT) Date: Mon, 19 Apr 2021 17:00:16 +1000 From: Balbir Singh To: Yafang Shao Cc: akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Tejun Heo , Josh Snyder Subject: Re: [PATCH] delayacct: clear right task's flag after blkio completes Message-ID: <20210419070016.GB8178@balbir-desktop> References: <20210414083720.24083-1-laoar.shao@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210414083720.24083-1-laoar.shao@gmail.com> X-Rspamd-Queue-Id: AFBF0407F8F3 X-Stat-Signature: xpegs6betm6kh1nsyqmqjezc7ws5uxpr X-Rspamd-Server: rspam02 Received-SPF: none (gmail.com>: No applicable sender policy available) receiver=imf17; identity=mailfrom; envelope-from=""; helo=mail-pf1-f179.google.com; client-ip=209.85.210.179 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1618815619-687020 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 Wed, Apr 14, 2021 at 04:37:20PM +0800, Yafang Shao wrote: > When I was implementing a latency analyze tool by using task->delays and > other things, I found there's issue in delayacct. The issue is it should > clear the target's flag instead of current's in delayacct_blkio_end(). > > When I git blame delayacct, I found there're some similar issues we have > fixed in delayacct_blkio_end(). > 'Commit c96f5471ce7d ("delayacct: Account blkio completion on the correct task")' > fixed the issue that it should account blkio completion on the target > task instead of current. > 'Commit b512719f771a ("delayacct: fix crash in delayacct_blkio_end() after delayacct init failure")' > fixed the issue that it should check target task's delays instead of > current task'. It seems that delayacct_blkio_{begin, end} are error prone. > So I introduce a new paratmeter - the target task 'p' into these helpers, > after that change, the callsite will specifilly set the right task, which > should make it less error prone. > > Signed-off-by: Yafang Shao > Cc: Tejun Heo > Cc: Josh Snyder > --- > include/linux/delayacct.h | 20 ++++++++++---------- > mm/memory.c | 8 ++++---- > 2 files changed, 14 insertions(+), 14 deletions(-) > > diff --git a/include/linux/delayacct.h b/include/linux/delayacct.h > index 2d3bdcccf5eb..21651f946751 100644 > --- a/include/linux/delayacct.h > +++ b/include/linux/delayacct.h > @@ -82,16 +82,16 @@ static inline int delayacct_is_task_waiting_on_io(struct task_struct *p) > return 0; > } > > -static inline void delayacct_set_flag(int flag) > +static inline void delayacct_set_flag(struct task_struct *p, int flag) > { > - if (current->delays) > - current->delays->flags |= flag; > + if (p->delays) > + p->delays->flags |= flag; > } > > -static inline void delayacct_clear_flag(int flag) > +static inline void delayacct_clear_flag(struct task_struct *p, int flag) > { > - if (current->delays) > - current->delays->flags &= ~flag; > + if (p->delays) > + p->delays->flags &= ~flag; > } > > static inline void delayacct_tsk_init(struct task_struct *tsk) > @@ -114,7 +114,7 @@ static inline void delayacct_tsk_free(struct task_struct *tsk) > > static inline void delayacct_blkio_start(void) > { > - delayacct_set_flag(DELAYACCT_PF_BLKIO); > + delayacct_set_flag(current, DELAYACCT_PF_BLKIO); > if (current->delays) > __delayacct_blkio_start(); > } > @@ -123,7 +123,7 @@ static inline void delayacct_blkio_end(struct task_struct *p) > { > if (p->delays) > __delayacct_blkio_end(p); > - delayacct_clear_flag(DELAYACCT_PF_BLKIO); > + delayacct_clear_flag(p, DELAYACCT_PF_BLKIO); > } > > static inline int delayacct_add_tsk(struct taskstats *d, > @@ -166,9 +166,9 @@ static inline void delayacct_thrashing_end(void) > } > > #else > -static inline void delayacct_set_flag(int flag) > +static inline void delayacct_set_flag(struct task_struct *p, int flag) > {} > -static inline void delayacct_clear_flag(int flag) > +static inline void delayacct_clear_flag(struct task_struct *p, int flag) > {} > static inline void delayacct_init(void) > {} > diff --git a/mm/memory.c b/mm/memory.c > index 550405fc3b5e..a013d32a6611 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -3296,7 +3296,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) > } > > > - delayacct_set_flag(DELAYACCT_PF_SWAPIN); > + delayacct_set_flag(current, DELAYACCT_PF_SWAPIN); > page = lookup_swap_cache(entry, vma, vmf->address); > swapcache = page; > > @@ -3347,7 +3347,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) > vmf->address, &vmf->ptl); > if (likely(pte_same(*vmf->pte, vmf->orig_pte))) > ret = VM_FAULT_OOM; > - delayacct_clear_flag(DELAYACCT_PF_SWAPIN); > + delayacct_clear_flag(current, DELAYACCT_PF_SWAPIN); > goto unlock; > } > > @@ -3361,13 +3361,13 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) > * owner processes (which may be unknown at hwpoison time) > */ > ret = VM_FAULT_HWPOISON; > - delayacct_clear_flag(DELAYACCT_PF_SWAPIN); > + delayacct_clear_flag(current, DELAYACCT_PF_SWAPIN); > goto out_release; > } > > locked = lock_page_or_retry(page, vma->vm_mm, vmf->flags); > > - delayacct_clear_flag(DELAYACCT_PF_SWAPIN); > + delayacct_clear_flag(current, DELAYACCT_PF_SWAPIN); > if (!locked) { > ret |= VM_FAULT_RETRY; > goto out_release; Acked-by: Balbir Singh The changes seem reasonable to me. I don't maintain a git tree, Andrew can we please queue them up in your tree? Balbir Singh.