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 A385BCD11DD for ; Fri, 29 Mar 2024 13:02:32 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D34636B007B; Fri, 29 Mar 2024 09:02:31 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CE4DC6B0082; Fri, 29 Mar 2024 09:02:31 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BACAE6B0083; Fri, 29 Mar 2024 09:02:31 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 9E87E6B007B for ; Fri, 29 Mar 2024 09:02:31 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 4153B409F5 for ; Fri, 29 Mar 2024 13:02:31 +0000 (UTC) X-FDA: 81950090502.20.97FD365 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf21.hostedemail.com (Postfix) with ESMTP id A21671C0047 for ; Fri, 29 Mar 2024 13:02:21 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=CkMteOQq; spf=pass (imf21.hostedemail.com: domain of bfoster@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=bfoster@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1711717341; 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=yN1Le78gH2spFQsvHwZ2DbTI7S30kof/5Z3400NZP38=; b=JvcfPIupBPx7fMqBW3QoFS3Fth+Mb10KIwt0KEVVY/QGXh0M2sf9CWJlDwTlImqA6UWX01 8HriYOM2D0b/0qLRaf8nuz4aorAhR9ro0GErFvUf0Ogtk+ChdVXboRUI9xaClMqIHM5uOA BRrMjLVG3DN6l55U4Fot0NxQxX1LdvU= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1711717341; a=rsa-sha256; cv=none; b=0VrLki2kC02Dj/NYXut+EllPvMTCCk4VXrp4MOSRUD3vtuCFCJ7t7hsinZpZQlWViX0kgZ +dexJ53Agg+jqJbRNu8QxoQEGoeK0BJBLpNN2fcIRTh40DIbZsOhjV6jHWvapZuBmLXEez zCfP1Ffv/OAF87GtaHnnWn4HV+sORIU= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=CkMteOQq; spf=pass (imf21.hostedemail.com: domain of bfoster@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=bfoster@redhat.com; dmarc=pass (policy=none) header.from=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1711717340; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=yN1Le78gH2spFQsvHwZ2DbTI7S30kof/5Z3400NZP38=; b=CkMteOQqUqfRvB0hgqjihusiBGbwunN0DzCy9uBDGN8co2e/+ftap+7vZrYvyhsjHxdX6X u29DIkNJDoq9EltnzLdzfHgKicnNZxat+YseI9d3CEJW7i7CTXMqCkZwE/pee2ArpLn/LS ZjGcjbNHQuB8cNOxQ6o3nvnO/2wfN1Y= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-226-Kgz-TJJCPN2MsAeuWC5f0w-1; Fri, 29 Mar 2024 09:02:12 -0400 X-MC-Unique: Kgz-TJJCPN2MsAeuWC5f0w-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 4504C3C02788; Fri, 29 Mar 2024 13:02:12 +0000 (UTC) Received: from bfoster (unknown [10.22.16.57]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9E09040C6CB4; Fri, 29 Mar 2024 13:02:11 +0000 (UTC) Date: Fri, 29 Mar 2024 09:04:09 -0400 From: Brian Foster To: Kemeng Shi Cc: akpm@linux-foundation.org, willy@infradead.org, jack@suse.cz, tj@kernel.org, dsterba@suse.com, mjguzik@gmail.com, dhowells@redhat.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org Subject: Re: [PATCH v2 2/6] writeback: collect stats of all wb of bdi in bdi_debug_stats_show Message-ID: References: <20240327155751.3536-1-shikemeng@huaweicloud.com> <20240327155751.3536-3-shikemeng@huaweicloud.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240327155751.3536-3-shikemeng@huaweicloud.com> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.2 X-Stat-Signature: dofznobrpp3bs6nadcnmbgidbinc4cx5 X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: A21671C0047 X-Rspam-User: X-HE-Tag: 1711717341-483418 X-HE-Meta: U2FsdGVkX191/LPDBBrjv2WZMeqh8VCLh6HykW3YknjRtKRXhLOWpdHHaKNEPGc09kHvJP4qOf7RI+36LlGb/IreB1ZMPpE4MVQgCrNEPxcxC8mJfVjyDudMSFBAiNbdMSR8LKyRDlz/XVHA3fF7OPwVy0MM3Ah+3mh7meIY3CiAZF0bQEWnw+a2jE0vqBu2mxyjzIMJLkLd6Ai4/xZlEgTWUSEOukyOyYc9437q0q9NBfHR4lfFyW0hSvQ1S2+lvbxIuaDeMQS3dCK9oJXfrctMaGnBLEqVL9Pk7VByMHMkqvIrOW5oMcmaNhT7Yw7Hh0MAQC+JXMbYJd6kw1Me8MdlwYsKLQeE84HTVy58yqzmXUgF2VBkmlrUcGr4LLmxbcrdyjAFW9BSmtGrfIiJ8O1AhwJOjfj3bteM6fDq88o8bZidjNjcuqT9iBbnzYEqz7q0Ns5OhcpNtc4yMQvoML63NCM6XW+ZuP06k+ypyOy4uRoPOh0Cutnp30SgwahF5BV50DeX/CkZZfQKqWbiatQrHThdUpjw4WdkfIJuEG41kMbkqTG0C6gnWP75aBpCBki1+SFbHTF9eaT3ZWULfT4x36Ljj32gPtuVvHVEDrkDIGl3vNiNxxgB5nY2hftulZAH/b2qXOxZ/+uBYac969wvp96yuYO+pce9HaY3JbmbcCcAP2dxTOxXvheabke8mZB/5f8st4cX/DN/OcKH4PLtHCS2EtHeyF7za87xVpy+TUMi0A8ZHFhit0yEWHjj9lyMJENa4UkiroUdJXNPSofTyC6I49m1Z52z+d6F9qZPwdsvwIDEZpUCqm7Y7Ogl+rD6WQG0Mh/AUKflGAo/dR430+GJnmt2QH9kvm2GEYxzuDYYH7CvakuxttCOL2I1CRuHY1cltULOPixetlcIEA== 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: List-Subscribe: List-Unsubscribe: On Wed, Mar 27, 2024 at 11:57:47PM +0800, Kemeng Shi wrote: > /sys/kernel/debug/bdi/xxx/stats is supposed to show writeback information > of whole bdi, but only writeback information of bdi in root cgroup is > collected. So writeback information in non-root cgroup are missing now. > To be more specific, considering following case: > > /* create writeback cgroup */ > cd /sys/fs/cgroup > echo "+memory +io" > cgroup.subtree_control > mkdir group1 > cd group1 > echo $$ > cgroup.procs > /* do writeback in cgroup */ > fio -name test -filename=/dev/vdb ... > /* get writeback info of bdi */ > cat /sys/kernel/debug/bdi/xxx/stats > The cat result unexpectedly implies that there is no writeback on target > bdi. > > Fix this by collecting stats of all wb in bdi instead of only wb in > root cgroup. > > Following domain hierarchy is tested: > global domain (320G) > / \ > cgroup domain1(10G) cgroup domain2(10G) > | | > bdi wb1 wb2 > > /* all writeback info of bdi is successfully collected */ > cat stats > BdiWriteback: 2912 kB > BdiReclaimable: 1598464 kB > BdiDirtyThresh: 167479028 kB > DirtyThresh: 195038532 kB > BackgroundThresh: 32466728 kB > BdiDirtied: 19141696 kB > BdiWritten: 17543456 kB > BdiWriteBandwidth: 1136172 kBps > b_dirty: 2 > b_io: 0 > b_more_io: 1 > b_dirty_time: 0 > bdi_list: 1 > state: 1 > > Signed-off-by: Kemeng Shi > --- > mm/backing-dev.c | 100 +++++++++++++++++++++++++++++++++-------------- > 1 file changed, 71 insertions(+), 29 deletions(-) > > diff --git a/mm/backing-dev.c b/mm/backing-dev.c > index 70f02959f3bd..8daf950e6855 100644 > --- a/mm/backing-dev.c > +++ b/mm/backing-dev.c ... > @@ -65,16 +78,54 @@ static struct backing_dev_info *lookup_bdi(struct seq_file *m) > return NULL; > } > > +static void collect_wb_stats(struct wb_stats *stats, > + struct bdi_writeback *wb) > +{ > + struct inode *inode; > + > + spin_lock(&wb->list_lock); > + list_for_each_entry(inode, &wb->b_dirty, i_io_list) > + stats->nr_dirty++; > + list_for_each_entry(inode, &wb->b_io, i_io_list) > + stats->nr_io++; > + list_for_each_entry(inode, &wb->b_more_io, i_io_list) > + stats->nr_more_io++; > + list_for_each_entry(inode, &wb->b_dirty_time, i_io_list) > + if (inode->i_state & I_DIRTY_TIME) > + stats->nr_dirty_time++; > + spin_unlock(&wb->list_lock); > + > + stats->nr_writeback += wb_stat(wb, WB_WRITEBACK); > + stats->nr_reclaimable += wb_stat(wb, WB_RECLAIMABLE); > + stats->nr_dirtied += wb_stat(wb, WB_DIRTIED); > + stats->nr_written += wb_stat(wb, WB_WRITTEN); > + stats->wb_thresh += wb_calc_thresh(wb, stats->dirty_thresh); Kinda nitty question, but is this a sum of per-wb writeback thresholds? If so, do you consider that useful information vs. the per-wb threshold data presumably exposed in the next patch? I'm not really that worried about what debug data we expose, it just seems a little odd. How would you document this value in a sentence or two, for example? > +} > + > +#ifdef CONFIG_CGROUP_WRITEBACK > +static void bdi_collect_stats(struct backing_dev_info *bdi, > + struct wb_stats *stats) > +{ > + struct bdi_writeback *wb; > + > + list_for_each_entry_rcu(wb, &bdi->wb_list, bdi_node) > + collect_wb_stats(stats, wb); Depending on discussion on the previous patch and whether the higher level rcu protection in bdi_debug_stats_show() is really necessary, it might make more sense to move it here. I'm also wondering if you'd want to check the state of the individual wb (i.e. WB_registered?) before reading it..? > +} > +#else > +static void bdi_collect_stats(struct backing_dev_info *bdi, > + struct wb_stats *stats) > +{ > + collect_wb_stats(stats, &bdi->wb); > +} > +#endif ... > @@ -115,18 +157,18 @@ static int bdi_debug_stats_show(struct seq_file *m, void *v) > "b_dirty_time: %10lu\n" > "bdi_list: %10u\n" > "state: %10lx\n", > - (unsigned long) K(wb_stat(wb, WB_WRITEBACK)), > - (unsigned long) K(wb_stat(wb, WB_RECLAIMABLE)), > - K(wb_thresh), > + K(stats.nr_writeback), > + K(stats.nr_reclaimable), > + K(stats.wb_thresh), > K(dirty_thresh), > K(background_thresh), > - (unsigned long) K(wb_stat(wb, WB_DIRTIED)), > - (unsigned long) K(wb_stat(wb, WB_WRITTEN)), > - (unsigned long) K(wb->write_bandwidth), > - nr_dirty, > - nr_io, > - nr_more_io, > - nr_dirty_time, > + K(stats.nr_dirtied), > + K(stats.nr_written), > + K(tot_bw), > + stats.nr_dirty, > + stats.nr_io, > + stats.nr_more_io, > + stats.nr_dirty_time, > !list_empty(&bdi->bdi_list), bdi->wb.state); Is it worth showing a list count here rather than list_empty() state? Brian > > rcu_read_unlock(); > -- > 2.30.0 >