linux-kernel - Re: [PATCH v2] blk-cgroup: don't clear stat in blkcg_reset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <migrlemuqjqff6y64o6ukfkuil6uwuarwvyg3xymfphnicznna@sy5dwhovytuz>
Date: Wed, 21 Aug 2024 17:51:39 +0200
From: Michal Koutný <mkoutny@...e.com>
To: Li Lingfeng <lilingfeng@...weicloud.com>
Cc: tj@...nel.org, josef@...icpanda.com, hch@....de, axboe@...nel.dk, 
	longman@...hat.com, ming.lei@...hat.com, cgroups@...r.kernel.org, 
	linux-block@...r.kernel.org, linux-kernel@...r.kernel.org, yukuai1@...weicloud.com, 
	houtao1@...wei.com, yi.zhang@...wei.com, yangerkun@...wei.com, yukuai3@...wei.com, 
	lilingfeng3@...wei.com
Subject: Re: [PATCH v2] blk-cgroup: don't clear stat in blkcg_reset_stats()

Hello.

On Wed, Aug 21, 2024 at 10:07:56AM GMT, Li Lingfeng <lilingfeng@...weicloud.com> wrote:
> The list of root cgroup can be used by both cgroup v1 and v2 while
> non-root cgroup can't since it must be removed before switch between
> cgroup v1 and v2.
> So it may has effect if the list of root used by cgroup v2 was corrupted
> after switching to cgroup v1, and switch back to cgroup v2 to use the
> corrupted list again.

> However, the root cgroup will not use the list any more after commit
> ef45fe470e1e("blk-cgroup: show global disk stats in root cgroup io.stat").

How come? Before that patch the root file was excluded with
CFTYPE_NOT_ON_ROOT. IOW how has that patch an effect on llist traversal?

(It doesn't matter, your patch doesn't restore memset anyway.)

This is the reasoning how I understand it:

| The removed function clears blkg.iostat structures that is only used on
| v2 whereas the function can only be called with v1 hierarchy attached.
| Zeroing effect could potentially be visible root blkcg "shared" between
| v1 and v2 but v2 actually synthesizes stats differently for root.

> Although this has no negative effect, it is not necessary. Remove the
> related code for cleanup. No function change.

I'm impressed by the amount of analysis you did to potentially remove
the unused function. If you feel like cleaning up more or sectioning,
see also [1] or [2] for inspiration.

Thanks,
Michal

[1] https://lore.kernel.org/all/20240625005906.106920-14-roman.gushchin@linux.dev/
[2] https://lore.kernel.org/all/20240628210317.272856-1-roman.gushchin@linux.dev/

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)