On Mar 11, 2024 / 10:22, Bart Van Assche wrote:
> On 3/10/24 23:54, Shin'ichiro Kawasaki wrote:
> > As the preparation for the dev_size_mb parameter changes through sysfs,
> > factor out the initialization of parameters affected by the dev_size_mb
> > changes.
> >
> > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx>
> > ---
> > drivers/scsi/scsi_debug.c | 52 ++++++++++++++++++++++++---------------
> > 1 file changed, 32 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> > index d03d66f11493..c6b32f07a82c 100644
> > --- a/drivers/scsi/scsi_debug.c
> > +++ b/drivers/scsi/scsi_debug.c
> > @@ -7234,10 +7234,39 @@ ATTRIBUTE_GROUPS(sdebug_drv);
> > static struct device *pseudo_primary;
> > +/*
> > + * Calculate size related parameters from sdebug_dev_zize_mb and
> > + * sdebug_sector_size.
> > + */
> > +static void scsi_debug_init_size_parameters(void)
> > +{
> > + unsigned long sz;
> > +
> > + sz = (unsigned long)sdebug_dev_size_mb * 1048576;
> > + sdebug_store_sectors = sz / sdebug_sector_size;
> > + sdebug_capacity = get_sdebug_capacity();
> > +
> > + /* play around with geometry, don't waste too much on track 0 */
> > + sdebug_heads = 8;
> > + sdebug_sectors_per = 32;
> > + if (sdebug_dev_size_mb >= 256)
> > + sdebug_heads = 64;
> > + else if (sdebug_dev_size_mb >= 16)
> > + sdebug_heads = 32;
> > + sdebug_cylinders_per = (unsigned long)sdebug_capacity /
> > + (sdebug_sectors_per * sdebug_heads);
> > + if (sdebug_cylinders_per >= 1024) {
> > + /* other LLDs do this; implies >= 1GB ram disk ... */
> > + sdebug_heads = 255;
> > + sdebug_sectors_per = 63;
> > + sdebug_cylinders_per = (unsigned long)sdebug_capacity /
> > + (sdebug_sectors_per * sdebug_heads);
> > + }
> > +}
> > +
> > static int __init scsi_debug_init(void)
> > {
> > bool want_store = (sdebug_fake_rw == 0);
> > - unsigned long sz;
> > int k, ret, hosts_to_add;
> > int idx = -1;
> > @@ -7369,26 +7398,9 @@ static int __init scsi_debug_init(void)
> > sdebug_dev_size_mb = DEF_DEV_SIZE_MB;
> > if (sdebug_dev_size_mb < 1)
> > sdebug_dev_size_mb = 1; /* force minimum 1 MB ramdisk */
> > - sz = (unsigned long)sdebug_dev_size_mb * 1048576;
> > - sdebug_store_sectors = sz / sdebug_sector_size;
> > - sdebug_capacity = get_sdebug_capacity();
> > - /* play around with geometry, don't waste too much on track 0 */
> > - sdebug_heads = 8;
> > - sdebug_sectors_per = 32;
> > - if (sdebug_dev_size_mb >= 256)
> > - sdebug_heads = 64;
> > - else if (sdebug_dev_size_mb >= 16)
> > - sdebug_heads = 32;
> > - sdebug_cylinders_per = (unsigned long)sdebug_capacity /
> > - (sdebug_sectors_per * sdebug_heads);
> > - if (sdebug_cylinders_per >= 1024) {
> > - /* other LLDs do this; implies >= 1GB ram disk ... */
> > - sdebug_heads = 255;
> > - sdebug_sectors_per = 63;
> > - sdebug_cylinders_per = (unsigned long)sdebug_capacity /
> > - (sdebug_sectors_per * sdebug_heads);
> > - }
> > + scsi_debug_init_size_parameters();
> > +
> > if (scsi_debug_lbp()) {
> > sdebug_unmap_max_blocks =
> > clamp(sdebug_unmap_max_blocks, 0U, 0xffffffffU);
>
> Please remove sdebug_heads, sdebug_cylinders_per and sdebug_sectors_per
> instead of making this change. While these values are reported in a
> MODE SENSE response, I don't think that it is valuable to keep support
> for heads, cylinders and sectors in the scsi_debug driver.
I see. I guess we can return just zero as sdebug_sectors_per in the MODE
SENSE response instead.
I noticed that the three variables you suggest to remove are used in
sdebug_build_parts() also. It is not a good idea to remove the function
and drop or modify the partition table generation feature, probably. I
think we can make the three variables non-global, local variables in the
function. What do you think?
>
> Thanks,
>
> Bart.
