public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [patch 0/8] zfcp fixes for 2.6.28
@ 2008-11-04 15:35 Christof Schmitt
  2008-11-04 15:35 ` [patch 1/8] zfcp: Dont clear reference from SCSI device to unit Christof Schmitt
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: Christof Schmitt @ 2008-11-04 15:35 UTC (permalink / raw)
  To: James Bottomley; +Cc: linux-scsi, linux-s390, schwidefsky, heiko.carstens

James,

here is the series of zfcp fixes for 2.6.28. The patches apply on top
of the current linux-2.6 git tree.

Christof

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [patch 1/8] zfcp: Dont clear reference from SCSI device to unit
  2008-11-04 15:35 [patch 0/8] zfcp fixes for 2.6.28 Christof Schmitt
@ 2008-11-04 15:35 ` Christof Schmitt
  2008-11-04 15:35 ` [patch 2/8] zfcp: fix req_list_locking Christof Schmitt
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Christof Schmitt @ 2008-11-04 15:35 UTC (permalink / raw)
  To: James Bottomley
  Cc: linux-scsi, linux-s390, schwidefsky, heiko.carstens,
	Christof Schmitt, Swen Schillig

[-- Attachment #1: fix-abort-after-port-off-on.diff --]
[-- Type: text/plain, Size: 1365 bytes --]

From: Christof Schmitt <christof.schmitt@de.ibm.com>

It is possible that a remote port has a problem, the SCSI device gets
deleted after the rport timeout and then the timeout for pending SCSI
commands trigger an abort. For this case, don't delete the reference
from the SCSI device to the zfcp unit, so that we can still have the
reference to issue an abort request.

Signed-off-by: Christof Schmitt <christof.schmitt@de.ibm.com>
Signed-off-by: Swen Schillig <swen@vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_scsi.c |   12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

--- a/drivers/s390/scsi/zfcp_scsi.c	2008-11-04 14:44:04.000000000 +0100
+++ b/drivers/s390/scsi/zfcp_scsi.c	2008-11-04 14:44:11.000000000 +0100
@@ -24,14 +24,10 @@ char *zfcp_get_fcp_sns_info_ptr(struct f
 static void zfcp_scsi_slave_destroy(struct scsi_device *sdpnt)
 {
 	struct zfcp_unit *unit = (struct zfcp_unit *) sdpnt->hostdata;
-	WARN_ON(!unit);
-	if (unit) {
-		atomic_clear_mask(ZFCP_STATUS_UNIT_REGISTERED, &unit->status);
-		sdpnt->hostdata = NULL;
-		unit->device = NULL;
-		zfcp_erp_unit_failed(unit, 12, NULL);
-		zfcp_unit_put(unit);
-	}
+	atomic_clear_mask(ZFCP_STATUS_UNIT_REGISTERED, &unit->status);
+	unit->device = NULL;
+	zfcp_erp_unit_failed(unit, 12, NULL);
+	zfcp_unit_put(unit);
 }
 
 static int zfcp_scsi_slave_configure(struct scsi_device *sdp)

-- 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [patch 2/8] zfcp: fix req_list_locking.
  2008-11-04 15:35 [patch 0/8] zfcp fixes for 2.6.28 Christof Schmitt
  2008-11-04 15:35 ` [patch 1/8] zfcp: Dont clear reference from SCSI device to unit Christof Schmitt
@ 2008-11-04 15:35 ` Christof Schmitt
  2008-11-04 15:35 ` [patch 3/8] zfcp: fix mempool usage for status_read requests Christof Schmitt
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Christof Schmitt @ 2008-11-04 15:35 UTC (permalink / raw)
  To: James Bottomley
  Cc: linux-scsi, linux-s390, schwidefsky, heiko.carstens,
	Christof Schmitt

[-- Attachment #1: 718-zfcp-lockdep.diff --]
[-- Type: text/plain, Size: 4201 bytes --]

From: Heiko Carstens <heiko.carstens@de.ibm.com>

The per adapter req_list_lock must be held with interrupts disabled, otherwise
we might end up with nice deadlocks as lockdep tells us (see below).

zfcp 0.0.1804: QDIO problem occurred.

=========================================================
[ INFO: possible irq lock inversion dependency detected ]
2.6.27-rc8-00035-g4a77035-dirty #86
---------------------------------------------------------
swapper/0 just changed the state of lock:
 (&adapter->erp_lock){++..}, at: [<00000000002c82ae>] zfcp_erp_adapter_reopen+0x4e/0x8c
but this lock took another, hard-irq-unsafe lock in the past:
 (&adapter->req_list_lock){-+..}

and interrupts could create inverse lock ordering between them.

[tons of backtraces, but only the interesting part follows]

the second lock's dependencies:
-> (&adapter->req_list_lock){-+..} ops: 2280627634176 {
   initial-use  at:
                        [<0000000000071f10>] __lock_acquire+0x504/0x18bc
                        [<000000000007335c>] lock_acquire+0x94/0xbc
                        [<00000000003d7224>] _spin_lock_irqsave+0x6c/0xb0
                        [<00000000002cf684>] zfcp_fsf_req_dismiss_all+0x50/0x140
                        [<00000000002c87ee>] zfcp_erp_adapter_strategy_generic+0x66/0x3d0
                        [<00000000002c9498>] zfcp_erp_thread+0x88c/0x1318
                        [<000000000001b0d2>] kernel_thread_starter+0x6/0xc
                        [<000000000001b0cc>] kernel_thread_starter+0x0/0xc
   in-softirq-W at:
                        [<0000000000072172>] __lock_acquire+0x766/0x18bc
                        [<000000000007335c>] lock_acquire+0x94/0xbc
                        [<00000000003d7224>] _spin_lock_irqsave+0x6c/0xb0
                        [<00000000002ca73e>] zfcp_qdio_int_resp+0xbe/0x2ac
                        [<000000000027a1d6>] qdio_kick_inbound_handler+0x82/0xa0
                        [<000000000027daba>] tiqdio_inbound_processing+0x62/0xf8
                        [<0000000000047ba4>] tasklet_action+0x100/0x1f4
                        [<0000000000048b5a>] __do_softirq+0xae/0x154
                        [<0000000000021e4a>] do_softirq+0xea/0xf0
                        [<00000000000485de>] irq_exit+0xde/0xe8
                        [<0000000000268c64>] do_IRQ+0x160/0x1fc
                        [<00000000000261a2>] io_return+0x0/0x8
                        [<000000000001b8f8>] cpu_idle+0x17c/0x224
   hardirq-on-W at:
                        [<0000000000072190>] __lock_acquire+0x784/0x18bc
                        [<000000000007335c>] lock_acquire+0x94/0xbc
                        [<00000000003d702c>] _spin_lock+0x5c/0x9c
                        [<00000000002caff6>] zfcp_fsf_req_send+0x3e/0x158
                        [<00000000002ce7fe>] zfcp_fsf_exchange_config_data+0x106/0x124
                        [<00000000002c8948>] zfcp_erp_adapter_strategy_generic+0x1c0/0x3d0
                        [<00000000002c98ea>] zfcp_erp_thread+0xcde/0x1318
                        [<000000000001b0d2>] kernel_thread_starter+0x6/0xc
                        [<000000000001b0cc>] kernel_thread_starter+0x0/0xc
 }
 ... key      at: [<0000000000e356c8>] __key.26629+0x0/0x8

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Christof Schmitt <christof.schmit@de.ibm.com>
---

 drivers/s390/scsi/zfcp_fsf.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

--- a/drivers/s390/scsi/zfcp_fsf.c	2008-11-04 14:44:04.000000000 +0100
+++ b/drivers/s390/scsi/zfcp_fsf.c	2008-11-04 14:44:17.000000000 +0100
@@ -770,13 +770,14 @@ static int zfcp_fsf_req_send(struct zfcp
 {
 	struct zfcp_adapter *adapter = req->adapter;
 	struct zfcp_qdio_queue *req_q = &adapter->req_q;
+	unsigned long flags;
 	int idx;
 
 	/* put allocated FSF request into hash table */
-	spin_lock(&adapter->req_list_lock);
+	spin_lock_irqsave(&adapter->req_list_lock, flags);
 	idx = zfcp_reqlist_hash(req->req_id);
 	list_add_tail(&req->list, &adapter->req_list[idx]);
-	spin_unlock(&adapter->req_list_lock);
+	spin_unlock_irqrestore(&adapter->req_list_lock, flags);
 
 	req->qdio_outb_usage = atomic_read(&req_q->count);
 	req->issued = get_clock();

-- 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [patch 3/8] zfcp: fix mempool usage for status_read requests
  2008-11-04 15:35 [patch 0/8] zfcp fixes for 2.6.28 Christof Schmitt
  2008-11-04 15:35 ` [patch 1/8] zfcp: Dont clear reference from SCSI device to unit Christof Schmitt
  2008-11-04 15:35 ` [patch 2/8] zfcp: fix req_list_locking Christof Schmitt
@ 2008-11-04 15:35 ` Christof Schmitt
  2008-11-05 17:46   ` James Bottomley
  2008-11-04 15:35 ` [patch 4/8] zfcp: Fix request list handling in error path Christof Schmitt
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 11+ messages in thread
From: Christof Schmitt @ 2008-11-04 15:35 UTC (permalink / raw)
  To: James Bottomley
  Cc: linux-scsi, linux-s390, schwidefsky, heiko.carstens,
	Christof Schmitt, Swen Schillig

[-- Attachment #1: fix-status-read-alloc.diff --]
[-- Type: text/plain, Size: 733 bytes --]

From: Christof Schmitt <christof.schmitt@de.ibm.com>

When allocating fsf requests without qtcb, store the pointer to the
mempool in the fsf requests for later call to mempool_free. This
codepath is only used by the status_read requests.

Signed-off-by: Christof Schmitt <christof.schmitt@de.ibm.com>
Signed-off-by: Swen Schillig <swen@vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_fsf.c |    1 +
 1 file changed, 1 insertion(+)

--- a/drivers/s390/scsi/zfcp_fsf.c	2008-11-04 14:44:17.000000000 +0100
+++ b/drivers/s390/scsi/zfcp_fsf.c	2008-11-04 14:44:19.000000000 +0100
@@ -683,6 +683,7 @@ static struct zfcp_fsf_req *zfcp_fsf_all
 	if (!req)
 		return NULL;
 	memset(req, 0, sizeof(*req));
+	req->pool = pool;
 	return req;
 }
 

-- 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [patch 4/8] zfcp: Fix request list handling in error path
  2008-11-04 15:35 [patch 0/8] zfcp fixes for 2.6.28 Christof Schmitt
                   ` (2 preceding siblings ...)
  2008-11-04 15:35 ` [patch 3/8] zfcp: fix mempool usage for status_read requests Christof Schmitt
@ 2008-11-04 15:35 ` Christof Schmitt
  2008-11-04 15:35 ` [patch 5/8] zfcp: Fix cast warning Christof Schmitt
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Christof Schmitt @ 2008-11-04 15:35 UTC (permalink / raw)
  To: James Bottomley
  Cc: linux-scsi, linux-s390, schwidefsky, heiko.carstens,
	Christof Schmitt, Swen Schillig

[-- Attachment #1: fsf-error.diff --]
[-- Type: text/plain, Size: 2120 bytes --]

From: Christof Schmitt <christof.schmitt@de.ibm.com>

Fix the handling of the request list in the error path:
 - Use irqsave for the lock as in the good path.
 - Before removing the request, check if it is still in the list, a
   call to dismiss_all might have changed the list in between.
 - zfcp_qdio_send does not change the queue counters on failure,
   trying revert something is wrong, so remove this.

Signed-off-by: Christof Schmitt <christof.schmitt@de.ibm.com>
Signed-off-by: Swen Schillig <swen@vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_fsf.c |   17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

--- a/drivers/s390/scsi/zfcp_fsf.c	2008-11-04 14:44:19.000000000 +0100
+++ b/drivers/s390/scsi/zfcp_fsf.c	2008-11-04 14:44:22.000000000 +0100
@@ -770,7 +770,6 @@ static struct zfcp_fsf_req *zfcp_fsf_req
 static int zfcp_fsf_req_send(struct zfcp_fsf_req *req)
 {
 	struct zfcp_adapter *adapter = req->adapter;
-	struct zfcp_qdio_queue *req_q = &adapter->req_q;
 	unsigned long flags;
 	int idx;
 
@@ -780,19 +779,15 @@ static int zfcp_fsf_req_send(struct zfcp
 	list_add_tail(&req->list, &adapter->req_list[idx]);
 	spin_unlock_irqrestore(&adapter->req_list_lock, flags);
 
-	req->qdio_outb_usage = atomic_read(&req_q->count);
+	req->qdio_outb_usage = atomic_read(&adapter->req_q.count);
 	req->issued = get_clock();
 	if (zfcp_qdio_send(req)) {
-		/* Queues are down..... */
 		del_timer(&req->timer);
-		spin_lock(&adapter->req_list_lock);
-		zfcp_reqlist_remove(adapter, req);
-		spin_unlock(&adapter->req_list_lock);
-		/* undo changes in request queue made for this request */
-		atomic_add(req->sbal_number, &req_q->count);
-		req_q->first -= req->sbal_number;
-		req_q->first += QDIO_MAX_BUFFERS_PER_Q;
-		req_q->first %= QDIO_MAX_BUFFERS_PER_Q; /* wrap */
+		spin_lock_irqsave(&adapter->req_list_lock, flags);
+		/* lookup request again, list might have changed */
+		if (zfcp_reqlist_find_safe(adapter, req))
+			zfcp_reqlist_remove(adapter, req);
+		spin_unlock_irqrestore(&adapter->req_list_lock, flags);
 		zfcp_erp_adapter_reopen(adapter, 0, 116, req);
 		return -EIO;
 	}

-- 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [patch 5/8] zfcp: Fix cast warning
  2008-11-04 15:35 [patch 0/8] zfcp fixes for 2.6.28 Christof Schmitt
                   ` (3 preceding siblings ...)
  2008-11-04 15:35 ` [patch 4/8] zfcp: Fix request list handling in error path Christof Schmitt
@ 2008-11-04 15:35 ` Christof Schmitt
  2008-11-04 15:35 ` [patch 6/8] zfcp: Wait for port scan to complete when setting adapter online Christof Schmitt
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Christof Schmitt @ 2008-11-04 15:35 UTC (permalink / raw)
  To: James Bottomley
  Cc: linux-scsi, linux-s390, schwidefsky, heiko.carstens,
	Christof Schmitt

[-- Attachment #1: fix-cast.diff --]
[-- Type: TEXT/PLAIN, Size: 1253 bytes --]

From: Christof Schmitt <christof.schmitt@de.ibm.com>

Fix leftover from last typecast patch:

drivers/s390/scsi/zfcp_aux.c: In function ‘zfcp_port_enqueue’:
drivers/s390/scsi/zfcp_aux.c:629: warning: format ‘%016llx’ expects
type ‘long long unsigned int’, but argument 3 has type ‘u64’

Signed-off-by: Christof Schmitt <christof.schmitt@de.ibm.com>
---
 drivers/s390/scsi/zfcp_aux.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/drivers/s390/scsi/zfcp_aux.c	2008-11-04 14:44:04.000000000 +0100
+++ b/drivers/s390/scsi/zfcp_aux.c	2008-11-04 14:44:24.000000000 +0100
@@ -610,7 +610,8 @@ struct zfcp_port *zfcp_port_enqueue(stru
 	atomic_set_mask(status | ZFCP_STATUS_COMMON_REMOVE, &port->status);
 	atomic_set(&port->refcount, 0);
 
-	dev_set_name(&port->sysfs_device, "0x%016llx", wwpn);
+	dev_set_name(&port->sysfs_device, "0x%016llx",
+		     (unsigned long long)wwpn);
 	port->sysfs_device.parent = &adapter->ccw_device->dev;
 
 	port->sysfs_device.release = zfcp_sysfs_port_release;

-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [patch 6/8] zfcp: Wait for port scan to complete when setting adapter online
  2008-11-04 15:35 [patch 0/8] zfcp fixes for 2.6.28 Christof Schmitt
                   ` (4 preceding siblings ...)
  2008-11-04 15:35 ` [patch 5/8] zfcp: Fix cast warning Christof Schmitt
@ 2008-11-04 15:35 ` Christof Schmitt
  2008-11-04 15:35 ` [patch 7/8] zfcp: fix erp timeout cleanup for port open requests Christof Schmitt
  2008-11-04 15:35 ` [patch 8/8] zfcp: Fix hexdump data in s390dbf traces Christof Schmitt
  7 siblings, 0 replies; 11+ messages in thread
From: Christof Schmitt @ 2008-11-04 15:35 UTC (permalink / raw)
  To: James Bottomley
  Cc: linux-scsi, linux-s390, schwidefsky, heiko.carstens,
	Christof Schmitt, Swen Schillig

[-- Attachment #1: scan-wait-online.diff --]
[-- Type: text/plain, Size: 1037 bytes --]

From: Christof Schmitt <christof.schmitt@de.ibm.com>

Attaching a unit immediately after setting the adapter online should
be possible. The problem right now is that the port_scan runs from a
workqueue and has not finished when the set_online call returns and
the sysfs structures for the ports are not available yet. Fix that by
waiting for the port scan to complete.

Signed-off-by: Christof Schmitt <christof.schmitt@de.ibm.com>
Signed-off-by: Swen Schillig <swen@vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_ccw.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

--- a/drivers/s390/scsi/zfcp_ccw.c	2008-11-04 14:44:04.000000000 +0100
+++ b/drivers/s390/scsi/zfcp_ccw.c	2008-11-04 14:44:27.000000000 +0100
@@ -116,7 +116,9 @@ static int zfcp_ccw_set_online(struct cc
 	zfcp_erp_adapter_reopen(adapter, ZFCP_STATUS_COMMON_ERP_FAILED, 85,
 				NULL);
 	zfcp_erp_wait(adapter);
-	goto out;
+	up(&zfcp_data.config_sema);
+	flush_work(&adapter->scan_work);
+	return 0;
 
  out_scsi_register:
 	zfcp_erp_thread_kill(adapter);

-- 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [patch 7/8] zfcp: fix erp timeout cleanup for port open requests
  2008-11-04 15:35 [patch 0/8] zfcp fixes for 2.6.28 Christof Schmitt
                   ` (5 preceding siblings ...)
  2008-11-04 15:35 ` [patch 6/8] zfcp: Wait for port scan to complete when setting adapter online Christof Schmitt
@ 2008-11-04 15:35 ` Christof Schmitt
  2008-11-04 15:35 ` [patch 8/8] zfcp: Fix hexdump data in s390dbf traces Christof Schmitt
  7 siblings, 0 replies; 11+ messages in thread
From: Christof Schmitt @ 2008-11-04 15:35 UTC (permalink / raw)
  To: James Bottomley
  Cc: linux-scsi, linux-s390, schwidefsky, heiko.carstens,
	Martin Petermann, Christof Schmitt

[-- Attachment #1: 708-zfcp-erp-timeout.diff --]
[-- Type: text/plain, Size: 996 bytes --]

From: Martin Petermann <martin.petermann@de.ibm.com>

If an open port fsf request times out (in erp) the 
corresponding erp_action member of the fsf 
request need to set to NULL. If the port structure
will be removed later-on there will be still a 
reference in the fsf request to the non existing
erp_action otherwise.

Signed-off-by: Martin Petermann <martin.petermann@de.ibm.com>
Signed-off-by: Christof Schmitt <christof.schmitt@de.ibm.com>
---

 drivers/s390/scsi/zfcp_erp.c |    1 +
 1 file changed, 1 insertion(+)

--- a/drivers/s390/scsi/zfcp_erp.c	2008-11-04 14:44:04.000000000 +0100
+++ b/drivers/s390/scsi/zfcp_erp.c	2008-11-04 14:44:29.000000000 +0100
@@ -472,6 +472,7 @@ static void zfcp_erp_strategy_check_fsfr
 				   ZFCP_STATUS_ERP_TIMEDOUT)) {
 			act->fsf_req->status |= ZFCP_STATUS_FSFREQ_DISMISSED;
 			zfcp_rec_dbf_event_action(142, act);
+			act->fsf_req->erp_action = NULL;
 		}
 		if (act->status & ZFCP_STATUS_ERP_TIMEDOUT)
 			zfcp_rec_dbf_event_action(143, act);

-- 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [patch 8/8] zfcp: Fix hexdump data in s390dbf traces
  2008-11-04 15:35 [patch 0/8] zfcp fixes for 2.6.28 Christof Schmitt
                   ` (6 preceding siblings ...)
  2008-11-04 15:35 ` [patch 7/8] zfcp: fix erp timeout cleanup for port open requests Christof Schmitt
@ 2008-11-04 15:35 ` Christof Schmitt
  7 siblings, 0 replies; 11+ messages in thread
From: Christof Schmitt @ 2008-11-04 15:35 UTC (permalink / raw)
  To: James Bottomley
  Cc: linux-scsi, linux-s390, schwidefsky, heiko.carstens,
	Christof Schmitt, Swen Schillig

[-- Attachment #1: debug-hexdump.diff --]
[-- Type: text/plain, Size: 7074 bytes --]

From: Christof Schmitt <christof.schmitt@de.ibm.com>

Fix multiple problems found in the hexdump data:
 - length calculation was wrong, traces were incomplete
 - FC payloads were dumped in different record than the output
   function tried to read
 - minor fixes in output
 - allow complete RSCN traces (up to 1024 bytes according to spec)

Signed-off-by: Christof Schmitt <christof.schmitt@de.ibm.com>
Signed-off-by: Swen Schillig <swen@vnet.ibm.com>
---
 drivers/s390/scsi/zfcp_dbf.c |   42 ++++++++++++++++--------------------------
 drivers/s390/scsi/zfcp_dbf.h |    8 ++------
 2 files changed, 18 insertions(+), 32 deletions(-)

--- a/drivers/s390/scsi/zfcp_dbf.c	2008-11-03 16:47:05.000000000 +0100
+++ b/drivers/s390/scsi/zfcp_dbf.c	2008-11-03 16:49:33.000000000 +0100
@@ -30,7 +30,7 @@ static void zfcp_dbf_hexdump(debug_info_
 		dump->offset = offset;
 		dump->size = min(from_len - offset, room);
 		memcpy(dump->data, from + offset, dump->size);
-		debug_event(dbf, level, dump, dump->size);
+		debug_event(dbf, level, dump, dump->size + sizeof(*dump));
 	}
 }
 
@@ -108,7 +108,7 @@ static int zfcp_dbf_view_header(debug_in
 			     t.tv_sec, t.tv_nsec);
 		zfcp_dbf_out(&p, "cpu", "%02i", entry->id.fields.cpuid);
 	} else	{
-		zfcp_dbf_outd(&p, NULL, dump->data, dump->size, dump->offset,
+		zfcp_dbf_outd(&p, "", dump->data, dump->size, dump->offset,
 			      dump->total_size);
 		if ((dump->offset + dump->size) == dump->total_size)
 			p += sprintf(p, "\n");
@@ -366,6 +366,7 @@ static void zfcp_hba_dbf_view_response(c
 			break;
 		zfcp_dbf_out(p, "scsi_cmnd", "0x%0Lx", r->u.fcp.cmnd);
 		zfcp_dbf_out(p, "scsi_serial", "0x%016Lx", r->u.fcp.serial);
+		p += sprintf(*p, "\n");
 		break;
 
 	case FSF_QTCB_OPEN_PORT_WITH_DID:
@@ -465,7 +466,8 @@ static int zfcp_hba_dbf_view_format(debu
 	else if (strncmp(r->tag, "berr", ZFCP_DBF_TAG_SIZE) == 0)
 		zfcp_hba_dbf_view_berr(&p, &r->u.berr);
 
-	p += sprintf(p, "\n");
+	if (strncmp(r->tag, "resp", ZFCP_DBF_TAG_SIZE) != 0)
+		p += sprintf(p, "\n");
 	return p - out_buf;
 }
 
@@ -880,6 +882,7 @@ void zfcp_san_dbf_event_ct_request(struc
 	struct ct_hdr *hdr = sg_virt(ct->req);
 	struct zfcp_san_dbf_record *r = &adapter->san_dbf_buf;
 	struct zfcp_san_dbf_record_ct_request *oct = &r->u.ct_req;
+	int level = 3;
 	unsigned long flags;
 
 	spin_lock_irqsave(&adapter->san_dbf_lock, flags);
@@ -896,9 +899,10 @@ void zfcp_san_dbf_event_ct_request(struc
 	oct->options = hdr->options;
 	oct->max_res_size = hdr->max_res_size;
 	oct->len = min((int)ct->req->length - (int)sizeof(struct ct_hdr),
-		       ZFCP_DBF_CT_PAYLOAD);
-	memcpy(oct->payload, (void *)hdr + sizeof(struct ct_hdr), oct->len);
-	debug_event(adapter->san_dbf, 3, r, sizeof(*r));
+		       ZFCP_DBF_SAN_MAX_PAYLOAD);
+	debug_event(adapter->san_dbf, level, r, sizeof(*r));
+	zfcp_dbf_hexdump(adapter->san_dbf, r, sizeof(*r), level,
+			 (void *)hdr + sizeof(struct ct_hdr), oct->len);
 	spin_unlock_irqrestore(&adapter->san_dbf_lock, flags);
 }
 
@@ -914,6 +918,7 @@ void zfcp_san_dbf_event_ct_response(stru
 	struct ct_hdr *hdr = sg_virt(ct->resp);
 	struct zfcp_san_dbf_record *r = &adapter->san_dbf_buf;
 	struct zfcp_san_dbf_record_ct_response *rct = &r->u.ct_resp;
+	int level = 3;
 	unsigned long flags;
 
 	spin_lock_irqsave(&adapter->san_dbf_lock, flags);
@@ -929,9 +934,10 @@ void zfcp_san_dbf_event_ct_response(stru
 	rct->expl = hdr->reason_code_expl;
 	rct->vendor_unique = hdr->vendor_unique;
 	rct->len = min((int)ct->resp->length - (int)sizeof(struct ct_hdr),
-		       ZFCP_DBF_CT_PAYLOAD);
-	memcpy(rct->payload, (void *)hdr + sizeof(struct ct_hdr), rct->len);
-	debug_event(adapter->san_dbf, 3, r, sizeof(*r));
+		       ZFCP_DBF_SAN_MAX_PAYLOAD);
+	debug_event(adapter->san_dbf, level, r, sizeof(*r));
+	zfcp_dbf_hexdump(adapter->san_dbf, r, sizeof(*r), level,
+			 (void *)hdr + sizeof(struct ct_hdr), rct->len);
 	spin_unlock_irqrestore(&adapter->san_dbf_lock, flags);
 }
 
@@ -954,7 +960,7 @@ static void zfcp_san_dbf_event_els(const
 	rec->u.els.ls_code = ls_code;
 	debug_event(adapter->san_dbf, level, rec, sizeof(*rec));
 	zfcp_dbf_hexdump(adapter->san_dbf, rec, sizeof(*rec), level,
-			 buffer, min(buflen, ZFCP_DBF_ELS_MAX_PAYLOAD));
+			 buffer, min(buflen, ZFCP_DBF_SAN_MAX_PAYLOAD));
 	spin_unlock_irqrestore(&adapter->san_dbf_lock, flags);
 }
 
@@ -1008,8 +1014,6 @@ static int zfcp_san_dbf_view_format(debu
 				    char *out_buf, const char *in_buf)
 {
 	struct zfcp_san_dbf_record *r = (struct zfcp_san_dbf_record *)in_buf;
-	char *buffer = NULL;
-	int buflen = 0, total = 0;
 	char *p = out_buf;
 
 	if (strncmp(r->tag, "dump", ZFCP_DBF_TAG_SIZE) == 0)
@@ -1029,9 +1033,6 @@ static int zfcp_san_dbf_view_format(debu
 		zfcp_dbf_out(&p, "gs_subtype", "0x%02x", ct->gs_subtype);
 		zfcp_dbf_out(&p, "options", "0x%02x", ct->options);
 		zfcp_dbf_out(&p, "max_res_size", "0x%04x", ct->max_res_size);
-		total = ct->len;
-		buffer = ct->payload;
-		buflen = min(total, ZFCP_DBF_CT_PAYLOAD);
 	} else if (strncmp(r->tag, "rctc", ZFCP_DBF_TAG_SIZE) == 0) {
 		struct zfcp_san_dbf_record_ct_response *ct = &r->u.ct_resp;
 		zfcp_dbf_out(&p, "cmd_rsp_code", "0x%04x", ct->cmd_rsp_code);
@@ -1039,23 +1040,12 @@ static int zfcp_san_dbf_view_format(debu
 		zfcp_dbf_out(&p, "reason_code", "0x%02x", ct->reason_code);
 		zfcp_dbf_out(&p, "reason_code_expl", "0x%02x", ct->expl);
 		zfcp_dbf_out(&p, "vendor_unique", "0x%02x", ct->vendor_unique);
-		total = ct->len;
-		buffer = ct->payload;
-		buflen = min(total, ZFCP_DBF_CT_PAYLOAD);
 	} else if (strncmp(r->tag, "oels", ZFCP_DBF_TAG_SIZE) == 0 ||
 		   strncmp(r->tag, "rels", ZFCP_DBF_TAG_SIZE) == 0 ||
 		   strncmp(r->tag, "iels", ZFCP_DBF_TAG_SIZE) == 0) {
 		struct zfcp_san_dbf_record_els *els = &r->u.els;
 		zfcp_dbf_out(&p, "ls_code", "0x%02x", els->ls_code);
-		total = els->len;
-		buffer = els->payload;
-		buflen = min(total, ZFCP_DBF_ELS_PAYLOAD);
 	}
-
-	zfcp_dbf_outd(&p, "payload", buffer, buflen, 0, total);
-	if (buflen == total)
-		p += sprintf(p, "\n");
-
 	return p - out_buf;
 }
 
--- a/drivers/s390/scsi/zfcp_dbf.h	2008-11-03 16:47:05.000000000 +0100
+++ b/drivers/s390/scsi/zfcp_dbf.h	2008-11-03 16:49:20.000000000 +0100
@@ -163,8 +163,6 @@ struct zfcp_san_dbf_record_ct_request {
 	u8 options;
 	u16 max_res_size;
 	u32 len;
-#define ZFCP_DBF_CT_PAYLOAD	24
-	u8 payload[ZFCP_DBF_CT_PAYLOAD];
 } __attribute__ ((packed));
 
 struct zfcp_san_dbf_record_ct_response {
@@ -174,15 +172,11 @@ struct zfcp_san_dbf_record_ct_response {
 	u8 expl;
 	u8 vendor_unique;
 	u32 len;
-	u8 payload[ZFCP_DBF_CT_PAYLOAD];
 } __attribute__ ((packed));
 
 struct zfcp_san_dbf_record_els {
 	u8 ls_code;
 	u32 len;
-#define ZFCP_DBF_ELS_PAYLOAD	32
-#define ZFCP_DBF_ELS_MAX_PAYLOAD 1024
-	u8 payload[ZFCP_DBF_ELS_PAYLOAD];
 } __attribute__ ((packed));
 
 struct zfcp_san_dbf_record {
@@ -196,6 +190,8 @@ struct zfcp_san_dbf_record {
 		struct zfcp_san_dbf_record_ct_response ct_resp;
 		struct zfcp_san_dbf_record_els els;
 	} u;
+#define ZFCP_DBF_SAN_MAX_PAYLOAD 1024
+	u8 payload[32];
 } __attribute__ ((packed));
 
 struct zfcp_scsi_dbf_record {

-- 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [patch 3/8] zfcp: fix mempool usage for status_read requests
  2008-11-04 15:35 ` [patch 3/8] zfcp: fix mempool usage for status_read requests Christof Schmitt
@ 2008-11-05 17:46   ` James Bottomley
  2008-11-06 15:16     ` Christof Schmitt
  0 siblings, 1 reply; 11+ messages in thread
From: James Bottomley @ 2008-11-05 17:46 UTC (permalink / raw)
  To: Christof Schmitt
  Cc: linux-scsi, linux-s390, schwidefsky, heiko.carstens,
	Swen Schillig

On Tue, 2008-11-04 at 16:35 +0100, Christof Schmitt wrote:
> plain text document attachment (fix-status-read-alloc.diff)
> From: Christof Schmitt <christof.schmitt@de.ibm.com>
> 
> When allocating fsf requests without qtcb, store the pointer to the
> mempool in the fsf requests for later call to mempool_free. This
> codepath is only used by the status_read requests.
> 
> Signed-off-by: Christof Schmitt <christof.schmitt@de.ibm.com>
> Signed-off-by: Swen Schillig <swen@vnet.ibm.com>

I put this one in, but can you explain this last signoff better?  You
sent me the patch, so your signoff should appear last.  If Swen altered
the patch, his signoff should appear before yours (with a small
explanation of the alteration in square brackets).  If he's just
approving it, it should be Acked-by: not Signed-off-by:

James



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [patch 3/8] zfcp: fix mempool usage for status_read requests
  2008-11-05 17:46   ` James Bottomley
@ 2008-11-06 15:16     ` Christof Schmitt
  0 siblings, 0 replies; 11+ messages in thread
From: Christof Schmitt @ 2008-11-06 15:16 UTC (permalink / raw)
  To: James Bottomley
  Cc: linux-scsi, linux-s390, schwidefsky, heiko.carstens,
	Swen Schillig

On Wed, Nov 05, 2008 at 12:46:12PM -0500, James Bottomley wrote:
> On Tue, 2008-11-04 at 16:35 +0100, Christof Schmitt wrote:
> > plain text document attachment (fix-status-read-alloc.diff)
> > From: Christof Schmitt <christof.schmitt@de.ibm.com>
> > 
> > When allocating fsf requests without qtcb, store the pointer to the
> > mempool in the fsf requests for later call to mempool_free. This
> > codepath is only used by the status_read requests.
> > 
> > Signed-off-by: Christof Schmitt <christof.schmitt@de.ibm.com>
> > Signed-off-by: Swen Schillig <swen@vnet.ibm.com>
> 
> I put this one in, but can you explain this last signoff better?  You
> sent me the patch, so your signoff should appear last.  If Swen altered
> the patch, his signoff should appear before yours (with a small
> explanation of the alteration in square brackets).  If he's just
> approving it, it should be Acked-by: not Signed-off-by:

I wrote the patch, Swen reviewed it and i sent it as part of the patch
series. It looks like this did not follow the exact guidelines. For
future patches, i will document the review with the "Acked-by:".

Christof

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2008-11-06 15:16 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-11-04 15:35 [patch 0/8] zfcp fixes for 2.6.28 Christof Schmitt
2008-11-04 15:35 ` [patch 1/8] zfcp: Dont clear reference from SCSI device to unit Christof Schmitt
2008-11-04 15:35 ` [patch 2/8] zfcp: fix req_list_locking Christof Schmitt
2008-11-04 15:35 ` [patch 3/8] zfcp: fix mempool usage for status_read requests Christof Schmitt
2008-11-05 17:46   ` James Bottomley
2008-11-06 15:16     ` Christof Schmitt
2008-11-04 15:35 ` [patch 4/8] zfcp: Fix request list handling in error path Christof Schmitt
2008-11-04 15:35 ` [patch 5/8] zfcp: Fix cast warning Christof Schmitt
2008-11-04 15:35 ` [patch 6/8] zfcp: Wait for port scan to complete when setting adapter online Christof Schmitt
2008-11-04 15:35 ` [patch 7/8] zfcp: fix erp timeout cleanup for port open requests Christof Schmitt
2008-11-04 15:35 ` [patch 8/8] zfcp: Fix hexdump data in s390dbf traces Christof Schmitt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox