NeilBrown <neilb@xxxxxxx> wrote:
>
> + if (test_bit(MD_RECOVERY_REQUESTED, &pi->mddev->recovery))
> + j = pi->raid_disks;
> + else
> + j = 1;
> + while(j--) {
> + bio = r1_bio->bios[j];
> + for (i = 0; i < RESYNC_PAGES; i++) {
> + page = alloc_page(gfp_flags);
> + if (unlikely(!page))
> + goto out_free_pages;
> +
> + bio->bi_io_vec[i].bv_page = page;
> + }
> + }
> + /* If not user-requests, copy the page pointers to all bios */
> + if (!test_bit(MD_RECOVERY_REQUESTED, &pi->mddev->recovery)) {
> + for (i=0; i<RESYNC_PAGES ; i++)
> + for (j=1; j<pi->raid_disks; j++)
> + r1_bio->bios[j]->bi_io_vec[i].bv_page =
> + r1_bio->bios[0]->bi_io_vec[i].bv_page;
> }
>
> r1_bio->master_bio = NULL;
> @@ -122,8 +137,10 @@ static void * r1buf_pool_alloc(gfp_t gfp
> return r1_bio;
>
> out_free_pages:
> - for ( ; i > 0 ; i--)
> - __free_page(bio->bi_io_vec[i-1].bv_page);
> + for (i=0; i < RESYNC_PAGES ; i++)
> + for (j=0 ; j < pi->raid_disks; j++)
> + __free_page(r1_bio->bios[j]->bi_io_vec[i].bv_page);
> + j = -1;
> out_free_bio:
Are you sure the error handling here is correct?
a) we loop up to RESYNC_PAGES, but the allocation loop may not have got
that far
b) we loop in the ascending-index direction, but the allocating loop
loops in the descending-index direction.
c) we loop up to pi->raid_disks, but the allocating loop may have
done `j = 1;'.
d) there was a d), but I forgot what it was.
-
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
