Hi Song
Is the patch ok? If so, are there chances to merge this into md-next this week?
Regards
Xiao
On Fri, Feb 3, 2023 at 1:16 PM Xiao Ni <xni@xxxxxxxxxx> wrote:
>
> It has added io_acct_set for raid0/raid5 io accounting and it needs to
> alloc md_io_acct in the i/o path. They are free when the bios come back
> from member disks. Now we don't have a method to monitor if those bios
> are all come back. In the takeover process, it needs to free the raid0
> memory resource including the memory pool for md_io_acct. But maybe some
> bios are still not returned. When those bios are returned, it can cause
> panic bcause of introducing NULL pointer or invalid address.
>
> [ 6973.767999] RIP: 0010:mempool_free+0x52/0x80
> [ 6973.786098] Call Trace:
> [ 6973.786549] md_end_io_acct+0x31/0x40
> [ 6973.787227] blk_update_request+0x224/0x380
> [ 6973.787994] blk_mq_end_request+0x1a/0x130
> [ 6973.788739] blk_complete_reqs+0x35/0x50
> [ 6973.789456] __do_softirq+0xd7/0x2c8
> [ 6973.790114] ? sort_range+0x20/0x20
> [ 6973.790763] run_ksoftirqd+0x2a/0x40
> [ 6973.791400] smpboot_thread_fn+0xb5/0x150
> [ 6973.792114] kthread+0x10b/0x130
> [ 6973.792724] ? set_kthread_struct+0x50/0x50
> [ 6973.793491] ret_from_fork+0x1f/0x40
>
> This patch trys to use ->active_io to count acct_io. The regression
> tests have passed. And I did takeover from raid0 to raid5 and from
> raid5 to raid0 while I/O is happening. This patch works well.
>
> Reported-by: Fine Fan <ffan@xxxxxxxxxx>
> Signed-off-by: Xiao Ni <xni@xxxxxxxxxx>
> ---
> drivers/md/md.c | 6 ++++++
> drivers/md/md.h | 7 ++++---
> 2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index da6370835c47..7f06eb953488 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -8627,12 +8627,15 @@ static void md_end_io_acct(struct bio *bio)
> {
> struct md_io_acct *md_io_acct = bio->bi_private;
> struct bio *orig_bio = md_io_acct->orig_bio;
> + struct mddev *mddev = md_io_acct->mddev;
>
> orig_bio->bi_status = bio->bi_status;
>
> bio_end_io_acct(orig_bio, md_io_acct->start_time);
> bio_put(bio);
> bio_endio(orig_bio);
> +
> + percpu_ref_put(&mddev->active_io);
> }
>
> /*
> @@ -8648,10 +8651,13 @@ void md_account_bio(struct mddev *mddev, struct bio **bio)
> if (!blk_queue_io_stat(bdev->bd_disk->queue))
> return;
>
> + percpu_ref_get(&mddev->active_io);
> +
> clone = bio_alloc_clone(bdev, *bio, GFP_NOIO, &mddev->io_acct_set);
> md_io_acct = container_of(clone, struct md_io_acct, bio_clone);
> md_io_acct->orig_bio = *bio;
> md_io_acct->start_time = bio_start_io_acct(*bio);
> + md_io_acct->mddev = mddev;
>
> clone->bi_end_io = md_end_io_acct;
> clone->bi_private = md_io_acct;
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index 6335cb86e52e..e148e3c83b0d 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -710,9 +710,10 @@ struct md_thread {
> };
>
> struct md_io_acct {
> - struct bio *orig_bio;
> - unsigned long start_time;
> - struct bio bio_clone;
> + struct mddev *mddev;
> + struct bio *orig_bio;
> + unsigned long start_time;
> + struct bio bio_clone;
> };
>
> #define THREAD_WAKEUP 0
> --
> 2.32.0 (Apple Git-132)
>
