md_make_request: don't touch the bio after calling make_request
PATCH 1/1 md_make_request: don't touch the bio after calling make_request
md_make_request was calling bio_sectors() for part_stat_add after it was calling the make_request function. This is bad because the make_request function can free the bio and because the bi_size field can change around.
The fix here was suggested by Jens Axboe. It saves the sector count before the make_request call. I hit this with CONFIG_DEBUG_PAGEALLOC turned on while trying to break his pretty fusionio card.
--- drivers/md/md.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
drivers/md/md.c +7 -2
@@ -287,6 +287,7 @@ static int md_make_request(struct request_queue *q, struct bio *bio)mddev_t *mddev = q->queuedata;int rv;int cpu; +unsigned int sectors;if (mddev ==NULL || mddev->pers ==NULL || !mddev->ready) {
@@ -311,12 +312,16 @@ static int md_make_request(struct request_queue *q, struct bio *bio)atomic_inc (&mddev->active_io);rcu_read_unlock (); +/* +* save the sectors now since our bio can +* go away inside make_request +*/ + sectors =bio_sectors (bio); rv = mddev->pers->make_request (mddev, bio); cpu =part_stat_lock ();part_stat_inc (cpu, &mddev->gendisk->part0, ios[rw]); -part_stat_add (cpu, &mddev->gendisk->part0, sectors[rw], -bio_sectors (bio)); +part_stat_add (cpu, &mddev->gendisk->part0, sectors[rw], sectors);part_stat_unlock ();if (atomic_dec_and_test (&mddev->active_io) && mddev->suspended)
Signature · 1 line
1.7.1.1
Quoted 9 lines · On Mon, 07 Feb 2011 14:21:48 -0500 Chris Mason <[email protected]> wrote:
md_make_requestwas callingbio_sectors()forpart_stat_addafter it was calling themake_requestfunction. This is bad because themake_requestfunction can free the bio and because thebi_sizefield can change around.The fix here was suggested by Jens Axboe. It saves the sector count before the
make_requestcall. I hit this withCONFIG_DEBUG_PAGEALLOCturned on while trying to break his pretty fusionio card.
Thanks Chris!!
I've added it to my queue of md fixes (which I really must send of soon) and tagged it for -stable.
NeilBrown
Quoted 37 lines
Signed-off-by Chris Mason <[email protected]>--- drivers/md/md.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)drivers/md/md.c +7
-2@@ -287,6 +287,7 @@ static int md_make_request(struct request_queue *q, struct bio *bio)mddev_t *mddev = q->queuedata;int rv;int cpu; +unsigned int sectors;if (mddev ==NULL || mddev->pers ==NULL || !mddev->ready) {@@ -311,12 +312,16 @@ static int md_make_request(struct request_queue *q, struct bio *bio)atomic_inc (&mddev->active_io);rcu_read_unlock (); +/* +* save the sectors now since our bio can +* go away inside make_request +*/ + sectors =bio_sectors (bio); rv = mddev->pers->make_request (mddev, bio); cpu =part_stat_lock ();part_stat_inc (cpu, &mddev->gendisk->part0, ios[rw]); -part_stat_add (cpu, &mddev->gendisk->part0, sectors[rw], -bio_sectors (bio)); +part_stat_add (cpu, &mddev->gendisk->part0, sectors[rw], sectors);part_stat_unlock ();if (atomic_dec_and_test (&mddev->active_io) && mddev->suspended)