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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 12/12/22 07:59, Michal Koutný wrote:
Hello.

On Sun, Dec 11, 2022 at 05:20:57PM -0500, Waiman Long <longman@xxxxxxxxxx> 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





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux