Hello, On Wed, Sep 21, 2022 at 08:04:49PM +0200, Christoph Hellwig wrote: > The combinations of an error check with an ERR_PTR return and a lookup > with a NULL return leads to ugly handling of the return values in the > callers. Just open coding the check and the lookup is much simpler. > > Signed-off-by: Christoph Hellwig <hch@xxxxxx> Acked-by: Tejun Heo <tj@xxxxxxxxxx> but please look below. > /** > * blkcg_conf_open_bdev - parse and open bdev for per-blkg config update > * @inputp: input string pointer > @@ -697,14 +678,16 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol, > rcu_read_lock(); > spin_lock_irq(&q->queue_lock); > > - blkg = blkg_lookup_check(blkcg, pol, q); > - if (IS_ERR(blkg)) { > - ret = PTR_ERR(blkg); > + if (!blkcg_policy_enabled(q, pol)) { > + ret = -EOPNOTSUPP; > goto fail_unlock; > } > > - if (blkg) > + blkg = blkg_lookup(blkcg, q); > + if (blkg) { > + blkg_update_hint(blkcg, blkg); > goto success; > + } > > /* > * Create blkgs walking down from blkcg_root to @blkcg, so that all > @@ -740,14 +723,15 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol, > rcu_read_lock(); > spin_lock_irq(&q->queue_lock); > > - blkg = blkg_lookup_check(pos, pol, q); > - if (IS_ERR(blkg)) { > - ret = PTR_ERR(blkg); > + if (!blkcg_policy_enabled(q, pol)) { > blkg_free(new_blkg); > + ret = -EOPNOTSUPP; > goto fail_preloaded; > } > > + blkg = blkg_lookup(pos, q); > if (blkg) { > + blkg_update_hint(pos, blkg); > blkg_free(new_blkg); I don't think conf_prep needs to update the hint in the first place, so we can just do blkg_lookup()'s and drop the blkg_update_hint() calls. Thanks. -- tejun