Re: [PATCH 1/2] md/raid5: Remove unnecessary bio_put() in raid5_read_one_chunk()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dear Logan, dear David,


Thank you for the patch.

Am 02.09.22 um 19:16 schrieb Logan Gunthorpe:
From: David Sloan <david.sloan@xxxxxxxxxxxxx>

When running chunk-sized reads on disks with badblocks duplicate bio
free/puts are observed:

    =============================================================================
    BUG bio-200 (Not tainted): Object already free
    -----------------------------------------------------------------------------
    Allocated in mempool_alloc_slab+0x17/0x20 age=3 cpu=2 pid=7504
     __slab_alloc.constprop.0+0x5a/0xb0

[…]

The double free is caused by an unnecessary bio_put() in the
if(is_badblock(...)) error path in raid5_read_one_chunk().

The error path was moved ahead of bio_alloc_clone() in c82aa1b76787c
("md/raid5: move checking badblock before clone bio in
raid5_read_one_chunk"). The previous code checked and freed align_bio
which required a bio_put. After he move that is no longer needed as

*t*he move

raid_bio is returned to the control of the common io path which
performs its own endio resulting in a double free on bad device blocks.

Fixes: c82aa1b76787c ("md/raid5: move checking badblock before clone bio in raid5_read_one_chunk")

This commit was added to Linux in v5.14-rc1, so it’s not a recent commit – or I misunderstood the cover letter.


Kind regards,

Paul


Signed-off-by: David Sloan <david.sloan@xxxxxxxxxxxxx>
[logang@xxxxxxxxxxxx: minor rewording of commit message]
Signed-off-by: Logan Gunthorpe <logang@xxxxxxxxxxxx>
---
  drivers/md/raid5.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 4e6d865a6456..734f92e75f85 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -5538,7 +5538,6 @@ static int raid5_read_one_chunk(struct mddev *mddev, struct bio *raid_bio)
if (is_badblock(rdev, sector, bio_sectors(raid_bio), &first_bad,
  			&bad_sectors)) {
-		bio_put(raid_bio);
  		rdev_dec_pending(rdev, mddev);
  		return 0;
  	}



[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux