Re: [RFC] implementing tape statistics single file vs multi-file in sysfs
On 8.2.2015, at 4.45, Greg KH gre...@linuxfoundation.org wrote: On Sat, Feb 07, 2015 at 09:27:05PM -0500, Laurence Oberman wrote: Hello Its not going to be tens of thousands of devices. That count was an aggregate based on 1000's of servers. In reality its unlikely to ever be more than 100 tapes drives per individual Linux kernel instance. Therefore sysfs will be the valid way to do this and make the data available to user space. Even if it is only 2 tape drives, again, what's wrong with using the existing i/o statistic interfaces that all block devices have? Don't go making special one-off interfaces for one type of device if at all possible. The tape driver does not have access to the block device i/o statistics because the tapes are not block devices but character devices. They use the Linux block subsystem enough to do i/o but this does not include access to the statistics counters. The purpose of the suggested text vector format patch is to create statistics that have the same format as the existing i/o statistics that the block devices so that the existing tools can be used with minimal modifications. One alternative is, of course, to tie the st driver more into the Linux block device system. I have looked into this several times but have never seen how to do this in a simple enough way. Thanks, Kai -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] implementing tape statistics single file vs multi-file in sysfs
On Sun, 2015-02-08 at 10:45 +0800, Greg KH wrote: On Sat, Feb 07, 2015 at 09:27:05PM -0500, Laurence Oberman wrote: Hello Its not going to be tens of thousands of devices. That count was an aggregate based on 1000's of servers. In reality its unlikely to ever be more than 100 tapes drives per individual Linux kernel instance. Therefore sysfs will be the valid way to do this and make the data available to user space. Even if it is only 2 tape drives, again, what's wrong with using the existing i/o statistic interfaces that all block devices have? Tape is a character device. It only uses block via SCSI (SCSI uses block to give an issue queue for every device). One of the problems with this model is that the block kobj, where all the statistics hang, is actually never exposed for these devices because they don't have a block name. Even granted that we could alter block to give names to the nameless queues and expose them in /sys/block, we'd still have the problem, the queue statistics are the property of the pluggable I/O scheduler, so there's a disconnect between the SCSI upper layer drivers and the block scheduler (since the latter is embedded by design). Pulling that apart would get us into a fairly nasty layering violation (drivers aren't supposed to care about the scheulders). Don't go making special one-off interfaces for one type of device if at all possible. I don't really see any way around this. The statistics the block schedulers collect are relevant to I/O load balancing; that's not at all the same class of statistics as the users of tape are interested in. This problem is equivalent to the fibrechannel one where we collect the fc_host_statistics in the scsi_transport_fc.c class as an attribute group (block doesn't want to see or know any of the information because it's all relevant to the transport, not the block abstraction). James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] st: implement sysfs based tape statistics v2
One feature of tape statistics is that they're as much about the *tape* as they are about the *drive*, which is uncommon for block devices. It might be useful to have a set of counters which is cleared every time a new tape is inserted into the drive. In particular, bad reads since this tape was inserted would be very useful for monitoring the quality of tapes. Dale -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issues with commit 34b48db6 (block: remove artifical max_hw_sectors cap)
On Mon, 19 Jan 2015, Alan Stern wrote: ... If anyone (still?) cares about this bug, commit 3a9794d3 (sd: Fix max transfer length for 4k disks) fixes it, with no patches required. -Kenny -- Kenneth R. Crudup Sr. SW Engineer, Scott County Consulting, Silicon Valley -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xen-scsiback: some modifications about code comment
On 02/07/2015 04:31 AM, Rudy Zhang wrote:
From: Tao Chen boby.c...@huawei.com
Signed-off-by: Tao Chen boby.c...@huawei.com
Are some white space fixes in comments really worth a patch?
Juergen
---
drivers/xen/xen-scsiback.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index 3e32146..59f09fd 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -83,7 +83,7 @@ struct ids_tuple {
struct v2p_entry {
struct ids_tuple v; /* translate from */
- struct scsiback_tpg *tpg; /* translate to */
+ struct scsiback_tpg *tpg; /* translate to */
unsigned int lun;
struct kref kref;
struct list_head l;
@@ -525,7 +525,7 @@ static int scsiback_gnttab_data_map(struct vscsiif_request
*ring_req,
}
}
- /* free of (sgl) in fast_flush_area()*/
+ /* free of (sgl) in fast_flush_area() */
pending_req-sgl = kmalloc_array(nr_segments,
sizeof(struct scatterlist), GFP_KERNEL);
if (!pending_req-sgl)
@@ -1084,7 +1084,7 @@ static void scsiback_do_1lun_hotplug(struct vscsibk_info
*info, int op,
}
}
break;
- /*When it is necessary, processing is added here.*/
+ /* When it is necessary, processing is added here. */
default:
break;
}
@@ -1475,8 +1475,8 @@ static u32 scsiback_tpg_get_inst_index(struct
se_portal_group *se_tpg)
static int scsiback_check_stop_free(struct se_cmd *se_cmd)
{
/*
-* Do not release struct se_cmd's containing a valid TMR
-* pointer. These will be released directly in scsiback_device_action()
+* Do not release struct se_cmd's containing a valid TMR pointer.
+* These will be released directly in scsiback_device_action()
* with transport_generic_free_cmd().
*/
if (se_cmd-se_cmd_flags SCF_SCSI_TMR_CDB)
@@ -1642,7 +1642,7 @@ static int scsiback_make_nexus(struct scsiback_tpg *tpg,
return -ENOMEM;
}
/*
-* Initialize the struct se_session pointer
+* Initialize the struct se_session pointer
*/
tv_nexus-tvn_se_sess = transport_init_session(TARGET_PROT_NORMAL);
if (IS_ERR(tv_nexus-tvn_se_sess)) {
@@ -1759,7 +1759,7 @@ static ssize_t scsiback_tpg_store_nexus(struct
se_portal_group *se_tpg,
unsigned char i_port[VSCSI_NAMELEN], *ptr, *port_ptr;
int ret;
/*
-* Shutdown the active I_T nexus if 'NULL' is passed..
+* Shutdown the active I_T nexus if 'NULL' is passed.
*/
if (!strncmp(page, NULL, 4)) {
ret = scsiback_drop_nexus(tpg);
@@ -1930,7 +1930,7 @@ static void scsiback_drop_tpg(struct se_portal_group
*se_tpg)
*/
scsiback_drop_nexus(tpg);
/*
-* Deregister the se_tpg from TCM..
+* Deregister the se_tpg from TCM.
*/
core_tpg_deregister(se_tpg);
kfree(tpg);
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] st implement tape statistics v3
Hi All,
Please find attached v3 of the patch. It implements the changes requested by
Greg. The statistics files aren't in a separate directory any more they're
implemented directly as device attributes unless someone has objections to them
being in a place like /sys/class/scsi_tape/*/.
There's also an ABI for file them in the patch for the testing directory. The
kernel version is ?? because I'm not sure if there will be more changes
based on feedback - let me know if I should drop that from the patch than
submit a separate patch requesting the file be created once I know the kernel
version it makes it into. Is the ABI file named correctly as
sysfs-class-scsi_tape?
I've dropped the sync file stuff as well - As much as I'd like consistent
statistics Greg is right it won't hurt if they're not quite in sync. The patch
including the additional ABI documentation is about two thirds of the original
size.
Thanks
Shane
--
Signed-off-by: shane.seym...@hp.com
Tested-by: shane.seym...@hp.com
---
--- a/drivers/scsi/st.c 2015-01-11 14:46:00.243814755 -0600
+++ b/drivers/scsi/st.c 2015-02-08 16:50:51.368552780 -0600
@@ -476,10 +476,22 @@ static void st_scsi_execute_end(struct r
struct st_request *SRpnt = req-end_io_data;
struct scsi_tape *STp = SRpnt-stp;
struct bio *tmp;
+ u64 ticks;
STp-buffer-cmdstat.midlevel_result = SRpnt-result = req-errors;
STp-buffer-cmdstat.residual = req-resid_len;
+ if (STp-stats != NULL) {
+ ticks = get_jiffies_64();
+ STp-stats-in_flight--;
+ ticks -= STp-stats-stamp;
+ STp-stats-io_ticks += ticks;
+ if (req-cmd[0] == WRITE_6)
+ STp-stats-write_ticks += ticks;
+ else if (req-cmd[0] == READ_6)
+ STp-stats-read_ticks += ticks;
+ }
+
tmp = SRpnt-bio;
if (SRpnt-waiting)
complete(SRpnt-waiting);
@@ -496,6 +508,7 @@ static int st_scsi_execute(struct st_req
struct rq_map_data *mdata = SRpnt-stp-buffer-map_data;
int err = 0;
int write = (data_direction == DMA_TO_DEVICE);
+ struct scsi_tape *STp = SRpnt-stp;
req = blk_get_request(SRpnt-stp-device-request_queue, write,
GFP_KERNEL);
@@ -516,6 +529,20 @@ static int st_scsi_execute(struct st_req
}
}
+ if (STp-stats != NULL) {
+ if (cmd[0] == WRITE_6) {
+ STp-stats-write_cnt++;
+ STp-stats-write_byte_cnt += bufflen;
+ } else if (cmd[0] == READ_6) {
+ STp-stats-read_cnt++;
+ STp-stats-read_byte_cnt += bufflen;
+ } else {
+ STp-stats-other_cnt++;
+ }
+ STp-stats-stamp = get_jiffies_64();
+ STp-stats-in_flight++;
+ }
+
SRpnt-bio = req-bio;
req-cmd_len = COMMAND_SIZE(cmd[0]);
memset(req-cmd, 0, BLK_MAX_CDB);
@@ -4064,6 +4091,7 @@ out:
static int create_cdevs(struct scsi_tape *tape)
{
int mode, error;
+
for (mode = 0; mode ST_NBR_MODES; ++mode) {
error = create_one_cdev(tape, mode, 0);
if (error)
@@ -4222,6 +4250,8 @@ static int st_probe(struct device *dev)
}
tpnt-index = error;
sprintf(disk-disk_name, st%d, tpnt-index);
+/* Allocate stats structure */
+ tpnt-stats = kzalloc(sizeof(struct scsi_tape_stats), GFP_ATOMIC);
dev_set_drvdata(dev, tpnt);
@@ -4248,6 +4278,8 @@ out_put_queue:
blk_put_queue(disk-queue);
out_put_disk:
put_disk(disk);
+ if (tpnt-stats != NULL)
+ kfree(tpnt-stats);
kfree(tpnt);
out_buffer_free:
kfree(buffer);
@@ -4298,6 +4330,10 @@ static void scsi_tape_release(struct kre
disk-private_data = NULL;
put_disk(disk);
+ if (tpnt-stats != NULL) {
+ kfree(tpnt-stats);
+ tpnt-stats = NULL;
+ }
kfree(tpnt);
return;
}
@@ -4513,12 +4549,238 @@ options_show(struct device *dev, struct
}
static DEVICE_ATTR_RO(options);
+/* Support for tape stats */
+
+/**
+ * read_cnt_how - return read count - count of reads made from tape drive
+ * @dev: struct device
+ * @attr: attribute structure
+ * @buf: buffer to return formatted data in
+ */
+static ssize_t read_cnt_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct st_modedef *STm = dev_get_drvdata(dev);
+ struct scsi_tape *STp = STm-tape;
+ struct scsi_tape_stats *STt;
+
+ if ((STp == 0) || (STp-stats == 0))
+ return -EINVAL;
+ STt = STp-stats;
+ if (STt == 0)
+ return -EINVAL;
+ return snprintf(buf, PAGE_SIZE, %llu, STt-read_cnt);
+}
+static DEVICE_ATTR_RO(read_cnt);
+
+/**
+ * read_byte_cnt_show - return read byte count - tape
RE: [PATCH] st: implement sysfs based tape statistics v2
Kai - see last 3 paragraphs for question about if something is a bug or not. BTW I had a look - I couldn't quickly find out if there was a way to tell if the medium has changed in a tape drive (there could be something device specific). At the device level there's a record of I/O errors: [root@tapedrive device]# pwd /sys/class/scsi_tape/st0/device [root@tapedrive device]# cat ioerr_cnt 0x5 That doesn't distinguish between read or write or any other kind of error though - it doesn't even really have to mean an error since reading with a block size too small also increments the error count: [root@tapedrive tape-stats]# ./tape_exercise write /dev/st0 /dev/urandom 100 About to write from /dev/urandom to /dev/st0, max size = 100, blksize = 4096 Write complete on /dev/st0 after 1003520 bytes ./tape_[root@tapedrive tape-stats]# ./tape_exercise read /dev/st0 About to read from /dev/st0 blksize = 256 Failed to read from /dev/st0 using current blksize, will try using a bigger blksize About to read from /dev/st0 blksize = 512 Failed to read from /dev/st0 using current blksize, will try using a bigger blksize About to read from /dev/st0 blksize = 1024 Failed to read from /dev/st0 using current blksize, will try using a bigger blksize About to read from /dev/st0 blksize = 2048 Failed to read from /dev/st0 using current blksize, will try using a bigger blksize About to read from /dev/st0 blksize = 4096 Reading complete for /dev/st0, 987136 bytes read [root@tapedrive tape-stats]# cd /sys/class/scsi_tape/st0/device [root@tapedrive device]# cat ioerr_cnt 0xa It would seem that ioerr_cnt is probably a count of SCSI check conditions encountered? Unfortunately for the MTIOCGET ioctl the value of mt_resid is the partition number of the tape not what I would have expected it to be - the residual left after the last read or write that returned an error (and 0 if the last read/write didn't return an error). Kai - is that a bug? Shouldn't mt_resid be the residual from the last failed read or write indicating how much data didn't make it to the device and 0 on a successful read or write? I've used mt_resid from MTIOCGET on HP-UX previously when issuing reads and repositioning and retrying tape reads when starting with too low a block size to try and automatically determine the block size in use (it's never a good idea to under or overread tape blocks because it always results in check conditions and in the st driver under reading the block size always creates messages in dmesg). If you don't have time to look at it I may look at if it's possible to provide the correct mt_resid for MTIOCGET - assuming that a long time if misuse prevents us from correcting it. If there's no way to export a partition number for the devices that support it I can add a new sysfs file (call it partition) to export it that way and see if I can get the correct value into mt_resid. -Original Message- From: Seymour, Shane M Sent: Monday, February 09, 2015 10:19 AM To: 'Dale R. Worley' Cc: linux-scsi@vger.kernel.org Subject: RE: [PATCH] st: implement sysfs based tape statistics v2 Both of those things would have to be futures and require discussion - the very original version cleared stats on a device open but I got asked to keep it the stats cumulative so they would be more similar to disk stats. I'm not sure about the bad reads idea I'd have to look and see if some other layer provided device error information. I've got some changes to make and don't want to change it into a moving target that needs more discussion at this point. -Original Message- From: Dale R. Worley [mailto:wor...@alum.mit.edu] Sent: Monday, February 09, 2015 4:08 AM To: Seymour, Shane M Cc: linux-scsi@vger.kernel.org Subject: Re: [PATCH] st: implement sysfs based tape statistics v2 One feature of tape statistics is that they're as much about the *tape* as they are about the *drive*, which is uncommon for block devices. It might be useful to have a set of counters which is cleared every time a new tape is inserted into the drive. In particular, bad reads since this tape was inserted would be very useful for monitoring the quality of tapes. Dale -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] st: implement sysfs based tape statistics v2
Both of those things would have to be futures and require discussion - the very original version cleared stats on a device open but I got asked to keep it the stats cumulative so they would be more similar to disk stats. I'm not sure about the bad reads idea I'd have to look and see if some other layer provided device error information. I've got some changes to make and don't want to change it into a moving target that needs more discussion at this point. -Original Message- From: Dale R. Worley [mailto:wor...@alum.mit.edu] Sent: Monday, February 09, 2015 4:08 AM To: Seymour, Shane M Cc: linux-scsi@vger.kernel.org Subject: Re: [PATCH] st: implement sysfs based tape statistics v2 One feature of tape statistics is that they're as much about the *tape* as they are about the *drive*, which is uncommon for block devices. It might be useful to have a set of counters which is cleared every time a new tape is inserted into the drive. In particular, bad reads since this tape was inserted would be very useful for monitoring the quality of tapes. Dale -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
