Hello,
A couple minor nitpicks:
Can you please add a short comment here too saying that @ioc->rqos.disk
isn't initialized yet when this function is called from the init path?
> +static int ioc_autop_idx(struct ioc *ioc, struct gendisk *disk)
> {
> int idx = ioc->autop_idx;
> const struct ioc_params *p = &autop[idx];
> @@ -808,11 +808,11 @@ static int ioc_autop_idx(struct ioc *ioc)
> u64 now_ns;
>
> /* rotational? */
> - if (!blk_queue_nonrot(ioc->rqos.disk->queue))
> + if (!blk_queue_nonrot(disk->queue))
> return AUTOP_HDD;
>
> /* handle SATA SSDs w/ broken NCQ */
> - if (blk_queue_depth(ioc->rqos.disk->queue) == 1)
> + if (blk_queue_depth(disk->queue) == 1)
> return AUTOP_SSD_QD1;
>
> /* use one of the normal ssd sets */
> @@ -901,14 +901,19 @@ static void ioc_refresh_lcoefs(struct ioc *ioc)
> &c[LCOEF_WPAGE], &c[LCOEF_WSEQIO], &c[LCOEF_WRANDIO]);
> }
>
> -static bool ioc_refresh_params(struct ioc *ioc, bool force)
> +/*
> + * struct gendisk is required as an argument because ioc->rqos.disk
> + * might not be properly initialized
> + */
Here too, let's explicitly say when it's not initialized.
> +static bool _ioc_refresh_params(struct ioc *ioc, bool force,
> + struct gendisk *disk)
Given that __ are about an order of magnitude more common in the kernel,
would you mind renaming it to __ioc_refresh_params() or e.g.
ioc_refresh_params_disk()?
Please feel free to add
Acked-by: Tejun Heo <tj@xxxxxxxxxx>
Thanks.
--
tejun
