Re: [PATCH] zfcp: fix sense_buffer access bug
On Mon, Jan 28, 2008 at 06:29:12PM +0900, FUJITA Tomonori wrote: > > ACK for fixing the access to the sense buffer. > > > > We are working internally on cleaning up the zfcp messages. With this > > change, the 'trace' and 'hex dump' messages will disappear. So, could > > you simply remove the ZFCP_HEX_DUMP message above, instead of fixing > > it? > > I can but James has already merged the above patch to scsi-misc. So it > would be more convenient for everyone if you could rebase your > patchset on top of scsi-misc? Ok, if the pach is already merged, i am fine with that. We will have to rebase the patches we are working on anyway. Thanks for providing the fix. Christof - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] zfcp: fix sense_buffer access bug
On Sun, Jan 27, 2008 at 12:41:50PM +0900, FUJITA Tomonori wrote:
> The commit de25deb18016f66dcdede165d07654559bb332bc changed
> scsi_cmnd.sense_buffer from a static array to a dynamically allocated
> buffer. We can't access to sense_buffer in '&cmd->sense_buffer' way.
>
> Signed-off-by: FUJITA Tomonori <[EMAIL PROTECTED]>
> ---
> drivers/s390/scsi/zfcp_fsf.c |4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
> index fe57941..a9a147d 100644
> --- a/drivers/s390/scsi/zfcp_fsf.c
> +++ b/drivers/s390/scsi/zfcp_fsf.c
> @@ -4224,10 +4224,10 @@ zfcp_fsf_send_fcp_command_task_handler(struct
> zfcp_fsf_req *fsf_req)
>
> ZFCP_LOG_TRACE("%i bytes sense data provided by FCP\n",
> fcp_rsp_iu->fcp_sns_len);
> - memcpy(&scpnt->sense_buffer,
> + memcpy(scpnt->sense_buffer,
> zfcp_get_fcp_sns_info_ptr(fcp_rsp_iu), sns_len);
> ZFCP_HEX_DUMP(ZFCP_LOG_LEVEL_TRACE,
> - (void *) &scpnt->sense_buffer, sns_len);
> + (void *)scpnt->sense_buffer, sns_len);
> }
>
> /* check for overrun */
ACK for fixing the access to the sense buffer.
We are working internally on cleaning up the zfcp messages. With this
change, the 'trace' and 'hex dump' messages will disappear. So, could
you simply remove the ZFCP_HEX_DUMP message above, instead of fixing
it?
Christof
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Common header file for FC definitions
On Tue, Jan 08, 2008 at 10:46:51AM -0500, James Smart wrote:
> Christoph Hellwig wrote:
> >On Tue, Jan 08, 2008 at 09:09:36AM +0100, Christof Schmitt wrote:
> >>zfcp has a couple of definitions to describe the FC protocol. Grepping
> >>through the complete source tree shows that e.g. the lpfc module makes
> >>similar private definitions. It think that it would make sense to
> >>introduce a global header file for FC related definitions that each FC
> >>driver can use.
> >>
> >>The attached patch contains some definitons that i found in zfcp and
> >>could be reused by other drivers.
> >>
> >>What do others think? Would this be useful?
> >
> >This is a very good idea. It might make more sense to have the more or
> >less complete defintions for the fc protocol in the header. Robert &
> >folks have been working on a software fc stack part as the fcoe header
> >so they might have one to contribute.
>
> I Agree - A common header for standard FC definitions would be a good thing.
>
> That said, we have to be a little lenient on when drivers convert over to
> the standard headers as there's likely code change required.
For zfcp, i would like to move the FC specific definitions out of the
zfcp_def.h header file. My idea would be to introduce a simple header
file like proposed and to adapt zfcp to use these. I don't intend to
write header files for the full FC protocol, since zfcp only uses a
small part. When other drivers use more definitions, then the
definitions can be added to the existing file.
When other drivers want to use the global definitions, they can
include the global file, but they don't have to.
Would others also be interested in having structs in the global header
file for FC commands like this?
struct zfcp_ls_adisc {
u8 code;
u8 field[3];
u32 hard_nport_id;
u64 wwpn;
u64 wwnn;
u32 nport_id;
} __attribute__ ((packed));
This could be renamed to fc_ls_adisc and also be part of the global
header file. But probably it makes more sense to start simple now and
think about the common structs later.
Thanks for the comments.
Christof
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] Common header file for FC definitions
zfcp has a couple of definitions to describe the FC protocol. Grepping through the complete source tree shows that e.g. the lpfc module makes similar private definitions. It think that it would make sense to introduce a global header file for FC related definitions that each FC driver can use. The attached patch contains some definitons that i found in zfcp and could be reused by other drivers. What do others think? Would this be useful? Christof --- include/scsi/fc.h | 47 +++ 1 file changed, 47 insertions(+) --- /dev/null 1970-01-01 00:00:00.0 + +++ b/include/scsi/fc.h 2008-01-08 08:56:32.0 +0100 @@ -0,0 +1,47 @@ +/* + * Public definitions that are used by Fibre Channel (FC) + * device drivers. + */ + +/* FC-PH/FC-GS well-known address identifiers for generic services */ +#define FC_DID_KEY_DISTRIBUTION_SERVICE0xF7 +#define FC_DID_ALIAS_SERVICE 0xF8 +#define FC_DID_MANAGEMENT_SERVICE 0xFA +#define FC_DID_TIME_SERVICE0xFB +#define FC_DID_DIRECTORY_SERVICE 0xFC + +#define FC_DID_MASK 0x00FF + +/* task attribute values in FCP-2 FCP_CMND IU */ +#define SIMPLE_Q 0 +#define HEAD_OF_Q 1 +#define ORDERED_Q 2 +#define ACA_Q 4 +#define UNTAGGED 5 + +/* task management flags in FCP-2 FCP_CMND IU */ +#define FCP_ABORT_TASK_SET 0x02 +#define FCP_CLEAR_TASK_SET 0x04 +#define FCP_LOGICAL_UNIT_RESET 0x10 +#define FCP_TARGET_RESET 0x20 +#define FCP_CLEAR_ACA 0x40 + +/* FC-LS */ +#define FC_LS_RLS 0x0f +#define FC_LS_ADISC0x52 +#define FC_LS_RPS 0x56 +#define FC_LS_RSCN 0x61 +#define FC_LS_RNID 0x78 + +/* + * FC-GS-2 + */ +#define FC_CT_REVISION 0x01 +#define FC_CT_DIRECTORY_SERVICE0xFC +#define FC_CT_NAME_SERVER 0x02 +#define FC_CT_SYNCHRONOUS 0x00 +#define FC_CT_GID_PN 0x0121 +#define FC_CT_MAX_SIZE 0x1020 +#define FC_CT_ACCEPT 0x8002 +#define FC_CT_REJECT 0x8001 + - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/1] zfcp: Add some statistics provided by the FCP adapter to the sysfs
From: Swen Schillig <[EMAIL PROTECTED]>
The new statistics from the FCP adapter provide informations about the
virtual adapter (subchannel). In order to collect this information the
zFCP driver is extended on one side to query the adapter and on the
other side summarize certain values which can then be fetched on
demand.
These are the attributes are added for each adapter attached to zfcp.
Note that the data refers only to the virtual adapter (subchannel).
The already existing data in the FC transport class refers to the
adapter and one adapter is usually shared across multiple Linux
systems.
 /sys/class/scsi_host/host/seconds_active
 /sys/class/scsi_host/host/requests
 /sys/class/scsi_host/host/megabytes
 /sys/class/scsi_host/host/utilization
These are the statistics on a virtual adapter (subchannel) level. In
addition latency information is provided on a SCSI device level (LUN)
which can be found at the following location
For each SCSI device, these attributes are added for latencies. They
include the average of the latencies in the fabrice and in the
channel.
 /sys/class/scsi_device//device/cmd_latency
 /sys/class/scsi_device//device/read_latency
 /sys/class/scsi_device//device/write_latency
The information provided is raw and not modified or interpreted by any
means. No interpretation or modification of the values is done by the
zFCP driver. The individual values are summed up during normal
operation of the virtual adapter. An overrun of the variables is
neither detected nor treated. The conclusion is that the file has to
be read twice to make a meaningful statement, because only the
differences of the values between the two reads can be used.
The statistics make use of the SCSI mid-layer interface to provide its
data to the user. In detail two hooks from the scsi_host_template are
used to integrate the zFCP statistics.
  struct scsi_host_template {
      ...
      .shost_attrs = zfcp_a_stats_attrs,
      .sdev_attrs  = zfcp_sysfs_sdev_attrs,
      ...
  };
Signed-off-by: Swen Schillig <[EMAIL PROTECTED]>
Signed-off-by: Christof Schmitt <[EMAIL PROTECTED]>
---
drivers/s390/scsi/zfcp_aux.c |2
drivers/s390/scsi/zfcp_def.h | 14 +++
drivers/s390/scsi/zfcp_fsf.c | 36 +
drivers/s390/scsi/zfcp_fsf.h | 29 ++-
drivers/s390/scsi/zfcp_scsi.c | 158 ++
5 files changed, 234 insertions(+), 5 deletions(-)
--- a/drivers/s390/scsi/zfcp_aux.c 2008-01-07 17:19:10.0 +0100
+++ b/drivers/s390/scsi/zfcp_aux.c 2008-01-07 17:19:31.0 +0100
@@ -847,6 +847,8 @@ zfcp_unit_enqueue(struct zfcp_port *port
/* mark unit unusable as long as sysfs registration is not complete */
atomic_set_mask(ZFCP_STATUS_COMMON_REMOVE, &unit->status);
+ spin_lock_init(&unit->latencies.lock);
+
if (device_register(&unit->sysfs_device)) {
kfree(unit);
return NULL;
--- a/drivers/s390/scsi/zfcp_def.h 2008-01-07 17:19:10.0 +0100
+++ b/drivers/s390/scsi/zfcp_def.h 2008-01-07 17:19:31.0 +0100
@@ -869,6 +869,18 @@ struct zfcp_erp_action {
struct timer_list timer;
};
+struct latency_cont {
+ u64 channel;
+ u64 fabric;
+ u64 counter;
+};
+
+struct zfcp_latencies {
+ struct latency_cont read;
+ struct latency_cont write;
+ struct latency_cont cmd;
+ spinlock_t lock;
+};
struct zfcp_adapter {
struct list_headlist; /* list of adapters */
@@ -884,6 +896,7 @@ struct zfcp_adapter {
u32 adapter_features; /* FCP channel features */
u32 connection_features; /* host connection
features */
u32hardware_version; /* of FCP channel */
+ u16 timer_ticks; /* time int for a tick */
struct Scsi_Host*scsi_host;/* Pointer to mid-layer */
struct list_headport_list_head;/* remote port list */
struct list_headport_remove_lh;/* head of ports to be
@@ -983,6 +996,7 @@ struct zfcp_unit {
struct scsi_device *device;/* scsi device struct pointer */
struct zfcp_erp_action erp_action; /* pending error recovery */
atomic_t erp_counter;
+ struct zfcp_latencies latencies;
};
/* FSF request */
--- a/drivers/s390/scsi/zfcp_fsf.c 2008-01-07 17:19:10.0 +0100
+++ b/drivers/s390/scsi/zfcp_fsf.c 2008-01-07 17:19:31.0 +0100
@@ -2080,6 +2080,7 @@ zfcp_fsf_exchange_config_evaluate(struct
fc_host_supported_classes(shost) =
FC_COS_CLASS2 | FC_COS_CLASS3;
adapter->hydra_version = bottom->adapter_type;
+ adapter->timer_ticks = bottom->timer_interval;
[patch 0/1] Resend of zfcp adapter statistics
James, this is the extension for zfcp that adds some statistics retrieved from the FCP adapter. This version already includes the fixes and applies on top of the other patches on scsi-misc. Like Swen mentioned before, this adds new data attributes that only refer to one Linux system. Similar looking attributes for the FC host refer to the adapter. Since the FCP adapter can be shared across multiple Linux systems, the two groups of attributes have a different scope. -- Christof Schmitt - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/1] blktrace: Add blktrace ioctls to SCSI generic devices
From: Christof Schmitt <[EMAIL PROTECTED]>
Since the SCSI layer uses the request queues from the block layer, blktrace can
also be used to trace the requests to all SCSI devices (like SCSI tape drives),
not only disks. The only missing part is the ioctl interface to start and stop
tracing.
This patch adds the SETUP, START, STOP and TEARDOWN ioctls from blktrace to the
sg device files. With this change, blktrace can be used for SCSI devices like
for disks, e.g.: blktrace -d /dev/sg1 -o - | blkparse -i -
Signed-off-by: Christof Schmitt <[EMAIL PROTECTED]>
---
block/blktrace.c | 24 ++--
block/compat_ioctl.c |5 -
drivers/scsi/sg.c| 12
include/linux/blktrace_api.h | 12 ++--
4 files changed, 40 insertions(+), 13 deletions(-)
--- a/block/blktrace.c 2007-12-17 21:51:45.0 +0100
+++ b/block/blktrace.c 2007-12-17 21:51:45.0 +0100
@@ -236,7 +236,7 @@ static void blk_trace_cleanup(struct blk
kfree(bt);
}
-static int blk_trace_remove(struct request_queue *q)
+int blk_trace_remove(struct request_queue *q)
{
struct blk_trace *bt;
@@ -250,6 +250,7 @@ static int blk_trace_remove(struct reque
return 0;
}
+EXPORT_SYMBOL_GPL(blk_trace_remove);
static int blk_dropped_open(struct inode *inode, struct file *filp)
{
@@ -317,18 +318,17 @@ static struct rchan_callbacks blk_relay_
/*
* Setup everything required to start tracing
*/
-int do_blk_trace_setup(struct request_queue *q, struct block_device *bdev,
+int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
struct blk_user_trace_setup *buts)
{
struct blk_trace *old_bt, *bt = NULL;
struct dentry *dir = NULL;
- char b[BDEVNAME_SIZE];
int ret, i;
if (!buts->buf_size || !buts->buf_nr)
return -EINVAL;
- strcpy(buts->name, bdevname(bdev, b));
+ strcpy(buts->name, name);
/*
* some device names have larger paths - convert the slashes
@@ -353,7 +353,7 @@ int do_blk_trace_setup(struct request_qu
goto err;
bt->dir = dir;
- bt->dev = bdev->bd_dev;
+ bt->dev = dev;
atomic_set(&bt->dropped, 0);
ret = -EIO;
@@ -400,8 +400,8 @@ err:
return ret;
}
-static int blk_trace_setup(struct request_queue *q, struct block_device *bdev,
- char __user *arg)
+int blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
+ char __user *arg)
{
struct blk_user_trace_setup buts;
int ret;
@@ -410,7 +410,7 @@ static int blk_trace_setup(struct reques
if (ret)
return -EFAULT;
- ret = do_blk_trace_setup(q, bdev, &buts);
+ ret = do_blk_trace_setup(q, name, dev, &buts);
if (ret)
return ret;
@@ -419,8 +419,9 @@ static int blk_trace_setup(struct reques
return 0;
}
+EXPORT_SYMBOL_GPL(blk_trace_setup);
-static int blk_trace_startstop(struct request_queue *q, int start)
+int blk_trace_startstop(struct request_queue *q, int start)
{
struct blk_trace *bt;
int ret;
@@ -453,6 +454,7 @@ static int blk_trace_startstop(struct re
return ret;
}
+EXPORT_SYMBOL_GPL(blk_trace_startstop);
/**
* blk_trace_ioctl: - handle the ioctls associated with tracing
@@ -465,6 +467,7 @@ int blk_trace_ioctl(struct block_device
{
struct request_queue *q;
int ret, start = 0;
+ char b[BDEVNAME_SIZE];
q = bdev_get_queue(bdev);
if (!q)
@@ -474,7 +477,8 @@ int blk_trace_ioctl(struct block_device
switch (cmd) {
case BLKTRACESETUP:
- ret = blk_trace_setup(q, bdev, arg);
+ strcpy(b, bdevname(bdev, b));
+ ret = blk_trace_setup(q, b, bdev->bd_dev, arg);
break;
case BLKTRACESTART:
start = 1;
--- a/drivers/scsi/sg.c 2007-12-17 21:51:45.0 +0100
+++ b/drivers/scsi/sg.c 2007-12-17 22:36:30.0 +0100
@@ -48,6 +48,7 @@ static int sg_version_num = 30534;/* 2
#include
#include
#include
+#include
#include "scsi.h"
#include
@@ -1063,6 +1064,17 @@ sg_ioctl(struct inode *inode, struct fil
case BLKSECTGET:
return put_user(sdp->device->request_queue->max_sectors * 512,
ip);
+ case BLKTRACESETUP:
+ return blk_trace_setup(sdp->device->request_queue,
+ sdp->disk->disk_name,
+ sdp->device->sdev_gendev.devt,
+ (char *)arg);
+ case BLKTRACESTART:
+ return blk_trace_startstop(sdp->device->request_queue, 1);
+ case BLKTRACESTOP:
+ return blk_trace_startstop(sdp->device-&g
[patch 0/1] Add blktrace interface to sg device files
There was no response on this patch last time, so i am resubmitting it here. It applies on the current linux-2.6 git tree. -- Christof Schmitt - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 6/6] zfcp: Hold queue lock when checking port/unit handle for task management cmd
From: Christof Schmitt <[EMAIL PROTECTED]>
We need to hold the queue-lock when checking whether we still have a valid
unit/port handle for the task management command, i.e whether we can issue this
request for this unit/port. If the error recovery is about to close this
unit/port, then it competes for the queue-lock. If the close request issued by
the error recovery wins, then it is guaranteed that this unit/port has been
blocked for other requests.
Signed-off-by: Christof Schmitt <[EMAIL PROTECTED]>
Signed-off-by: Martin Peschke <[EMAIL PROTECTED]>
---
drivers/s390/scsi/zfcp_fsf.c | 26 +-
1 file changed, 9 insertions(+), 17 deletions(-)
--- a/drivers/s390/scsi/zfcp_fsf.c 2007-12-20 11:19:01.0 +0100
+++ b/drivers/s390/scsi/zfcp_fsf.c 2007-12-20 11:19:19.0 +0100
@@ -3774,6 +3774,10 @@ zfcp_fsf_send_fcp_command_task_managemen
goto out;
}
+ if (unlikely(!atomic_test_mask(ZFCP_STATUS_COMMON_UNBLOCKED,
+ &unit->status)))
+ goto unit_blocked;
+
/*
* Used to decide on proper handler in the return path,
* could be either zfcp_fsf_send_fcp_command_task_handler or
@@ -3807,25 +3811,13 @@ zfcp_fsf_send_fcp_command_task_managemen
zfcp_fsf_start_timer(fsf_req, ZFCP_SCSI_ER_TIMEOUT);
retval = zfcp_fsf_req_send(fsf_req);
- if (retval) {
- ZFCP_LOG_INFO("error: Could not send an FCP-command (task "
- "management) on adapter %s, port 0x%016Lx for "
- "unit LUN 0x%016Lx\n",
- zfcp_get_busid_by_adapter(adapter),
- unit->port->wwpn,
- unit->fcp_lun);
- zfcp_fsf_req_free(fsf_req);
- fsf_req = NULL;
+ if (!retval)
goto out;
- }
- ZFCP_LOG_TRACE("Send FCP Command (task management function) initiated "
- "(adapter %s, port 0x%016Lx, unit 0x%016Lx, "
- "tm_flags=0x%x)\n",
- zfcp_get_busid_by_adapter(adapter),
- unit->port->wwpn,
- unit->fcp_lun,
- tm_flags);
+ unit_blocked:
+ zfcp_fsf_req_free(fsf_req);
+ fsf_req = NULL;
+
out:
write_unlock_irqrestore(&adapter->request_queue.queue_lock, lock_flags);
return fsf_req;
--
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 3/6] zfcp: Hold queue lock when checking port/unit handle for abort command
From: Christof Schmitt <[EMAIL PROTECTED]>
We need to hold the queue-lock when checking whether we still have a valid
unit/port handle for the abort command, i.e whether we can issue this request
for this unit/port. If the error recovery is about to close this unit/port,
then it competes for the queue-lock. If the close request issued by the error
recovery wins, then it is guaranteed that this unit/port has been blocked for
other requests.
Signed-off-by: Christof Schmitt <[EMAIL PROTECTED]>
Signed-off-by: Martin Peschke <[EMAIL PROTECTED]>
---
drivers/s390/scsi/zfcp_fsf.c | 21 -
1 file changed, 8 insertions(+), 13 deletions(-)
--- a/drivers/s390/scsi/zfcp_fsf.c 2007-12-20 11:18:03.0 +0100
+++ b/drivers/s390/scsi/zfcp_fsf.c 2007-12-20 11:18:23.0 +0100
@@ -1116,6 +1116,10 @@ zfcp_fsf_abort_fcp_command(unsigned long
goto out;
}
+ if (unlikely(!atomic_test_mask(ZFCP_STATUS_COMMON_UNBLOCKED,
+ &unit->status)))
+ goto unit_blocked;
+
sbale = zfcp_qdio_sbale_req(fsf_req, fsf_req->sbal_curr, 0);
sbale[0].flags |= SBAL_FLAGS0_TYPE_READ;
sbale[1].flags |= SBAL_FLAGS_LAST_ENTRY;
@@ -1131,22 +1135,13 @@ zfcp_fsf_abort_fcp_command(unsigned long
zfcp_fsf_start_timer(fsf_req, ZFCP_SCSI_ER_TIMEOUT);
retval = zfcp_fsf_req_send(fsf_req);
- if (retval) {
- ZFCP_LOG_INFO("error: Failed to send abort command request "
- "on adapter %s, port 0x%016Lx, unit 0x%016Lx\n",
- zfcp_get_busid_by_adapter(adapter),
- unit->port->wwpn, unit->fcp_lun);
+ if (!retval)
+ goto out;
+
+ unit_blocked:
zfcp_fsf_req_free(fsf_req);
fsf_req = NULL;
- goto out;
- }
- ZFCP_LOG_DEBUG("Abort FCP Command request initiated "
- "(adapter%s, port d_id=0x%06x, "
- "unit x%016Lx, old_req_id=0x%lx)\n",
- zfcp_get_busid_by_adapter(adapter),
- unit->port->d_id,
- unit->fcp_lun, old_req_id);
out:
write_unlock_irqrestore(&adapter->request_queue.queue_lock, lock_flags);
return fsf_req;
--
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 5/6] zfcp: Hold queue lock when checking port/unit handle for FCP command
From: Christof Schmitt <[EMAIL PROTECTED]>
We need to hold the queue-lock when checking whether we still have a valid
unit/port handle for the FCP command, i.e whether we can issue this request for
this unit/port. If the error recovery is about to close this unit/port, then it
competes for the queue-lock. If the close request issued by the error recovery
wins, then it is guaranteed that this unit/port has been blocked for other
requests.
Signed-off-by: Christof Schmitt <[EMAIL PROTECTED]>
Signed-off-by: Martin Peschke <[EMAIL PROTECTED]>
---
drivers/s390/scsi/zfcp_fsf.c |7 +++
drivers/s390/scsi/zfcp_scsi.c |8 +++-
2 files changed, 10 insertions(+), 5 deletions(-)
--- a/drivers/s390/scsi/zfcp_fsf.c 2007-12-20 11:18:42.0 +0100
+++ b/drivers/s390/scsi/zfcp_fsf.c 2007-12-20 11:19:01.0 +0100
@@ -3593,6 +3593,12 @@ zfcp_fsf_send_fcp_command_task(struct zf
goto failed_req_create;
}
+ if (unlikely(!atomic_test_mask(ZFCP_STATUS_COMMON_UNBLOCKED,
+ &unit->status))) {
+ retval = -EBUSY;
+ goto unit_blocked;
+ }
+
zfcp_unit_get(unit);
fsf_req->unit = unit;
@@ -3733,6 +3739,7 @@ zfcp_fsf_send_fcp_command_task(struct zf
send_failed:
no_fit:
failed_scsi_cmnd:
+ unit_blocked:
zfcp_unit_put(unit);
zfcp_fsf_req_free(fsf_req);
fsf_req = NULL;
--- a/drivers/s390/scsi/zfcp_scsi.c 2007-12-20 11:15:10.0 +0100
+++ b/drivers/s390/scsi/zfcp_scsi.c 2007-12-20 11:19:01.0 +0100
@@ -258,8 +258,9 @@ zfcp_scsi_command_async(struct zfcp_adap
goto out;
}
- if (unlikely(
-!atomic_test_mask(ZFCP_STATUS_COMMON_UNBLOCKED, &unit->status))) {
+ tmp = zfcp_fsf_send_fcp_command_task(adapter, unit, scpnt, use_timer,
+ZFCP_REQ_AUTO_CLEANUP);
+ if (unlikely(tmp == -EBUSY)) {
ZFCP_LOG_DEBUG("adapter %s not ready or unit 0x%016Lx "
"on port 0x%016Lx in recovery\n",
zfcp_get_busid_by_unit(unit),
@@ -268,9 +269,6 @@ zfcp_scsi_command_async(struct zfcp_adap
goto out;
}
- tmp = zfcp_fsf_send_fcp_command_task(adapter, unit, scpnt, use_timer,
-ZFCP_REQ_AUTO_CLEANUP);
-
if (unlikely(tmp < 0)) {
ZFCP_LOG_DEBUG("error: initiation of Send FCP Cmnd failed\n");
retval = SCSI_MLQUEUE_HOST_BUSY;
--
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 4/6] zfcp: Hold queue lock when checking port handle for ELS command
From: Christof Schmitt <[EMAIL PROTECTED]>
We need to hold the queue-lock when checking whether we still have a valid port
handle for the ELS command, i.e whether we can issue this request for this
port. If the error recovery is about to close this port, then it competes for
the queue-lock. If the close request issued by the error recovery wins, then it
is guaranteed that this port has been blocked for other requests.
Signed-off-by: Christof Schmitt <[EMAIL PROTECTED]>
Signed-off-by: Martin Peschke <[EMAIL PROTECTED]>
---
drivers/s390/scsi/zfcp_erp.c |2 +-
drivers/s390/scsi/zfcp_fsf.c |7 +++
2 files changed, 8 insertions(+), 1 deletion(-)
--- a/drivers/s390/scsi/zfcp_erp.c 2007-12-20 11:17:46.0 +0100
+++ b/drivers/s390/scsi/zfcp_erp.c 2007-12-20 11:18:42.0 +0100
@@ -456,7 +456,7 @@ zfcp_test_link(struct zfcp_port *port)
zfcp_port_get(port);
retval = zfcp_erp_adisc(port);
- if (retval != 0) {
+ if (retval != 0 && retval != -EBUSY) {
zfcp_port_put(port);
ZFCP_LOG_NORMAL("reopen needed for port 0x%016Lx "
"on adapter %s\n ", port->wwpn,
--- a/drivers/s390/scsi/zfcp_fsf.c 2007-12-20 11:18:23.0 +0100
+++ b/drivers/s390/scsi/zfcp_fsf.c 2007-12-20 11:18:42.0 +0100
@@ -1668,6 +1668,12 @@ zfcp_fsf_send_els(struct zfcp_send_els *
goto failed_req;
}
+ if (unlikely(!atomic_test_mask(ZFCP_STATUS_COMMON_UNBLOCKED,
+ &els->port->status))) {
+ ret = -EBUSY;
+ goto port_blocked;
+ }
+
sbale = zfcp_qdio_sbale_req(fsf_req, fsf_req->sbal_curr, 0);
if (zfcp_use_one_sbal(els->req, els->req_count,
els->resp, els->resp_count)){
@@ -1749,6 +1755,7 @@ zfcp_fsf_send_els(struct zfcp_send_els *
"0x%06x)\n", zfcp_get_busid_by_adapter(adapter), d_id);
goto out;
+ port_blocked:
failed_send:
zfcp_fsf_req_free(fsf_req);
--
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/6] zfcp: fix use after free bug.
From: Heiko Carstens <[EMAIL PROTECTED]>
zfcp_erp_strategy_check_fsfreq() checks if it is safe to access the
fsf_req associated with the erp_action that gets passed. To test if
it is safe it accesses the fsf_req in order to get its index into
the hash list. This is broken since the fsf_req might be freed already
and the read index has no meaning. It could lead to memory corruption.
Fix this by introducing a new zfcp_reqlist_find_safe() method which
just checks if addresses are equal. This is slower, but only gets
called in case of error recovery.
Signed-off-by: Heiko Carstens <[EMAIL PROTECTED]>
Signed-off-by: Martin Schwidefsky <[EMAIL PROTECTED]>
Signed-off-by: Christof Schmitt <[EMAIL PROTECTED]>
Signed-off-by: Martin Peschke <[EMAIL PROTECTED]>
---
drivers/s390/scsi/zfcp_def.h | 14 ++
drivers/s390/scsi/zfcp_erp.c |3 ++-
2 files changed, 16 insertions(+), 1 deletion(-)
--- a/drivers/s390/scsi/zfcp_def.h 2007-12-20 11:15:10.0 +0100
+++ b/drivers/s390/scsi/zfcp_def.h 2007-12-20 11:17:46.0 +0100
@@ -1123,6 +1123,20 @@ zfcp_reqlist_find(struct zfcp_adapter *a
return NULL;
}
+static inline struct zfcp_fsf_req *
+zfcp_reqlist_find_safe(struct zfcp_adapter *adapter, struct zfcp_fsf_req *req)
+{
+ struct zfcp_fsf_req *request;
+ unsigned int idx;
+
+ for (idx = 0; idx < REQUEST_LIST_SIZE; idx++) {
+ list_for_each_entry(request, &adapter->req_list[idx], list)
+ if (request == req)
+ return request;
+ }
+ return NULL;
+}
+
/*
* functions needed for reference/usage counting
*/
--- a/drivers/s390/scsi/zfcp_erp.c 2007-12-20 11:15:10.0 +0100
+++ b/drivers/s390/scsi/zfcp_erp.c 2007-12-20 11:17:46.0 +0100
@@ -846,7 +846,8 @@ zfcp_erp_strategy_check_fsfreq(struct zf
if (erp_action->fsf_req) {
/* take lock to ensure that request is not deleted meanwhile */
spin_lock(&adapter->req_list_lock);
- if (zfcp_reqlist_find(adapter, erp_action->fsf_req->req_id)) {
+ if (zfcp_reqlist_find_safe(adapter, erp_action->fsf_req) &&
+ erp_action->fsf_req->erp_action == erp_action) {
/* fsf_req still exists */
debug_text_event(adapter->erp_dbf, 3, "a_ca_req");
debug_event(adapter->erp_dbf, 3, &erp_action->fsf_req,
--
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 2/6] zfcp: Fix evaluation of port handles in abort handler
From: Christof Schmitt <[EMAIL PROTECTED]>
According to the FSF spec, word 0 (bytes 0-3) has the handle
specified with the abort command and word 1 (bytes 4-7) has the
handle for the command to be aborted. Fix the if statements
that try to compare those.
Signed-off-by: Christof Schmitt <[EMAIL PROTECTED]>
Signed-off-by: Martin Peschke <[EMAIL PROTECTED]>
---
drivers/s390/scsi/zfcp_fsf.c |9 -
1 file changed, 4 insertions(+), 5 deletions(-)
--- a/drivers/s390/scsi/zfcp_fsf.c 2007-12-20 11:16:38.0 +0100
+++ b/drivers/s390/scsi/zfcp_fsf.c 2007-12-20 11:18:03.0 +0100
@@ -1164,8 +1164,8 @@ zfcp_fsf_abort_fcp_command_handler(struc
{
int retval = -EINVAL;
struct zfcp_unit *unit;
- unsigned char status_qual =
- new_fsf_req->qtcb->header.fsf_status_qual.word[0];
+ union fsf_status_qual *fsf_stat_qual =
+ &new_fsf_req->qtcb->header.fsf_status_qual;
if (new_fsf_req->status & ZFCP_STATUS_FSFREQ_ERROR) {
/* do not set ZFCP_STATUS_FSFREQ_ABORTSUCCEEDED */
@@ -1178,7 +1178,7 @@ zfcp_fsf_abort_fcp_command_handler(struc
switch (new_fsf_req->qtcb->header.fsf_status) {
case FSF_PORT_HANDLE_NOT_VALID:
- if (status_qual >> 4 != status_qual % 0xf) {
+ if (fsf_stat_qual->word[0] != fsf_stat_qual->word[1]) {
debug_text_event(new_fsf_req->adapter->erp_dbf, 3,
"fsf_s_phand_nv0");
/*
@@ -1207,8 +1207,7 @@ zfcp_fsf_abort_fcp_command_handler(struc
break;
case FSF_LUN_HANDLE_NOT_VALID:
- if (status_qual >> 4 != status_qual % 0xf) {
- /* 2 */
+ if (fsf_stat_qual->word[0] != fsf_stat_qual->word[1]) {
debug_text_event(new_fsf_req->adapter->erp_dbf, 3,
"fsf_s_lhand_nv0");
/*
--
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 0/6] zfcp fixes for scsi-misc
James, here are some zfcp fixes. They apply on the current version of scsi-misc. Regards, Christof Schmitt - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] blktrace: Add blktrace ioctls to SCSI generic devices
Since the SCSI layer uses the request queues from the block layer,
blktrace can also be used to trace the requests to all SCSI devices
(like SCSI tape drives), not only disks. The only missing part is the
ioctl interface to start and stop tracing.
This patch adds the SETUP, START, STOP and TEARDOWN ioctls from
blktrace to the sg device files. With this change, blktrace can be
used for SCSI devices like for disks, e.g.:
blktrace -d /dev/sg1 -o - | blkparse -i -
Signed-off-by: Christof Schmitt <[EMAIL PROTECTED]>
---
block/blktrace.c | 24 ++--
block/compat_ioctl.c |5 -
drivers/scsi/sg.c| 12
include/linux/blktrace_api.h | 12 ++--
4 files changed, 40 insertions(+), 13 deletions(-)
--- a/block/blktrace.c 2007-12-17 21:51:45.0 +0100
+++ b/block/blktrace.c 2007-12-17 21:51:45.0 +0100
@@ -236,7 +236,7 @@ static void blk_trace_cleanup(struct blk
kfree(bt);
}
-static int blk_trace_remove(struct request_queue *q)
+int blk_trace_remove(struct request_queue *q)
{
struct blk_trace *bt;
@@ -250,6 +250,7 @@ static int blk_trace_remove(struct reque
return 0;
}
+EXPORT_SYMBOL_GPL(blk_trace_remove);
static int blk_dropped_open(struct inode *inode, struct file *filp)
{
@@ -317,18 +318,17 @@ static struct rchan_callbacks blk_relay_
/*
* Setup everything required to start tracing
*/
-int do_blk_trace_setup(struct request_queue *q, struct block_device *bdev,
+int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
struct blk_user_trace_setup *buts)
{
struct blk_trace *old_bt, *bt = NULL;
struct dentry *dir = NULL;
- char b[BDEVNAME_SIZE];
int ret, i;
if (!buts->buf_size || !buts->buf_nr)
return -EINVAL;
- strcpy(buts->name, bdevname(bdev, b));
+ strcpy(buts->name, name);
/*
* some device names have larger paths - convert the slashes
@@ -353,7 +353,7 @@ int do_blk_trace_setup(struct request_qu
goto err;
bt->dir = dir;
- bt->dev = bdev->bd_dev;
+ bt->dev = dev;
atomic_set(&bt->dropped, 0);
ret = -EIO;
@@ -400,8 +400,8 @@ err:
return ret;
}
-static int blk_trace_setup(struct request_queue *q, struct block_device *bdev,
- char __user *arg)
+int blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
+ char __user *arg)
{
struct blk_user_trace_setup buts;
int ret;
@@ -410,7 +410,7 @@ static int blk_trace_setup(struct reques
if (ret)
return -EFAULT;
- ret = do_blk_trace_setup(q, bdev, &buts);
+ ret = do_blk_trace_setup(q, name, dev, &buts);
if (ret)
return ret;
@@ -419,8 +419,9 @@ static int blk_trace_setup(struct reques
return 0;
}
+EXPORT_SYMBOL_GPL(blk_trace_setup);
-static int blk_trace_startstop(struct request_queue *q, int start)
+int blk_trace_startstop(struct request_queue *q, int start)
{
struct blk_trace *bt;
int ret;
@@ -453,6 +454,7 @@ static int blk_trace_startstop(struct re
return ret;
}
+EXPORT_SYMBOL_GPL(blk_trace_startstop);
/**
* blk_trace_ioctl: - handle the ioctls associated with tracing
@@ -465,6 +467,7 @@ int blk_trace_ioctl(struct block_device
{
struct request_queue *q;
int ret, start = 0;
+ char b[BDEVNAME_SIZE];
q = bdev_get_queue(bdev);
if (!q)
@@ -474,7 +477,8 @@ int blk_trace_ioctl(struct block_device
switch (cmd) {
case BLKTRACESETUP:
- ret = blk_trace_setup(q, bdev, arg);
+ strcpy(b, bdevname(bdev, b));
+ ret = blk_trace_setup(q, b, bdev->bd_dev, arg);
break;
case BLKTRACESTART:
start = 1;
--- a/drivers/scsi/sg.c 2007-12-17 21:51:45.0 +0100
+++ b/drivers/scsi/sg.c 2007-12-17 22:36:30.0 +0100
@@ -48,6 +48,7 @@ static int sg_version_num = 30534;/* 2
#include
#include
#include
+#include
#include "scsi.h"
#include
@@ -1063,6 +1064,17 @@ sg_ioctl(struct inode *inode, struct fil
case BLKSECTGET:
return put_user(sdp->device->request_queue->max_sectors * 512,
ip);
+ case BLKTRACESETUP:
+ return blk_trace_setup(sdp->device->request_queue,
+ sdp->disk->disk_name,
+ sdp->device->sdev_gendev.devt,
+ (char *)arg);
+ case BLKTRACESTART:
+ return blk_trace_startstop(sdp->device->request_queue, 1);
+ case BLKTRACESTOP:
+ return blk_trace_startstop(sdp->device->request_queue, 0);
+ case BLKTR
Re: [RFC] blktrace interface for sg devices
On Thu, Dec 13, 2007 at 10:19:42AM +0100, Jens Axboe wrote:
[...]
> I think this approach is the simplest and right way to do it. Tracing is
> really just tied to the "transport" (transport here meaning how we
> transport commands to the device), and even character scsi devices use
> the block layer queue for this operation, as you note.
>
> Let me know when you are happy with the patch, and I'll queue it up for
> 2.6.25.
> > @@ -1066,6 +1068,16 @@ sg_ioctl(struct inode *inode, struct fil
> > case BLKSECTGET:
> > return put_user(sdp->device->request_queue->max_sectors * 512,
> > ip);
> > + case BLKTRACESETUP:
> > + {
> > + return blk_trace_setup(sdp->device->request_queue ,
> > sdp->device->sdev_gendev.bus_id, &sdp->device->sdev_gendev, arg);
> > + }
>
> Don't need those braces, some other space and long line style issues as
> well.
>
> > --- a/include/linux/blkdev.h2007-12-13 08:48:23.0 +0100
> > +++ b/include/linux/blkdev.h2007-12-13 08:48:25.0 +0100
> > @@ -747,6 +747,16 @@ static inline void blkdev_dequeue_reques
> > elv_dequeue_request(req->q, req);
> > }
> >
> > +#ifdef CONFIG_BLK_DEV_IO_TRACE
> > +extern int blk_trace_setup(request_queue_t *q, char * name, dev_t dev,
> > char __user *arg);
> > +extern int blk_trace_startstop(request_queue_t *q, int start);
> > +extern int blk_trace_remove(request_queue_t *q);
> > +#else
> > +#define blk_trace_setup(q, name, dev, arg) do { } while(0)
> > +#define blk_trace_startstop(q, start) do { } while(0)
> > +#define blk_trace_remove(q) do { } while(0)
> > +#endif
> > +
>
> Put these in the blktrace include file.
Thanks for your input. I will prepare and send an updated version of
the patch. I also want to do some more testing, especially to see how
i can get the sizes of read and write requests and latencies for SCSI
tape drives.
Christof Schmitt
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] blktrace interface for sg devices
I am referring to the discussion of introducing statistics in the SCSI
layer and the conclusion that blktrace already provides the data:
http://lkml.org/lkml/2006/10/21/72
http://lkml.org/lkml/2006/11/2/141
While blktrace works fine for disk devices, it currently does not
provide data for non-disk devices like tape drives. To close this gap,
i am looking for a way to get the same trace data also from other SCSI
devices.
Since the SCSI layer internally uses the same request queuest for all
devices and the queues already use the blktrace interface, the main
missing part is the interface to enable the tracing for all SCSI
devices.
Attached is a patch that adds the ioctl interface for blktrace to the
sg generic scsi interface. This already allows to get some trace data
for SCSI tape drives, although i have to do more testing.
For testing, any sg device file can be passed to blktrace, e.g.:
# blktrace -d /dev/sg1 -o - | blkparse -i -
I am seeking input in this approach: Is this approach worth pursuing
to enable blktrace to trace SCSI tape drives? Would there be a better
approach to get this trace data?
Christof Schmitt
---
block/blktrace.c | 19 +++
drivers/scsi/sg.c | 12
include/linux/blkdev.h | 10 ++
3 files changed, 33 insertions(+), 8 deletions(-)
--- a/block/blktrace.c 2007-12-13 08:48:23.0 +0100
+++ b/block/blktrace.c 2007-12-13 08:48:25.0 +0100
@@ -231,7 +231,7 @@ static void blk_trace_cleanup(struct blk
kfree(bt);
}
-static int blk_trace_remove(struct request_queue *q)
+int blk_trace_remove(struct request_queue *q)
{
struct blk_trace *bt;
@@ -245,6 +245,7 @@ static int blk_trace_remove(struct reque
return 0;
}
+EXPORT_SYMBOL_GPL(blk_trace_remove);
static int blk_dropped_open(struct inode *inode, struct file *filp)
{
@@ -312,13 +313,11 @@ static struct rchan_callbacks blk_relay_
/*
* Setup everything required to start tracing
*/
-static int blk_trace_setup(struct request_queue *q, struct block_device *bdev,
- char __user *arg)
+int blk_trace_setup(struct request_queue *q, char *name, dev_t dev, char
__user *arg)
{
struct blk_user_trace_setup buts;
struct blk_trace *old_bt, *bt = NULL;
struct dentry *dir = NULL;
- char b[BDEVNAME_SIZE];
int ret, i;
if (copy_from_user(&buts, arg, sizeof(buts)))
@@ -327,7 +326,7 @@ static int blk_trace_setup(struct reques
if (!buts.buf_size || !buts.buf_nr)
return -EINVAL;
- strcpy(buts.name, bdevname(bdev, b));
+ strcpy(buts.name, name);
/*
* some device names have larger paths - convert the slashes
@@ -355,7 +354,7 @@ static int blk_trace_setup(struct reques
goto err;
bt->dir = dir;
- bt->dev = bdev->bd_dev;
+ bt->dev = dev;
atomic_set(&bt->dropped, 0);
ret = -EIO;
@@ -400,8 +399,9 @@ err:
}
return ret;
}
+EXPORT_SYMBOL_GPL(blk_trace_setup);
-static int blk_trace_startstop(struct request_queue *q, int start)
+int blk_trace_startstop(struct request_queue *q, int start)
{
struct blk_trace *bt;
int ret;
@@ -434,6 +434,7 @@ static int blk_trace_startstop(struct re
return ret;
}
+EXPORT_SYMBOL_GPL(blk_trace_startstop);
/**
* blk_trace_ioctl: - handle the ioctls associated with tracing
@@ -446,6 +447,7 @@ int blk_trace_ioctl(struct block_device
{
struct request_queue *q;
int ret, start = 0;
+ char b[BDEVNAME_SIZE];
q = bdev_get_queue(bdev);
if (!q)
@@ -455,7 +457,8 @@ int blk_trace_ioctl(struct block_device
switch (cmd) {
case BLKTRACESETUP:
- ret = blk_trace_setup(q, bdev, arg);
+ strcpy(b, bdevname(bdev, b));
+ ret = blk_trace_setup(q, b, bdev->bd_dev, arg);
break;
case BLKTRACESTART:
start = 1;
--- a/drivers/scsi/sg.c 2007-12-13 08:48:23.0 +0100
+++ b/drivers/scsi/sg.c 2007-12-13 08:48:25.0 +0100
@@ -55,6 +55,8 @@ static int sg_version_num = 30534;/* 2
#include
#include
+#include
+
#include "scsi_logging.h"
#ifdef CONFIG_SCSI_PROC_FS
@@ -1066,6 +1068,16 @@ sg_ioctl(struct inode *inode, struct fil
case BLKSECTGET:
return put_user(sdp->device->request_queue->max_sectors * 512,
ip);
+ case BLKTRACESETUP:
+ {
+ return blk_trace_setup(sdp->device->request_queue ,
sdp->device->sdev_gendev.bus_id, &sdp->device->sdev_gendev, arg);
+ }
+ case BLKTRACESTART:
+ return blk_trace_startstop(sdp->device->request_queue, 1);
+ case BLKTRACESTOP:
+ return blk_trace_startstop(sdp->device->request_queue, 0);
+ case BLKTRACETEARDOWN:
+
Update: [PATCH] FC transport: Allow LLDD to disable LUN scanning
From: Christof Schmitt <[EMAIL PROTECTED]>
Introduce a flag that allows the SCSI low level device driver to
disable the automatic scanning for LUNs from the FC transport class.
This is for the zfcp device driver that manages the attached units on
its own and does not want to have the automatic discovery of other
LUNs.
Signed-off-by: Christof Schmitt <[EMAIL PROTECTED]>
---
drivers/scsi/scsi_transport_fc.c |4 +++-
include/scsi/scsi_transport_fc.h |2 ++
2 files changed, 5 insertions(+), 1 deletion(-)
--- linux-2.6.orig/drivers/scsi/scsi_transport_fc.c
+++ linux-2.6/drivers/scsi/scsi_transport_fc.c
@@ -2988,10 +2988,12 @@ fc_scsi_scan_rport(struct work_struct *w
struct fc_rport *rport =
container_of(work, struct fc_rport, scan_work);
struct Scsi_Host *shost = rport_to_shost(rport);
+ struct fc_internal *i = to_fc_internal(shost->transportt);
unsigned long flags;
if ((rport->port_state == FC_PORTSTATE_ONLINE) &&
- (rport->roles & FC_PORT_ROLE_FCP_TARGET)) {
+ (rport->roles & FC_PORT_ROLE_FCP_TARGET) &&
+ !(i->f->disable_target_scan)) {
scsi_scan_target(&rport->dev, rport->channel,
rport->scsi_target_id, SCAN_WILD_CARD, 1);
}
--- linux-2.6.orig/include/scsi/scsi_transport_fc.h
+++ linux-2.6/include/scsi/scsi_transport_fc.h
@@ -632,6 +632,8 @@ struct fc_function_template {
unsigned long show_host_fabric_name:1;
unsigned long show_host_symbolic_name:1;
unsigned long show_host_system_hostname:1;
+
+ unsigned long disable_target_scan:1;
};
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] FC transport: Allow LLDD to disable LUN scanning
On Tue, Aug 21, 2007 at 09:46:34AM -0600, Matthew Wilcox wrote: > The comment exceeds 80 columns. Why have a comment at all, none of the > other fields here do, and it doesn't seem to add anything to me. Agree. I will remove the comment. - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] FC transport: Allow LLDD to disable LUN scanning
From: Christof Schmitt <[EMAIL PROTECTED]>
Introduce a flag that allows the SCSI low level device driver to
disable the automatic scanning for LUNs from the FC transport class.
This is for the zfcp device driver that manages the attached units on
its own and does not want to have the automatic discovery of other
LUNs.
Signed-off-by: Christof Schmitt <[EMAIL PROTECTED]>
---
drivers/scsi/scsi_transport_fc.c |4 +++-
include/scsi/scsi_transport_fc.h |3 +++
2 files changed, 6 insertions(+), 1 deletion(-)
--- linux-2.6.orig/drivers/scsi/scsi_transport_fc.c
+++ linux-2.6/drivers/scsi/scsi_transport_fc.c
@@ -2988,10 +2988,12 @@ fc_scsi_scan_rport(struct work_struct *w
struct fc_rport *rport =
container_of(work, struct fc_rport, scan_work);
struct Scsi_Host *shost = rport_to_shost(rport);
+ struct fc_internal *i = to_fc_internal(shost->transportt);
unsigned long flags;
if ((rport->port_state == FC_PORTSTATE_ONLINE) &&
- (rport->roles & FC_PORT_ROLE_FCP_TARGET)) {
+ (rport->roles & FC_PORT_ROLE_FCP_TARGET) &&
+ !(i->f->disable_target_scan)) {
scsi_scan_target(&rport->dev, rport->channel,
rport->scsi_target_id, SCAN_WILD_CARD, 1);
}
--- linux-2.6.orig/include/scsi/scsi_transport_fc.h
+++ linux-2.6/include/scsi/scsi_transport_fc.h
@@ -632,6 +632,9 @@ struct fc_function_template {
unsigned long show_host_fabric_name:1;
unsigned long show_host_symbolic_name:1;
unsigned long show_host_system_hostname:1;
+
+ /* The driver can disable the LUN scanning from the FC transport class
*/
+ unsigned long disable_target_scan:1;
};
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] FC transport: Disable LUN scanning from low level driver
On Tue, Aug 21, 2007 at 09:03:17AM -0400, James Smart wrote: > Nit (but preferred) : change "no_target_scan" to "disable_target_scan". > Otherwise, patch is good. Ok. I will send the fixed version in a separate mail. > Yep. However, I don't agree with Hannes in this case. Given that the > firmware is already doing funky things to share the real adapter, I > believe it (or the driver) should do the right thing in the LUN > presentation to keep the system happy. But - I guess that's something > for a different time. Yes, i think it is better to fix the problems when they occur. Adapter sharing in general works well. Christof - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] FC transport: Disable LUN scanning from low level driver
James,
thanks for the feedback.
On Mon, Aug 20, 2007 at 09:07:23PM -0400, James Smart wrote:
> I'd prefer a flag/bit in the fc template to indicate no scanning by the
> transport. Thus, it's up to the driver to call __scsi_add_device, or
> perform the appropriate scans...
I attached a new patch that adds a flag to the fc template. So far, it
is untested. What is the better approach? Avoid scanning with the flag
being 1 or use the 1 to enable scaning and make this change in all FC
drivers?
> Note: this doesn't stop the problem in absolute. as long as there's scan
> interfaces via sysfs, any tool/admin could request a scan and hit your
> issues. To truly solve it, you need, within the LLDD, to munge what the
> midlayer sees for LUN lists.
We had the discussion about zfcp inventing LUNs in the past
(http://www.spinics.net/lists/linux-scsi/msg17125.html).
Disabling the scanning in the FC transport class is the most important
step at the moment, since the admin can only prevent the warning
messages by changing the LUN 0 to something else on the storage
system. I think for the user requested scan, we can now advice the
user not to issue it for zfcp, if he does anyway he will simply get
the warning message.
Christof Schmitt
---
drivers/scsi/scsi_transport_fc.c |4 +++-
include/scsi/scsi_transport_fc.h |3 +++
2 files changed, 6 insertions(+), 1 deletion(-)
--- linux-2.6.orig/drivers/scsi/scsi_transport_fc.c
+++ linux-2.6/drivers/scsi/scsi_transport_fc.c
@@ -2988,10 +2988,12 @@ fc_scsi_scan_rport(struct work_struct *w
struct fc_rport *rport =
container_of(work, struct fc_rport, scan_work);
struct Scsi_Host *shost = rport_to_shost(rport);
+ struct fc_internal *i = to_fc_internal(shost->transportt);
unsigned long flags;
if ((rport->port_state == FC_PORTSTATE_ONLINE) &&
- (rport->roles & FC_PORT_ROLE_FCP_TARGET)) {
+ (rport->roles & FC_PORT_ROLE_FCP_TARGET) &&
+ !(i->f->no_target_scan)) {
scsi_scan_target(&rport->dev, rport->channel,
rport->scsi_target_id, SCAN_WILD_CARD, 1);
}
--- linux-2.6.orig/include/scsi/scsi_transport_fc.h
+++ linux-2.6/include/scsi/scsi_transport_fc.h
@@ -632,6 +632,9 @@ struct fc_function_template {
unsigned long show_host_fabric_name:1;
unsigned long show_host_symbolic_name:1;
unsigned long show_host_system_hostname:1;
+
+ /* The driver can disable the LUN scanning from the FC transport class
*/
+ unsigned long no_target_scan:1;
};
---
drivers/s390/scsi/zfcp_scsi.c |1 +
1 file changed, 1 insertion(+)
--- linux-2.6.orig/drivers/s390/scsi/zfcp_scsi.c
+++ linux-2.6/drivers/s390/scsi/zfcp_scsi.c
@@ -800,6 +800,7 @@ struct fc_function_template zfcp_transpo
.show_host_port_type = 1,
.show_host_speed = 1,
.show_host_port_id = 1,
+ .no_target_scan = 1,
};
/**
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] FC transport: Disable LUN scanning from low level driver
The FC transport class calls scsi_scan_target with the SCAN_WILD_CARD
flag to automatically scan for logical units. zfcp, on the other hand,
only uses the units that are configured via the zfcp sysfs interface.
The main reason for this, is that the adapter behind zfcp supports
adapter sharing without NPIV: The adapter is logged into the SAN once,
and each unit can be used by one Linux system. If one Linux would grab
all LUNs, no other one can use them.
If a unit has the LUN 0, then the SCSI midlayer issue a REPORT LUNS,
checks the found LUN against the max_lun of the hostadapter. zfcp sets
this to 1 to only use its own managed units. If there is a LUN 0 that
is used by zfcp, then the SCSI midlayer produces lots of messages like
"scsi: host 0 channel 0 id 1 lun2 has a LUN larger than allowed by the
host adapter" to indicate the mismatch between the actual LUN and the
max_lun setting. These messages only confuse a user in the case of
zfcp, since there is no error.
To fix this problem, the LLD (zfcp) has to be able to prevent the
automatic scanning from the FC transport class. Attached is a patch
that adds a parameter to fc_remote_port_add(), another approach would
be an additional flag in the FC transport template.
What do others think? If there is an agreement, i will followup with a
new patch.
This can go away, as soon as zfcp does not have to support the
non-NPIV adapter sharing anymore, but this won't happen in the
foreseeable future.
Christof Schmitt
--- linux-2.6.orig/drivers/s390/scsi/zfcp_erp.c 2007-08-14 17:00:07.0
+0200
+++ linux-2.6/drivers/s390/scsi/zfcp_erp.c 2007-08-20 12:54:03.0
+0200
@@ -3175,7 +3175,7 @@ zfcp_erp_action_cleanup(int action, stru
ids.port_id = port->d_id;
ids.roles = FC_RPORT_ROLE_FCP_TARGET;
port->rport =
- fc_remote_port_add(adapter->scsi_host, 0, &ids);
+ fc_remote_port_add(adapter->scsi_host, 0, &ids,
0);
if (!port->rport)
ZFCP_LOG_NORMAL("failed registration of rport"
"(adapter %s, wwpn=0x%016Lx)\n",
--- linux-2.6.orig/drivers/scsi/scsi_transport_fc.c 2007-08-14
17:00:07.0 +0200
+++ linux-2.6/drivers/scsi/scsi_transport_fc.c 2007-08-20 12:56:15.0
+0200
@@ -2360,7 +2360,7 @@ fc_rport_final_delete(struct work_struct
**/
static struct fc_rport *
fc_rport_create(struct Scsi_Host *shost, int channel,
- struct fc_rport_identifiers *ids)
+ struct fc_rport_identifiers *ids, int scan_target)
{
struct fc_host_attrs *fc_host = shost_to_fc_host(shost);
struct fc_internal *fci = to_fc_internal(shost->transportt);
@@ -2424,6 +2424,9 @@ fc_rport_create(struct Scsi_Host *shost,
transport_add_device(dev);
transport_configure_device(dev);
+ if (scan_target)
+ rport->flags |= FC_RPORT_SCAN_TARGET;
+
if (rport->roles & FC_PORT_ROLE_FCP_TARGET) {
/* initiate a scan of the target */
rport->flags |= FC_RPORT_SCAN_PENDING;
@@ -2484,7 +2487,7 @@ delete_rport:
**/
struct fc_rport *
fc_remote_port_add(struct Scsi_Host *shost, int channel,
- struct fc_rport_identifiers *ids)
+ struct fc_rport_identifiers *ids, int scan_target)
{
struct fc_internal *fci = to_fc_internal(shost->transportt);
struct fc_host_attrs *fc_host = shost_to_fc_host(shost);
@@ -2574,6 +2577,10 @@ fc_remote_port_add(struct Scsi_Host *sho
spin_lock_irqsave(shost->host_lock, flags);
rport->flags &= ~FC_RPORT_DEVLOSS_PENDING;
+ if (scan_target)
+ rport->flags |= FC_RPORT_SCAN_TARGET;
+ else
+ rport->flags &= ~FC_RPORT_SCAN_TARGET;
/* if target, initiate a scan */
if (rport->scsi_target_id != -1) {
@@ -2657,7 +2664,7 @@ fc_remote_port_add(struct Scsi_Host *sho
spin_unlock_irqrestore(shost->host_lock, flags);
/* No consistent binding found - create new remote port entry */
- rport = fc_rport_create(shost, channel, ids);
+ rport = fc_rport_create(shost, channel, ids, scan_target);
return rport;
}
@@ -2991,7 +2998,8 @@ fc_scsi_scan_rport(struct work_struct *w
unsigned long flags;
if ((rport->port_state == FC_PORTSTATE_ONLINE) &&
- (rport->roles & FC_PORT_ROLE_FCP_TARGET)) {
+ (rport->roles & FC_PORT_ROLE_FCP_TARGET) &&
+ (rport->flags & FC_RPORT_SCAN_TARGET)) {
scsi_scan_target(&rport->dev, rpo
Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer
On Thu, Jun 21, 2007 at 09:40:35PM -0700, Mike Anderson wrote: > James Bottomley <[EMAIL PROTECTED]> wrote: > > A proposal to display the correct form of the LUN would be useful if you > > wish to make it? ... The problem is really that SAM specifies a > > possible 4 level structure with 4 possible address methods per level. > > > > The well known LUNs should be simple; there are only three: Report Lun, > > Access Controls and Target Log Pages. The rest we really need input on. > > For instance, I could see the vendors wishing us to combine a > > multi-level flat addressing space into a single logical unit number, > > whereas I could see them wanting us to supply some sort of hierarchy for > > the peripheral and logical unit methods of addressing. > > > > Since you're already using 2 level flat addressing, how do you want to > > see that represented? > > James, why would we would not want to display the lun per SAM4 4.6.2 as > suggested by Stefan in a previous comment. I would also agree to display the LUN in sysfs as 64 bit hex number as advised in SAM4. While the SAM describes different formats and addressing schemes, i don't see how this affects the SCSI layer in the kernel. The kernel gets the 64 bit LUNs and uses them as unique ids. 4.6.1 in SAM4 says in the last sentence, that different values for the 64 bits represent different LUNs, so the assumption of the LUNs being unique is guaranteed. My point of view is mainly from the device driver's (zfcp) and user's point of view. It is only that the disk systems i deal with use the 2 level addressing in the LUNs. The user interface of the disk system displays something like: Volume F000 is exported as FCP LUN 40F04000. When the SCSI midlayer uses and exports this value in sysfs, it should be displayed in the same format (hex). I don't know if the multi level scheme is actually used as hierarchy somewhere. If somebody would be interested in using this information, i think a userspace tool could grab the value from sysfs and display the detailed information according to SAM. -- Christof Schmitt - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer
Now that everybody is happy with using the FCP LUN in the SCSI midlayer, this raises a question from the user's perspective: sysfs and lsscsi display the LUN as decimal number. For the FCP LUN 0x401040c3, sysfs and lsscsi display this: $ ls -l /sys/bus/scsi/devices/ total 0 lrwxrwxrwx 1 root root 0 Jun 21 16:49 0:0:0:1086537744 -> ../../../devices/css0/0.0.0010/0.0.181d/host0/rport-0:0-0/target0:0:0/0:0:0:1086537744 $ lsscsi [0:0:0:1086537744]diskIBM 2107900 .270 /dev/sda Asking a system admin to translate the number to hex (0x40C34010) and then swap the pairs to get the real FCP LUN (0x401040c3) is too much. Are there any plans to improve this? Showing the LUN in sysfs as hex number would be the first step. Or does it make more sense to keep the decimal number in sysfs and let tools like lsscsi do the conversion? -- Christof Schmitt - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
blktrace for all SCSI devices
As a follow-up to earlier discussions about adding statistics to the SCSI layer (http://lkml.org/lkml/2006/9/26/151), i am looking what data blktrace can provide. One problem is that blktrace currently can only get the trace data from block device files. This excludes retrieving trace data from other SCSI devices, especially from tape drives. Looking at the code, the SCSI layer internally uses the same request_queue per device. It looks like the blktrace ioctls as interface to userspace are the first thing that is missing. Is this all? What would be a going idea to make the blktrace ioctls available for all SCSI devices? My first approach would be to add them to the sg devices. Any comments? If there are now objections, i will try to implement the ioctls for sg devices, to see if this actually works. -- Christof Schmitt - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] FC Transport support for vports based on NPIV
On Mon, May 21, 2007 at 11:45:13AM -0400, James Smart wrote: > True - so this means that who-ever is setting the subchannel device > permanent_port_name value needs to take into account this conversation, > when T11.3 actually makes a choice on what it should be. I will keep this in mind. Probably this would involve the FCP adapter and the reporting in zfcp. Anyway, my main goal was to understand the changes in the FC transport class and the possible impact to zfcp. Thanks for the clarification of my questions. Christof - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] FC Transport support for vports based on NPIV
On Mon, May 14, 2007 at 11:56:38AM -0400, James Smart wrote: > All true. But, there is the notion that there is a driver that thinks > it's controlling the adapter, but it's actually talking to a virtual > thing, that traps the driver's FLOGI's and turns it into FDISCs... > > >With zfcp, the hardware FCP adapter does NPIV and only hands out the > >virtual address to individual Linux instances. zfcp gets the assigned > >virtual address for the Linux instance. This address is used for > >allocating the scsi_host structure. Basically, the whole system uses > >NPIV, but each Linux only uses one assigned virtual WWPN. > > Yes - I understand. For all intents and purposes, that virtual address > is treated as an adapter. > > >My current understanding is that the vports introduced in the fc > >transport class do not affect the Linux systems that only use one > >virtual address. To map this to Xen, the dom0 would use the vports > >to show all virtual address, and each domU would use the assigned > >virtual address without showing the vport in sysfs. Is this correct? > > > >Christof > > Agreed. It should mean nothing to the zfcp driver. Nothing changes in > the driver if it will always be a single address. > > Although - if your hardware-to-driver interface had the ability to > instantiate a new address, you could augment your driver to support > the vport calls. That's a choice for you though. Thanks for your input. I looked at the interface between the FCP hardware adapter and zfcp again. The Linux guest only sees a subchannel device. This subchannel device is already a virtualized interface. The FCP adapter assigns a unique WWPN to each subchannel device (which is done by NPIV). zfcp only sees the subchannel device and knows that this device has only one WWPN. So this "virtual" device is used as SCSI/FC adapter in Linux and will always have one single address. So, i think reporting this subchannel device as a simple HBA without vports makes sense, since the virtualization part is handled by the FCP adapter, and not zfcp in Linux. > PS: One thing I didn't call out in the vport patches was the > expectation that the vport-supporting driver had to set the PPN > attribute appropriately. And... did you see that T11 is trying to > change what the PPN is set to - the fabric port_name not the physical > N_Port port_name. No, i did not follow the T11 change. Do you have a link to a draft document or something similar? zfcp uses the FC tranport class attributes port_name and permanent_port_name. PPN is the fixed WWPN of the physical adapter, while PN is the one virtual one used by each Linux guest and is assigned via NPIV. Both WWPNs are passed back via the "virtual" device, so i guess this would be only a matter of reporting the correct one in zfcp. Christof - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] FC Transport support for vports based on NPIV
James, i try to understand what the introduction of the vports means for zfcp, since this driver also supports NPIV. The documentation for the fc transport class describes a driver that would fully control the adapter and the creation of virtual address. Since you mentioned Xen, i assume that this could be a dom0. With zfcp, the hardware FCP adapter does NPIV and only hands out the virtual address to individual Linux instances. zfcp gets the assigned virtual address for the Linux instance. This address is used for allocating the scsi_host structure. Basically, the whole system uses NPIV, but each Linux only uses one assigned virtual WWPN. My current understanding is that the vports introduced in the fc transport class do not affect the Linux systems that only use one virtual address. To map this to Xen, the dom0 would use the vports to show all virtual address, and each domU would use the assigned virtual address without showing the vport in sysfs. Is this correct? Christof - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] zfcp: Fix deadlock between zfcp ERP and SCSI
On Tue, May 08, 2007 at 11:00:11AM -0400, James Smart wrote: > Curious why you are calling scsi_scan_target() or > scsi_target_block()/scsi_target_unblock() directly. I would have > thought the add/remove rport code would have done this for you, > and it deals with all the flush conditions, etc. fc_remote_port_add issues a scsi_scan_target with SCAN_WILD_CARD. This tries to get all LUNs for the target via REPORT LUNS for LUN 0. The current design for zfcp is to prevent the wildcard scan from the SCSI layer by not allocating a device for LUN 0. When a FCP adapter does not support NPIV, the adapter can still be shared among multiple Linux systems. Each Linux system can access different LUNs, that are activated manually via sysfs. So, the wildcard scan from the FC transport class does nothing and zfcp activates the LUNs it wants to use via direct calls to scsi_scan_target. If every adapter would support NPIV, zfcp could rely on the scan by the FC transport class, unfortunately, this is not the case. Christof Schmitt - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] zfcp: Stop system after memory corruption
On Tue, May 08, 2007 at 01:56:07AM +1000, Julian Calaby wrote:
> >From: Christof Schmitt <[EMAIL PROTECTED]>
> [snip]
> >- ZFCP_LOG_NORMAL("bug: unexpected inbound "
> >- "packet on adapter %s "
> >- "(reqid=0x%lx, "
> >- "first_element=%d, "
> >- "elements_processed=%d)\n",
[...]
> As a minor quibble, may I suggest that these lines belong just above
> the panic introduced above? I assume that they'd be useful for
> debugging should this situation occur.
If this problem occurs, the administrator of the Linux system will be
advised to dump the Linux system and to also capture the logs from the
FCP adapter. The memory dump already contains the information about the
unexpected packet and also information about the last requests
sent to the FCP adapter. So, the messages do not provide additional
debug data, and i removed them to simplify the code.
Christof Schmitt
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] zfcp: fix initialization of FSF timer
From: Christof Schmitt <[EMAIL PROTECTED]>
Correctly initialize the timer for FSF requests with jiffies + timeout.
Cc: Swen Schillig <[EMAIL PROTECTED]>
Acked-by: Heiko Carstens <[EMAIL PROTECTED]>
Signed-off-by: Christof Schmitt <[EMAIL PROTECTED]>
---
Please consider this patch for 2.6.21.
drivers/s390/scsi/zfcp_erp.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux/drivers/s390/scsi/zfcp_erp.c.orig 2007-03-07 09:53:22.0
+0100
+++ linux/drivers/s390/scsi/zfcp_erp.c 2007-03-07 09:53:35.0 +0100
@@ -186,7 +186,7 @@
{
fsf_req->timer.function = zfcp_fsf_request_timeout_handler;
fsf_req->timer.data = (unsigned long) fsf_req->adapter;
- fsf_req->timer.expires = timeout;
+ fsf_req->timer.expires = jiffies + timeout;
add_timer(&fsf_req->timer);
}
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
