On Sat, Feb 18 2017, colyli@xxxxxxx wrote:
> @@ -1447,36 +1497,26 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
>
> static void raid1_make_request(struct mddev *mddev, struct bio *bio)
> {
> - struct r1conf *conf = mddev->private;
> - struct r1bio *r1_bio;
> + void (*make_request_fn)(struct mddev *mddev, struct bio *bio);
> + struct bio *split;
> + sector_t sectors;
>
> - /*
> - * make_request() can abort the operation when read-ahead is being
> - * used and no empty request is available.
> - *
> - */
> - r1_bio = mempool_alloc(conf->r1bio_pool, GFP_NOIO);
> + make_request_fn = (bio_data_dir(bio) == READ) ?
> + raid1_read_request : raid1_write_request;
>
....
>
> - if (bio_data_dir(bio) == READ)
> - raid1_read_request(mddev, bio, r1_bio);
> - else
> - raid1_write_request(mddev, bio, r1_bio);
> + make_request_fn(mddev, split);
> + } while (split != bio);
> }
I don't think the use of make_request_fn() makes the code more readable
or more efficient, and it quite possibly has a cost.
If you left it as
if (bio_data_dir(bio) == READ)
raid1_read_request(mddev, bio, r1_bio);
else
raid1_write_request(mddev, bio, r1_bio);
then gcc would notice that both raid1_read_request and
raid1_write_request are static functions that are only used once, and
will normally inline them. This will reduce the total stack depth,
which you expressed some concern about in a previous email.
Using a function pointer like this makes it harder for gcc to perform
that optimization.
NeilBrown
Attachment:
signature.asc
Description: PGP signature
