linux-kernel - Re: [PATCH-block v2 2/3] blk-cgroup: Don't flush a blkg if destroyed

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Mon, 12 Dec 2022 09:58:27 -0500
From:   Waiman Long <longman@...hat.com>
To:     Michal Koutný <mkoutny@...e.com>
Cc:     Jens Axboe <axboe@...nel.dk>, Tejun Heo <tj@...nel.org>,
        Josef Bacik <josef@...icpanda.com>,
        Zefan Li <lizefan.x@...edance.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        cgroups@...r.kernel.org, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        "Dennis Zhou (Facebook)" <dennisszhou@...il.com>
Subject: Re: [PATCH-block v2 2/3] blk-cgroup: Don't flush a blkg if destroyed


On 12/12/22 07:59, Michal Koutný wrote:
> Hello.
>
> On Sun, Dec 11, 2022 at 05:20:57PM -0500, Waiman Long <longman@...hat.com> wrote:
>> Before commit 3b8cc6298724 ("blk-cgroup: Optimize blkcg_rstat_flush()"),
>> blkg's stats is only flushed if they are online.
> I'm not sure I follow -- css_release_work_fn/cgroup_rstat_flush may be
> called on an offlined blkcg (offlined!=released). There's no invariant
> ensuring offlined blkcg won't be flushed. (There is only current
> situation when there is no reader of io data that'd need them flushed
> [1].)
The original cgroup_rstat_flush() iterates the list of blkg's in the 
blkg_list. A blkg will be removed from the list when it is offlined. 
This patch just reverts its behavior to that previous behavior.
>
>> In addition, the stat flushing of blkgs in blkcg_rstat_flush()
>> includes propagating the rstat data to its parent. However, if a blkg
>> has been destroyed (offline), the validity of its parent may be
>> questionable.
> Parents won't be freed (neither offlined) before children (see
> css_killed_work_fn). It should be regularly OK to pass data into a
> parent of an offlined blkcg.
I guess it is likely to be safe to flush an offline blkg. I am just 
being conservative in case there is any corner case where it may be a 
problem which I haven't foreseen.
>
>> For safety, revert back to the old behavior by ignoring offline
>> blkg's.
> I don't know if this is a good reasoning. If you argue that offlined
> children needn't be taken into parent's account, then I think it's more
> efficient to exclude the offlined blkcgs from update. (With the caveat I
> have in [1].)

It is possible that a blkg may be updated before it becomes offline, but 
the flush isn't done in time before that happens. The next patch will 
catch some of that.

Cheers,
Longman