All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: wqu@suse.com
Cc: linux-btrfs@vger.kernel.org
Subject: [bug report] btrfs: introduce dedicated helper to scrub simple-mirror based range
Date: Wed, 26 Jan 2022 17:23:28 +0300	[thread overview]
Message-ID: <20220126142328.GA7357@kili> (raw)

Hello Qu Wenruo,

The patch b5b99b1e0296: "btrfs: introduce dedicated helper to scrub
simple-mirror based range" from Jan 7, 2022, leads to the following
Smatch static checker warning:

fs/btrfs/scrub.c:3678 scrub_stripe() error: uninitialized symbol 'offset'.
fs/btrfs/scrub.c:3680 scrub_stripe() error: uninitialized symbol 'physical_end'.

fs/btrfs/scrub.c
    3513 static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
    3514                                            struct btrfs_block_group *bg,
    3515                                            struct map_lookup *map,
    3516                                            struct btrfs_device *scrub_dev,
    3517                                            int stripe_index, u64 dev_extent_len)
    3518 {
    3519         struct btrfs_path *path;
    3520         struct btrfs_fs_info *fs_info = sctx->fs_info;
    3521         struct btrfs_root *root;
    3522         struct btrfs_root *csum_root;
    3523         struct blk_plug plug;
    3524         const u64 profile = map->type & BTRFS_BLOCK_GROUP_PROFILE_MASK;
    3525         const u64 chunk_logical = bg->start;
    3526         int ret;
    3527         u64 nstripes;
    3528         u64 physical;
    3529         u64 logical;
    3530         u64 logic_end;
    3531         u64 physical_end;
    3532         u64 increment;        /* The logical increment after finishing one stripe */
    3533         u64 offset;        /* Offset inside the chunk */
    3534         u64 stripe_logical;
    3535         u64 stripe_end;
    3536         int stop_loop = 0;
    3537 
    3538         path = btrfs_alloc_path();
    3539         if (!path)
    3540                 return -ENOMEM;
    3541 
    3542         /*
    3543          * Work on commit root. The related disk blocks are static as
    3544          * long as COW is applied. This means, it is save to rewrite
    3545          * them to repair disk errors without any race conditions
    3546          */
    3547         path->search_commit_root = 1;
    3548         path->skip_locking = 1;
    3549         path->reada = READA_FORWARD;
    3550 
    3551         wait_event(sctx->list_wait,
    3552                    atomic_read(&sctx->bios_in_flight) == 0);
    3553         scrub_blocked_if_needed(fs_info);
    3554 
    3555         root = btrfs_extent_root(fs_info, bg->start);
    3556         csum_root = btrfs_csum_root(fs_info, bg->start);
    3557 
    3558         /*
    3559          * Collect all data csums for the stripe to avoid seeking during
    3560          * the scrub. This might currently (crc32) end up to be about 1MB
    3561          */
    3562         blk_start_plug(&plug);
    3563 
    3564         if (sctx->is_dev_replace &&
    3565             btrfs_dev_is_sequential(sctx->wr_tgtdev,
    3566                                     map->stripes[stripe_index].physical)) {
    3567                 mutex_lock(&sctx->wr_lock);
    3568                 sctx->write_pointer = map->stripes[stripe_index].physical;
    3569                 mutex_unlock(&sctx->wr_lock);
    3570                 sctx->flush_all_writes = true;
    3571         }
    3572 
    3573         /*
    3574          * There used to be a big double loop to handle all profiles using the
    3575          * same routine, which grows larger and more gross over time.
    3576          *
    3577          * So here we handle each profile differently, so simpler profiles
    3578          * have simpler scrubing function.
    3579          */
    3580         if (!(profile & (BTRFS_BLOCK_GROUP_RAID0 | BTRFS_BLOCK_GROUP_RAID10 |
    3581                          BTRFS_BLOCK_GROUP_RAID56_MASK))) {
    3582                 /*
    3583                  * Above check rules out all complex profile, the remaining
    3584                  * profiles are SINGLE|DUP|RAID1|RAID1C*, which is simple
    3585                  * mirrored duplication without stripe.
    3586                  *
    3587                  * Only @phsyical and @mirror_num needs to calculated using
    3588                  * @stripe_index.
    3589                  */
    3590                 ret = scrub_simple_mirror(sctx, root, csum_root, bg, map,
    3591                                 bg->start, bg->length, scrub_dev,
    3592                                 map->stripes[stripe_index].physical,
    3593                                 stripe_index + 1);
    3594                 goto out;
                         ^^^^^^^^^

    3595         }
    3596         if (profile & (BTRFS_BLOCK_GROUP_RAID0 | BTRFS_BLOCK_GROUP_RAID10)) {
    3597                 ret = scrub_simple_stripe(sctx, root, csum_root, bg, map,
    3598                                           scrub_dev, stripe_index);
    3599                 goto out;
                         ^^^^^^^^^
At this point "offset" and "physical_end" are not intialized.

    3600         }
    3601 
    3602         /* Only RAID56 goes through the old code */
    3603         ASSERT(map->type & BTRFS_BLOCK_GROUP_RAID56_MASK);
    3604 
    3605         physical = map->stripes[stripe_index].physical;
    3606         offset = 0;
    3607         nstripes = div64_u64(dev_extent_len, map->stripe_len);
    3608         get_raid56_logic_offset(physical, stripe_index, map, &offset, NULL);
    3609         increment = map->stripe_len * nr_data_stripes(map);
    3610 
    3611         logical = chunk_logical + offset;
    3612         physical_end = physical + nstripes * map->stripe_len;
    3613         get_raid56_logic_offset(physical_end, stripe_index, map, &logic_end,
    3614                                 NULL);
    3615         logic_end += chunk_logical;
    3616 
    3617         ret = 0;
    3618         /*
    3619          * Due to the rotation, for RAID56 it's better to iterate each stripe
    3620          * using their physical offset.
    3621          */
    3622         while (physical < physical_end) {
    3623                 ret = get_raid56_logic_offset(physical, stripe_index, map,
    3624                                               &logical, &stripe_logical);
    3625                 logical += chunk_logical;
    3626                 if (ret) {
    3627                         /* It is parity strip */
    3628                         stripe_logical += chunk_logical;
    3629                         stripe_end = stripe_logical + increment;
    3630                         ret = scrub_raid56_parity(sctx, map, scrub_dev,
    3631                                                   stripe_logical,
    3632                                                   stripe_end);
    3633                         if (ret)
    3634                                 goto out;
    3635                         goto next;
    3636                 }
    3637 
    3638                 /*
    3639                  * Now we're at data stripes, scrub each extents in the range.
    3640                  *
    3641                  * At this stage, if we ignore the repair part, each data stripe
    3642                  * is no different than SINGLE profile.
    3643                  * We can reuse scrub_simple_mirror() here, as the repair part
    3644                  * is still based on @mirror_num.
    3645                  */
    3646                 ret = scrub_simple_mirror(sctx, root, csum_root, bg, map,
    3647                                           logical, map->stripe_len,
    3648                                           scrub_dev, physical, 1);
    3649                 if (ret < 0)
    3650                         goto out;
    3651 next:
    3652                 logical += increment;
    3653                 physical += map->stripe_len;
    3654                 spin_lock(&sctx->stat_lock);
    3655                 if (stop_loop)
    3656                         sctx->stat.last_physical = map->stripes[stripe_index].physical +
    3657                                                    dev_extent_len;
    3658                 else
    3659                         sctx->stat.last_physical = physical;
    3660                 spin_unlock(&sctx->stat_lock);
    3661                 if (stop_loop)
    3662                         break;
    3663         }
    3664 out:
    3665         /* push queued extents */
    3666         scrub_submit(sctx);
    3667         mutex_lock(&sctx->wr_lock);
    3668         scrub_wr_submit(sctx);
    3669         mutex_unlock(&sctx->wr_lock);
    3670 
    3671         blk_finish_plug(&plug);
    3672         btrfs_free_path(path);
    3673 
    3674         if (sctx->is_dev_replace && ret >= 0) {
    3675                 int ret2;
    3676 
    3677                 ret2 = sync_write_pointer_for_zoned(sctx,
--> 3678                                 chunk_logical + offset,
                                                         ^^^^^^
    3679                                 map->stripes[stripe_index].physical,
    3680                                 physical_end);
                                         ^^^^^^^^^^^^
    3681                 if (ret2)
    3682                         ret = ret2;
    3683         }
    3684 
    3685         return ret < 0 ? ret : 0;
    3686 }

regards,
dan carpenter

             reply	other threads:[~2022-01-26 14:23 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-26 14:23 Dan Carpenter [this message]
  -- strict thread matches above, loose matches on Subject: below --
2022-01-27  8:52 [bug report] btrfs: introduce dedicated helper to scrub simple-mirror based range Dan Carpenter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220126142328.GA7357@kili \
    --to=dan.carpenter@oracle.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=wqu@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.