Hi Malcolm,
Just curious if you ever got a chance to try the updated patch below on
your TYPE_TAPE setup..?
I'd like to get this merged in for v4.11-rc1, so if you have some time
in the next week please give it a shot, and let us know if you run into
any issues.
Thank you,
--nab
On Thu, 2016-11-03 at 23:06 -0700, Nicholas A. Bellinger wrote:
> Hi Malcolm,
>
> On Mon, 2016-10-31 at 18:04 +1100, Malcolm Haak wrote:
> > Hi Nicholas,
> >
> > I also patched rtslib (pull request on github) to work around not
> > being able to resolve the /dev/* device to the HCL and back the other
> > way.
> >
> > [root@tvtln01-eth SOURCES]# head /sys/kernel/config/target/core/pscsi_*/*/info
> > ==> /sys/kernel/config/target/core/pscsi_0/changer1/info <==
> > Status: ACTIVATED Max Queue Depth: 0 SectorSize: 0 HwMaxSectors: 0
> > SCSI Device Bus Location: Channel ID: 0 Target ID: 0 LUN: 0 Host ID: 10
> > Vendor: STK Model: L700 Rev: 0105
> >
> > ==> /sys/kernel/config/target/core/pscsi_1/tape1/info <==
> > Status: ACTIVATED Max Queue Depth: 0 SectorSize: 0 HwMaxSectors: 0
> > SCSI Device Bus Location: Channel ID: 0 Target ID: 1 LUN: 0 Host ID: 10
> > Vendor: IBM Model: ULT3580-TD5 Rev: 0105
> >
>
> <SNIP>
>
> >
> > ==> /sys/kernel/config/target/core/pscsi_1/tape1/attrib/hw_block_size <==
> > 0
> >
> > ==> /sys/kernel/config/target/core/pscsi_1/tape1/attrib/hw_max_sectors <==
> > 0
> >
> > ==> /sys/kernel/config/target/core/pscsi_1/tape1/attrib/hw_queue_depth <==
> > 32
>
> ...
>
> >
> > Here is my patch below. I went with <1 because I was too lazy to check
> > if the value was signed or not, so it can probably be changed to an =
> >
> > It's not a big patch at all but it works quite well!
> >
> > *** a/drivers/target/target_core_device.c 2015-01-29
> > 18:15:53.000000000 -0500
> > --- b/drivers/target/target_core_device.c.mod 2016-10-30
> > 03:21:41.955410797 -0400
> > ***************
> > *** 639,644 ****
> > --- 639,647 ----
> > * Limit max_sectors to a PAGE_SIZE aligned value for modern
> > * transport_allocate_data_tasks() operation.
> > */
> > +
> > + if (block_size < 1)
> > + return 0;
> > alignment = max(1ul, PAGE_SIZE / block_size);
> > aligned_max_sectors = rounddown(max_sectors, alignment);
> >
>
> Thanks for posting.
>
> Note PSCSI is (still) issuing MODE_SENSE to extract hw_sector_size at
> 'echo 1 > /sys/kernel/config/target/core/$PSCSI/$DEV/enable' time,
> via pscsi_tape_read_blocksize() code.
>
> pscsi_tape_read_blocksize() should be setting hw_sector_size = 1024 by
> default when '0' blocksize is returned by MODE_SENSE. Looking again,
> there is a bug wrt to MODE_SENSE failing + return without setting the
> hw_sector_size = 1024 default. Perhaps that is what you've been hitting
> with mvhtl..?
>
> Looking further, I think hw_max_sectors is also being set to '0' for
> TYPE_TAPE in pscsi_add_device_to_list(). This is likely where you ran
> into problems with hw_max_sectors = PAGE_SIZE.
>
> That said, here's a patch to address these two items. Let's see what
> MODE_SENSE is returning.
>
> At your earliest convenience, please verify.
>
> diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
> index 9125d93..33dc0fd 100644
> --- a/drivers/target/target_core_pscsi.c
> +++ b/drivers/target/target_core_pscsi.c
> @@ -154,7 +154,7 @@ static void pscsi_tape_read_blocksize(struct se_device *dev,
>
> buf = kzalloc(12, GFP_KERNEL);
> if (!buf)
> - return;
> + goto out_free;
>
> memset(cdb, 0, MAX_COMMAND_SIZE);
> cdb[0] = MODE_SENSE;
> @@ -162,16 +162,20 @@ static void pscsi_tape_read_blocksize(struct se_device *dev,
>
> ret = scsi_execute_req(sdev, cdb, DMA_FROM_DEVICE, buf, 12, NULL,
> HZ, 1, NULL);
> - if (ret)
> + if (ret) {
> + pr_warn("PSCSI TYPE_TAPE MODE_SENSE failed: %d\n", ret);
> goto out_free;
> + }
>
> /*
> * If MODE_SENSE still returns zero, set the default value to 1024.
> */
> sdev->sector_size = (buf[9] << 16) | (buf[10] << 8) | (buf[11]);
> + pr_info("PSCSI TYPE_TAPE detected hw sdev->sector_size: %d\n", sdev->sector_size);
> +out_free:
> if (!sdev->sector_size)
> sdev->sector_size = 1024;
> -out_free:
> +
> kfree(buf);
> }
>
> @@ -316,7 +320,7 @@ static int pscsi_add_device_to_list(struct se_device *dev,
>
> dev->dev_attrib.hw_block_size = sd->sector_size;
> dev->dev_attrib.hw_max_sectors =
> - min_t(int, sd->host->max_sectors, queue_max_hw_sectors(q));
> + min_not_zero(sd->host->max_sectors, queue_max_hw_sectors(q));
> dev->dev_attrib.hw_queue_depth = sd->queue_depth;
>
> /*
> @@ -339,8 +343,12 @@ static int pscsi_add_device_to_list(struct se_device *dev,
> /*
> * For TYPE_TAPE, attempt to determine blocksize with MODE_SENSE.
> */
> - if (sd->type == TYPE_TAPE)
> + if (sd->type == TYPE_TAPE) {
> pscsi_tape_read_blocksize(dev, sd);
> + dev->dev_attrib.hw_block_size = sd->sector_size;
> + pr_info("PSCSI: TYPE_TAPE setting hw_block_size: %u\n",
> + dev->dev_attrib.hw_block_size);
> + }
> return 0;
> }
>
> --
> To unsubscribe from this list: send the line "unsubscribe target-devel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
