Re: [PATCH v3] blk-iocost: Pass gendisk to ioc_refresh_params
From: Tejun Heo <tj@kernel.org>
Date: 2023-02-27 18:27:27
Also in:
cgroups, lkml
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?
quoted hunk ↗ jump to hunk
+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 [off-list ref] Thanks. -- tejun