Re: [PATCH 2/9] blkcg, writeback: Add wbc->no_wbc_acct

On Mon 24-06-19 05:58:56, Tejun Heo wrote:

Hello, Jan.

On Mon, Jun 24, 2019 at 10:21:30AM +0200, Jan Kara wrote:

quoted

OK, now I understand. Just one more question: So effectively, you are using
wbc->no_wbc_acct to pass information from btrfs code to btrfs code telling
it whether IO should or should not be accounted with wbc_account_io().

Yes.

quoted

Wouldn't it make more sense to just pass this information internally
within btrfs? Granted, if this mechanism gets more widespread use by other
filesystems, then probably using wbc flag makes more sense. But I'm not
sure if this isn't a premature generalization...

The btrfs async issuers end up using generic writeback path and uses
the generic wbc owner mechanisms so that ios are attached to the right
cgroup too.  So, I kinda prefer to provide a generic mechanism from
wbc side.

OK, I can live with that. We just have to be kind of careful so that people
just don't sprinkle no_wbc_acct writeback around because they don't know
better. Maybe you could at least add comment to no_wbc_acct mentioning that
this is for the cases where writeback has already been accounted for?

That said, the names are a bit misleading and I think it'd
be better to rename them to something more explicit, e.g. sth along
the line of wbc_update_cgroup_owner() and wbc->no_cgroup_owner.  What
do you think?

Yeah, renaming would probably make things clearer as well.

								Honza
-- 
Jan Kara [off-list ref]
SUSE Labs, CR