Re: [PATCH 12/15] ssc: add variable-length block read support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 25 Nov 2008 15:27:35 +1100
"Mark Harvey" <markh794@xxxxxxxxx> wrote:

> On Sun, Nov 23, 2008 at 4:26 AM, FUJITA Tomonori
> <fujita.tomonori@xxxxxxxxxxxxx> wrote:
> > On Thu, 13 Nov 2008 10:52:43 +1100
> > "Mark Harvey" <markh794@xxxxxxxxx> wrote:
> >
> > Seems that the attached patch fixes the 512-byte read case, but not
> > 256-byte. I guess that I still misunderstand something...
> >
> >
> >> >> Now test we can read in a 256k block from the tape.
> >> >> # dd if=/dev/st0 of=/tmp/block0 bs=256k count=1
> >> >> 1+0 records in
> >> >> 1+0 records out
> >> >> 262144 bytes (262 kB) copied, 0.00424289 s, 61.8 MB/s
> >> >>
> >> >> Now test if we can read in a partial block (this fails returning no
> >> >> data - we should have read in the 128k)
> >> >> # dd if=/dev/st0 of=/tmp/block0 bs=128k count=1
> >> >> dd: reading `/dev/st0': Input/output error
> >> >> 0+0 records in
> >> >> 0+0 records out
> >> >> 0 bytes (0 B) copied, 0.00249643 s, 0.0 kB/s
> >> >
> >> > Sorry for my dumb question here:
> >> >
> >> > What should stgt return? The first 128 KB of the first block and no
> >> > sense?
> >>
> >> The amount of data requested should be returned. In this example, 128k.
> >>
> >> The sense returned should be :NO SENSE | VALID | ILI (Illegal Length
> >> Indicator) and the overrun values (size of data block - size of data
> >> requested) filled in sense_buffer[3] -> sense_buffer[7].
> >
> > With the patch, stgt returns CHECK CONDITION with a sense (NO SENSE |
> > VALID | ILI ) and the information field sets to 128K.
> >
> > I got (actually, stgt returns 128KB data though...):
> >
> > nemo:/home/fujita# dd if=/dev/st0 of=/tmp/block0 bs=128k count=1
> > 0+0 records in
> > 0+0 records out
> > 0 bytes (0 B) copied, 0.0030381 seconds, 0.0 kB/s
> >
> > But the last line should be something like:
> >
> > 131072 bytes (131 kB) copied...
> >
> > Right?
> 
> Just re-tested on real hardware:
> # [2:0:4:2]    tape    QUANTUM  SuperDLT1        2323  /dev/st1  /dev/sg8
> # sg_inq -v /dev/sg8
>     inquiry cdb: 12 00 00 00 24 00
> standard INQUIRY:
>     inquiry cdb: 12 00 00 00 3a 00
>   PQual=0  Device_type=1  RMB=1  version=0x02  [SCSI-2]
>   [AERC=0]  [TrmTsk=0]  NormACA=0  HiSUP=1  Resp_data_format=2
>   SCCS=0  ACC=0  TGPS=0  3PC=0  Protect=0  BQue=0
>   EncServ=0  MultiP=0  [MChngr=0]  [ACKREQQ=0]  Addr16=0
>   [RelAdr=0]  WBus16=0  Sync=0  Linked=0  [TranDis=0]  CmdQue=0
>   Clocking=0x0  QAS=0  IUS=0
>     length=58 (0x3a)   Peripheral device type: tape
>  Vendor identification: QUANTUM
>  Product identification: SuperDLT1
>  Product revision level: 2323
>     inquiry cdb: 12 01 00 00 fc 00
>     inquiry: requested 252 bytes but  got 9 bytes
>     inquiry cdb: 12 01 80 00 fc 00
>     inquiry: requested 252 bytes but  got 16 bytes
>  Unit serial number: PKB53H0142

That's really helpful, thanks!


> An attempt to read 128k from a 256k block results with an error message:
> # dd if=/dev/st1 bs=128k count=1 of=/tmp/block0
> dd: reading `/dev/st1': Cannot allocate memory
> 0+0 records in
> 0+0 records out
> 0 bytes (0 B) copied, 0.01287 seconds, 0.0 kB/s
> 
> And /var/log/messages:
> kernel: st1: Failed to read 262144 byte block with 131072 byte transfer.

I think that this is a kernel message that st driver gives when the
INFORMATION field has a negative value. In this case, the INFORMATION
field had -128K.

I've attached a new patch. The difference from the previous one is
just one line, it sets a negative value to the INFORMATION field when
the requested transfer length is smaller than the actual logical block
length.


> Disclaimer...
> I'm 99.99% positive, last time I tried this I received a different
> message and did in fact read 128k of data. This may be an
> implementation of dd used..

With the same tape drive?


> # rpm -qf `which dd`
> coreutils-5.93-22.14
> # uname -a
> Linux dpg-markh 2.6.16.46-0.12-smp #1 SMP Thu May 17 14:00:09 UTC 2007
> i686 i686 i386 GNU/Linux
> 
> >
> >
> >> >> Now test if we can read in a more then a block (this fails returning
> >> >> no data - we should have read in the 256k)
> >> >> # dd if=/dev/st0 of=/tmp/block0 bs=512k count=1
> >> >> dd: reading `/dev/st0': Input/output error
> >> >> 0+0 records in
> >> >> 0+0 records out
> >> >> 0 bytes (0 B) copied, 0.00705821 s, 0.0 kB/s
> >> >
> >> > stgt should return the whole first block without sense, right?
> >> >
> >>
> >> The amount of data successfully read should be returned. In this example, 128k.
> >>
> >> The sense returned should be :NO SENSE | VALID | ILI (Illegal Length
> >> Indicator) and the amount of data NOT successfully returned -> filled
> >> in sense_buffer[3] -> sense_buffer[7]
> >
> > With the patch, stgt returns CHECK CONDITION with a sense (NO SENSE |
> > VALID | ILI ) and the information field sets to 256K.
> >
> > nemo:/home/fujita# dd if=/dev/st0 of=/tmp/block0 bs=512k count=1
> > 0+1 records in
> > 0+1 records out
> > 262144 bytes (262 kB) copied, 0.019265 seconds, 13.6 MB/s
> >
> > This is the right response?
> This is the correct response...
> 
> 
> Many thanks for having a look at this.

No problem at all. Thanks!


diff --git a/usr/bs_ssc.c b/usr/bs_ssc.c
index 0b9fedb..9ed9e65 100644
--- a/usr/bs_ssc.c
+++ b/usr/bs_ssc.c
@@ -317,15 +317,27 @@ static int resp_var_read(struct scsi_cmd *cmd, uint8_t *buf, uint32_t length,
 	*transferred = 0;
 
 	if (length != h->blk_sz) {
+		uint8_t info[4];
+		int val = length - h->blk_sz;
+
+		put_unaligned_be32(val, info);
+
 		if (h->blk_type == BLK_EOD)
 			sense_data_build(cmd, 0x40 | BLANK_CHECK,
 					 NO_ADDITIONAL_SENSE);
 		else
-			sense_data_build(cmd, NO_SENSE, NO_ADDITIONAL_SENSE);
+			ssc_sense_data_build(cmd, NO_SENSE | 0x20,
+					     NO_ADDITIONAL_SENSE,
+					     info, sizeof(info));
+
+		if (length > h->blk_sz)
+			scsi_set_in_resid_by_actual(cmd, length - h->blk_sz);
+		else
+			scsi_set_in_resid_by_actual(cmd, 0);
 
 		length = min(length, h->blk_sz);
+
 		result = SAM_STAT_CHECK_CONDITION;
-		scsi_set_in_resid_by_actual(cmd, length);
 
 		if (!length)
 			goto out;
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux SCSI]     [Linux RAID]     [Linux Clusters]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]

  Powered by Linux