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 AD24DC54E67 for ; Wed, 20 Mar 2024 13:19:18 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 22DBB6B0082; Wed, 20 Mar 2024 09:19:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1DDF46B0088; Wed, 20 Mar 2024 09:19:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0A7616B0089; Wed, 20 Mar 2024 09:19:18 -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 EB5186B0082 for ; Wed, 20 Mar 2024 09:19:17 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id C08EEA1BBD for ; Wed, 20 Mar 2024 13:19:17 +0000 (UTC) X-FDA: 81917473554.15.2F394D5 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf28.hostedemail.com (Postfix) with ESMTP id EA9D0C0003 for ; Wed, 20 Mar 2024 13:19:15 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=hYAOt8uO; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf28.hostedemail.com: domain of bfoster@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=bfoster@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1710940756; a=rsa-sha256; cv=none; b=3GgpfwomO7R+v6v4fX7j6YfFudiNAd5B9aK8e86R0N6MQhRIpaKCVn8hzQ++M/Uta/vgcr DuX6ilNi7cqPj+wvQC8HiF4NrHjILLf+ay3WckxY2AxgUG3NB+LnWoUck71U7/x8RY90gr LTqhpP23tSfFO/ATbl5EIhJdVyZlAnA= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=hYAOt8uO; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf28.hostedemail.com: domain of bfoster@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=bfoster@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1710940756; 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=NpmVP7TeXED0UIOg2IhouFjCmnaB0b7XL3z6nYN+g5c=; b=cwm6GGORyWcOg0fF+IhsqAY1D7eK4e4bYPUVuZgm0hV+Po8kGl8E+DUL20yB7HSt61Xyei KWWUjS0OchWYWGaLmWYFulStOjGTn86Sb39QfmYUrT20GYr3/HAWoZhJN9pgaaqY+GSEio JpkYPtOiwsJtbrpFTwlZXWcNgkS+5GE= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1710940755; 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=NpmVP7TeXED0UIOg2IhouFjCmnaB0b7XL3z6nYN+g5c=; b=hYAOt8uOWOgx9MwC2TcerGQA0NLN2V1aPNTK7y7BoW2/X3nzeatsuO9yrOJ3Ad2ACdJ/w6 w133V2+CC49k1tdzHzGtpYQgjE8RbQm8jvKhi2JSD17rq+29RRryKSijSLQXPaNDM1ckMZ 9jWIIkn7Im3oH5QTyBnM9iou5hZKy6k= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-225-wbANGGN0OhS4SocgmX5KQQ-1; Wed, 20 Mar 2024 09:19:12 -0400 X-MC-Unique: wbANGGN0OhS4SocgmX5KQQ-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (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 DF861800266; Wed, 20 Mar 2024 13:19:11 +0000 (UTC) Received: from bfoster (unknown [10.22.16.57]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 5F7E22166B34; Wed, 20 Mar 2024 13:19:11 +0000 (UTC) Date: Wed, 20 Mar 2024 09:21:05 -0400 From: Brian Foster To: Kemeng Shi Cc: akpm@linux-foundation.org, tj@kernel.org, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, willy@infradead.org, jack@suse.cz, dsterba@suse.com, mjguzik@gmail.com, dhowells@redhat.com, peterz@infradead.org Subject: Re: [PATCH 1/6] writeback: collect stats of all wb of bdi in bdi_debug_stats_show Message-ID: References: <20240320110222.6564-1-shikemeng@huaweicloud.com> <20240320110222.6564-2-shikemeng@huaweicloud.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240320110222.6564-2-shikemeng@huaweicloud.com> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.6 X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: EA9D0C0003 X-Stat-Signature: mmwy89a1fydmw4pgri4sfriyamjo7k96 X-HE-Tag: 1710940755-298163 X-HE-Meta: U2FsdGVkX1+ZgtuECdGrsloVqTVYKX9Ge+FsODEcSwDHEBbIB2ldfat0IqbGlyOuW7P6s0vdrGe9RihbBTd/rM/I86k8XEuMnrD4pzl/HI/1hKTNeqKEtiElDnCwtKU77qxbyXQbBIpbqn5adaHU4WxidZY2kTCrbQX3iC6q42ur+vEaDdule3Ec/5+uCfNBo2oyGFt/1TrtcJjF73fM6PAljUAM+3yJ4cOShAloOof5Gjmnz4Eua352RRtWbIhQ0kgiZikHuHFDXmpP4zzc6oTic6kvnimnc2fZo3qK/ifuD2uY4Z+AVE3ulAFMtZXu7RpkokQ6jZn7ZgDW1qCBQUlRp5VwZv97NN2PbAH1qm/kwyY6riW/YYcDPzn7WDgG+KmSIXbxxEwEPrjSfJ3UN6GxD9/zMkjZfIhdGLBiRimaKHhv0JBOwvgwRvQVggoO0d/sSbGnFsnl1EqDOGSrvDOMspvsFMiy+la8Ta/hyAAkouoWPZZTV/rxN4Dp7hXNhpRiCmWO0OjW+pYm6Iid8CZhsjN4rEid14uYwj4sYgyD5zEdzb2BCRrc+jsp1iygWvlVB0Aoycq/wIaJZ2tih7lA5b9zKl1wjQnMtePjdLvjXzsMNLSZOGvHQJ5N3VAc381xClZo2KN6xBsB2vp6p3+BmyrEEi/Vs0CcG8sM38nLGl+jYRxATrvzWrsxDGXKJdPfcwkwbQdY8j8zIqNK43CDP+jGKu76hXtJzRN+K7RfI1nomxEqzUwT6I2KMMoX9mANm4GW5bLRxyk9fucyRQXbqgPdO7iDmVV6QBIWhcPW+aORVfU11JaHZJcvGjgjHyc4hLO+stydL/dHgCshNFWgCqqVOgFc7AAMMpCH5jyxKM/w/OLjZxu5re8/kECUTtvfsqB+A3XCBHJf7gbX8DXJujS5br5Yy7Y2eugFCMezYaPodXmJuclYIVQcV+32xtCXnRnhX0Rlqa1HiY8 ROAkB4Zk u3t623UuWat3pKPFCzsGeSKKp4ACv3Km6dVGfJBIW1j43Pxbz0v3mBQeTB6D69s9Xd9tHMuK1YtitmhzD8MYcyIpU5CwGzIUK7Q7xSr6GSFyLvOJd5GuHm+/GNdTPO4fUdL4ue3M5fkA2Sn9itYGLqR90KLeT3Kk35VjCjNh6W8xeM5B6XixqctriF50pP0DUrdoxoAYzaKAbFzu1Nk/vfK+64jgF+PJcq84NVE9DbZMmwxX3O6+ckNOVlT/OO0yAqiLWAu7Ll3WY1DA= 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 20, 2024 at 07:02:17PM +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. > > Signed-off-by: Kemeng Shi > --- > mm/backing-dev.c | 93 ++++++++++++++++++++++++++++++++++++------------ > 1 file changed, 70 insertions(+), 23 deletions(-) > > diff --git a/mm/backing-dev.c b/mm/backing-dev.c > index 5f2be8c8df11..788702b6c5dd 100644 > --- a/mm/backing-dev.c > +++ b/mm/backing-dev.c ... > @@ -46,31 +59,65 @@ static void bdi_debug_init(void) > bdi_debug_root = debugfs_create_dir("bdi", NULL); > } > ... > +#ifdef CONFIG_CGROUP_WRITEBACK > +static void bdi_collect_stats(struct backing_dev_info *bdi, > + struct wb_stats *stats) > +{ > + struct bdi_writeback *wb; > + > + /* protect wb from release */ > + mutex_lock(&bdi->cgwb_release_mutex); > + list_for_each_entry(wb, &bdi->wb_list, bdi_node) > + collect_wb_stats(stats, wb); > + mutex_unlock(&bdi->cgwb_release_mutex); > +} > +#else > +static void bdi_collect_stats(struct backing_dev_info *bdi, > + struct wb_stats *stats) > +{ > + collect_wb_stats(stats, &bdi->wb); > +} > +#endif > + I'm not familiar enough with the cgwb code to say for sure (and I'd probably wait for more high level feedback before worrying too much about this), but do we need the ifdef here just to iterate ->wb_list? >From looking at the code, it appears bdi->wb ends up on the list while the bdi is registered for both cases, so that distinction seems unnecessary. WRT to wb release protection, I wonder if this could use a combination of rcu_read_lock()/list_for_each_safe() and wb_tryget() on each wb before collecting its stats..? See how bdi_split_work_to_wbs() works, for example. Also I see a patch conflict/compile error on patch 2 due to __wb_calc_thresh() only taking one parameter in my tree. What's the baseline commit for this series? Brian > +static int bdi_debug_stats_show(struct seq_file *m, void *v) > +{ > + struct backing_dev_info *bdi = m->private; > + unsigned long background_thresh; > + unsigned long dirty_thresh; > + struct wb_stats stats; > + unsigned long tot_bw; > + > global_dirty_limits(&background_thresh, &dirty_thresh); > - wb_thresh = wb_calc_thresh(wb, dirty_thresh); > + > + memset(&stats, 0, sizeof(stats)); > + stats.dirty_thresh = dirty_thresh; > + bdi_collect_stats(bdi, &stats); > + > + tot_bw = atomic_long_read(&bdi->tot_write_bandwidth); > > seq_printf(m, > "BdiWriteback: %10lu kB\n" > @@ -87,18 +134,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); > > return 0; > -- > 2.30.0 > >