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
