[Deepin-Kernel-SIG] [linux 6.6-y] [Upstream] block: improve struct request_queue layout && md: remove rcu protection to access rdev from conf

[opsiff]

Link: https://lore.kernel.org/all/[email protected]/#r

From: Yu Kuai [email protected]

Changes in v3:

• remove patch 1 from v2, and since all the print_conf() is called
  while 'reconfig_mutex' is held, it's safe to remove
  rcu_read_lock/unlock() directly.
• remove the definition of flag RemoveSynchronized;

Changes in v2:

• add cover leter in details.

The lifetime of rdev:

1. md_import_device() generate a rdev based on underlying disk;

   mddev_lock()
   rdev = kzalloc();
   rdev->bdev = blkdev_get_by_dev();
   mddev_unlock()

2. bind_rdev_to_array() add this rdev to mddev->disks;

   mddev_lock()
   kobject_add(&rdev->kobj, &mddev->kobj, ...);
   list_add_rcu(&rdev->same_set, &mddev->disks);
   mddev_unlock()

3. remove_and_add_spares() add this rdev to conf;

   mddev_lock()
   rdev_addable();
   pers->hot_add_disk();
   rcu_assign_pointer(conf->rdev, rdev);
   mddev_unlock()

4. Use this array with rdev;

5. remove_and_add_spares() remove rdev from conf;

   // triggered by sysfs/ioctl
   mddev_lock()
   rdev_removeable();
   pers->hot_remove_disk();
   rcu_assign_pointer(conf->rdev, NULL);
   synchronize_rcu();
   mddev_unlock()

   // triggered by daemon
   mddev_lock()
   rdev_removeable();
   synchronize_rcu(); -> this can't protect accessing rdev from conf
   pers->hot_remove_disk();
   rcu_assign_pointer(conf->rdev, NULL);
   mddev_unlock()

6. md_kick_rdev_from_array() remove rdev from mddev->disks;

   mddev_lock()
   list_del_rcu(&rdev->same_set);
   synchronize_rcu();
   list_add(&rdev->same_set, &mddev->deleting)
   mddev_unlock()
   export_rdev

There are two separate rcu protection for rdev, and this pathset remove
the protection of conf(step 3 and 5), because it's safe to access rdev
from conf in following cases:

• If 'reconfig_mutex' is held, because rdev can't be added or rmoved to
  conf;
• If there is normal IO inflight, because mddev_suspend() will wait for
  IO to be done and prevent rdev to be added or removed to conf;
• If sync thread is running, because remove_and_add_spares() can only be
  called from daemon thread when sync thread is done, and
  'MD_RECOVERY_RUNNING' is also checked for ioctl/sysfs;
• if any spinlock or rcu_read_lock() is held, because synchronize_rcu()
  from step 6 prevent rdev to be freed until spinlock is released or
  rcu_read_unlock();

Yu Kuai (5):
md: remove flag RemoveSynchronized
md/raid10: remove rcu protection to access rdev from conf
md/raid1: remove rcu protection to access rdev from conf
md/raid5: remove rcu protection to access rdev from conf
md/md-multipath: remove rcu protection to access rdev from conf

drivers/md/md-multipath.c | 32 +++---
drivers/md/md.c | 37 ++-----
drivers/md/md.h | 5 -
drivers/md/raid1.c | 71 ++++--------
drivers/md/raid10.c | 222 ++++++++++----------------------------
drivers/md/raid5-cache.c | 11 +-
drivers/md/raid5-ppl.c | 16 +--
drivers/md/raid5.c | 191 +++++++++++---------------------
drivers/md/raid5.h | 4 +-
9 files changed, 168 insertions(+), 421 deletions(-)

Summary by Sourcery

Simplify md RAID and multipath device handling by removing RCU-based protection around conf->rdev access and reworking device removal rules, while also tightening block queue and zoned block device interfaces.

Bug Fixes:

• Prevent unsafe rdev removal by tightening checks on pending I/O, fault, journal, and in_sync states instead of relying on RCU synchronization.

Enhancements:

• Refine removal criteria for md_rdev devices with a new helper and simplify spare removal logic in md core.
• Replace RCU-protected md device pointers in RAID1, RAID5, RAID10, and md-multipath with direct/WRITE_ONCE access patterns, adding appropriate lockdep assertions.
• Streamline RAID queue status and configuration printing paths to assume existing locks instead of per-call RCU sections.
• Reorganize struct request_queue fields to improve layout and clarify lifetime/ownership semantics in the block layer.
• Unify zoned block device APIs by centralizing bdev_nr_zones and blkdev_zone_mgmt definitions under CONFIG_BLK_DEV_ZONED guards.

[sourcery-ai]

Reviewer's Guide

Refactors md RAID personalities and block queue structures to remove RCU-based protection for accessing rdevs from in-memory configurations, relying instead on higher-level locking and inflight IO guarantees, while simplifying request_queue layout and zoned-block APIs.

Sequence diagram for rdev lifecycle and removal without RCU from conf

sequenceDiagram
    participant U as User_or_daemon
    participant mddev as mddev
    participant rdev as md_rdev
    participant conf as md_personality_conf

    U->>mddev: md_import_device()
    activate mddev
    mddev->>mddev: mddev_lock()
    mddev->>rdev: kzalloc(md_rdev)
    mddev->>rdev: rdev->bdev = blkdev_get_by_dev()
    mddev->>mddev: mddev_unlock()
    deactivate mddev

    U->>mddev: bind_rdev_to_array()
    activate mddev
    mddev->>mddev: mddev_lock()
    mddev->>mddev: kobject_add(rdev->kobj, mddev->kobj)
    mddev->>mddev: list_add_rcu(rdev->same_set, mddev->disks)
    mddev->>mddev: mddev_unlock()
    deactivate mddev

    U->>mddev: remove_and_add_spares()
    activate mddev
    mddev->>mddev: mddev_lock()
    loop for_each_rdev
        mddev->>mddev: rdev_removeable(rdev)
        alt removable
            mddev->>mddev: pers->hot_remove_disk(mddev, rdev)
            mddev->>conf: /* conf->rdev/replacement updated with WRITE_ONCE, no RCU */
            mddev->>mddev: sysfs_unlink_rdev(mddev, rdev)
            mddev->>rdev: rdev->saved_raid_disk = rdev->raid_disk
            mddev->>rdev: rdev->raid_disk = -1
        end
    end
    mddev->>mddev: mddev_unlock()
    deactivate mddev

    U->>mddev: md_kick_rdev_from_array()
    activate mddev
    mddev->>mddev: mddev_lock()
    mddev->>mddev: list_del_rcu(rdev->same_set)
    mddev->>mddev: synchronize_rcu()
    mddev->>mddev: list_add(rdev->same_set, mddev->deleting)
    mddev->>mddev: export_rdev(rdev)
    mddev->>mddev: mddev_unlock()
    deactivate mddev

Loading

File-Level Changes

Change	Details	Files
Remove RCU protection when accessing rdevs from RAID configurations and rely on explicit locking, inflight IO tracking, and READ/WRITE_ONCE for safety.	Replace rcu_dereference/rcu_read_lock usage on conf->mirrors/conf->disks/conf->multipaths with direct pointer loads or READ_ONCE where appropriate. Use WRITE_ONCE when updating rdev and replacement pointers to preserve ordering and visibility without RCU. Add lockdep_assert_held checks to status/print_conf paths that rely on mddev->lock or reconfig_mutex, documenting required locking context. Simplify replacement-selection logic in RAID1/RAID10/RAID5 paths by removing custom dereference helpers and RCU barriers.	drivers/md/raid10.c drivers/md/raid1.c drivers/md/raid5.c drivers/md/raid5-cache.c drivers/md/raid5-ppl.c drivers/md/md-multipath.c drivers/md/md.h drivers/md/raid5.h
Redesign rdev removal flow to avoid per-call synchronize_rcu and remove the RemoveSynchronized flag, using a new helper that checks rdev state and inflight IO before removal.	Introduce rdev_removeable() to centralize checks on raid_disk index, nr_pending, Blocked, Faulty, Journal, and In_sync flags when deciding if an rdev can be hot-removed. Rewrite remove_and_add_spares() to rely on rdev_removeable() and hot_remove_disk() directly, dropping the two-phase RemoveSynchronized plus synchronize_rcu scheme. Drop the RemoveSynchronized flag from md.h and remove all callsites that set or clear it. Update RAID1, RAID5, and multipath remove_disk implementations to clear rdev pointers with WRITE_ONCE and no longer perform synchronize_rcu loops.	drivers/md/md.c drivers/md/raid1.c drivers/md/raid5.c drivers/md/md-multipath.c drivers/md/md.h
Make RAID rdev pointer fields non-RCU and adjust helper APIs and structures accordingly.	Change disk_info.rdev and disk_info.replacement from __rcu pointers to plain struct md_rdev* and update all users accordingly. Remove rdev_pend_deref() and rdev_mdlock_deref() helpers that wrap rcu_dereference_protected, replacing their usage with direct pointer dereferences guarded by existing locks or state. Ensure reshape, recovery, and log-handling code treats rdev pointers as stable under mddev locking and inflight IO guarantees rather than RCU. Adjust RAID run/reshape/spare operations to use direct rdev/replacement access and to enforce constraints (e.g., no replacement during reshape) via simple pointer checks.	drivers/md/raid5.c drivers/md/raid5-cache.c drivers/md/raid5-ppl.c drivers/md/raid5.h
Reorganize struct request_queue and blk zoned-device APIs to improve layout and configuration-guarding of zoned helpers.	Reorder fields in struct request_queue to group related members (queuedata/elevator/mq_ops, queue flags and depth, refs, xarray hw queues, usage counter, last_merge, locks, disk, limits, pm_only, stats, rq_qos, id, dma_pad_mask, nr_requests, crypto, timeout, work, shared tags, icq/blkcg, node, requeue structures, flush queues, sysfs locks, mq_freeze_lock, tag_set) and move quiesce_depth closer to mq_freeze_lock. Move blkdev zoned APIs so blkdev_report_zones, blkdev_zone_mgmt, and blk_revalidate_disk_zones are always declared, with bdev_nr_zones only available when CONFIG_BLK_DEV_ZONED, and provide a stub bdev_nr_zones() returning 0 otherwise. Export bdev_nr_zones() declaration under CONFIG_BLK_DEV_ZONED and add a stub implementation in the !CONFIG_BLK_DEV_ZONED section to keep callers simple.	include/linux/blkdev.h

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from opsiff. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[sourcery-ai]

Hey - I've left some high level feedback:

• Now that rdev/replacement pointers are no longer RCU-protected, consider consistently using READ_ONCE/WRITE_ONCE for all unsynchronized pointer loads/stores (not just status paths) to make the concurrency assumptions explicit and avoid accidental compiler reordering.
• The new helper is named rdev_removeable(); for clarity and consistency with existing terminology, consider renaming it to rdev_removable() (and updating call sites) to avoid the spelling mismatch.
• You’ve added lockdep_assert_held() in some paths that rely on mddev->lock or reconfig_mutex instead of RCU; it may be worth auditing other code that now directly dereferences conf->*rdev fields and adding similar assertions where lock-based protection is assumed, to document and verify those invariants.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Now that rdev/replacement pointers are no longer RCU-protected, consider consistently using READ_ONCE/WRITE_ONCE for all unsynchronized pointer loads/stores (not just status paths) to make the concurrency assumptions explicit and avoid accidental compiler reordering.
- The new helper is named rdev_removeable(); for clarity and consistency with existing terminology, consider renaming it to rdev_removable() (and updating call sites) to avoid the spelling mismatch.
- You’ve added lockdep_assert_held() in some paths that rely on mddev->lock or reconfig_mutex instead of RCU; it may be worth auditing other code that now directly dereferences conf->*rdev fields and adding similar assertions where lock-based protection is assumed, to document and verify those invariants.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[Copilot]

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR removes RCU-based protection around conf->rdev access across multiple md personalities (raid1/10/5 and multipath), replaces it with direct / READ_ONCE / WRITE_ONCE accesses plus lockdep assertions, and also reorganizes struct request_queue layout and zoned block prototypes.

Changes:

• Remove RemoveSynchronized flag and simplify md core spare remove logic via a new rdev_removeable() helper.
• Drop rcu_read_lock()/rcu_dereference() usage for conf->{rdev,replacement} across raid1/10/5/multipath; add lockdep_assert_held() in config/status printing.
• Rework block layer header layout: reorganize struct request_queue fields and adjust zoned API declarations/guards.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
include/linux/blkdev.h	Reorders struct request_queue fields; adjusts zoned API declarations/guards.
drivers/md/md.h	Removes RemoveSynchronized flag bit.
drivers/md/md.c	Adds rdev_removeable() helper; simplifies remove_and_add_spares() logic.
drivers/md/md-multipath.c	Removes RCU around conf->multipaths[].rdev accesses; switches to READ_ONCE/WRITE_ONCE and lockdep assertions.
drivers/md/raid1.c	Removes RCU around conf->mirrors[].rdev accesses; adds lockdep assertions; uses WRITE_ONCE in updates.
drivers/md/raid10.c	Removes RCU and ordering barriers around mirrors[].{rdev,replacement} access and transitions; adds lockdep assertions.
drivers/md/raid5.h	Converts disk_info.{rdev,replacement} from __rcu pointers to plain pointers.
drivers/md/raid5.c	Removes RCU dereferences/locks across many paths; uses READ_ONCE/WRITE_ONCE in some updates; removes helper deref wrappers and ordering barriers.
drivers/md/raid5-cache.c	Removes RCU sections around disk pointer access during recovery and cache checks.
drivers/md/raid5-ppl.c	Removes RCU dereference usage in PPL recovery/init paths.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.