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

[Waiman Long]

On 12/12/22 17:16, Tejun Heo wrote:
> On Mon, Dec 12, 2022 at 01:59:53PM +0100, 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].)
> >
> > > 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.
> >
> > > 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].)
> Yeah, I'm not sure about this patch either. Doesn't seem necessary.

I wrote this patch because I am not totally sure it is safe to flush 
offline blkgs. Since both you and Michal don't see any problem with it. 
I am fine taking it out.

Cheers,
Longman