zfcp fixes and improvements
Subsystems (3)
cover zfcp fixes and improvements
James,
here is a series of small fixes and improvements for zfcp. The patches apply on current scsi-misc.
These patches were pending on the head of our internal patch queue. I hope that i can continue with the larger cleanup patches that are still pending during the next weeks.
Christof
PATCH 1/11 zfcp: receiving an unsolicted status can lead to I/O stall
From: Swen Schillig <[email protected]>
Processing of an unsolicted status request can lead to a locking race of the request_queue's queue_lock during the recreation of the used up status read request while still in interrupt context of the response handler.
Detaching the 'refill' of the long running status read requests from the handler to a scheduled work is solving this issue.
In addition, each refill-run is trying to re-establish the full amount of status read requests, which might have failed in earlier runs.
Signed-off-by Swen Schillig <[email protected]>
Signed-off-by Christof Schmitt <[email protected]>
--- drivers/s390/scsi/zfcp_aux.c | 23 +++++++++++++++++++++++ drivers/s390/scsi/zfcp_dbf.c | 2 +- drivers/s390/scsi/zfcp_def.h | 4 ++-- drivers/s390/scsi/zfcp_erp.c | 19 ++----------------- drivers/s390/scsi/zfcp_ext.h | 1 + drivers/s390/scsi/zfcp_fsf.c | 18 +++--------------- 6 files changed, 32 insertions(+), 35 deletions(-)
drivers/s390/scsi/zfcp_aux.c +23 -0
@@ -970,6 +970,27 @@ static void zfcp_dummy_release(struct dereturn ; } +int zfcp_status_read_refill (struct zfcp_adapter *adapter) +{ +while (atomic_read (&adapter->stat_miss) >0 ) +if (zfcp_fsf_status_read (adapter,ZFCP_WAIT_FOR_SBAL )) +break ; +else +atomic_dec (&adapter->stat_miss); + +if (ZFCP_STATUS_READS_RECOM <=atomic_read (&adapter->stat_miss)) { +zfcp_erp_adapter_reopen (adapter,0 ,103 ,NULL ); +return 1 ; + } +return 0 ; +} + +static void _zfcp_status_read_scheduler (struct work_struct *work) +{ +zfcp_status_read_refill (container_of(work ,struct zfcp_adapter , +stat_work )); +} +/* * Enqueues an adapter at the end of the adapter list in the driver data. * All adapter internal structures are set up.
@@ -1063,6 +1084,7 @@ zfcp_adapter_enqueue(struct ccw_device */* initialize lock of associated request queue */ rwlock_init (&adapter->request_queue.queue_lock); +INIT_WORK (&adapter->stat_work, _zfcp_status_read_scheduler);/* mark adapter unusable as long as sysfs registration is not complete */ atomic_set_mask (ZFCP_STATUS_COMMON_REMOVE , &adapter->status);
@@ -1123,6 +1145,7 @@ zfcp_adapter_dequeue(struct zfcp_adapterint retval =0 ;unsigned long flags; +cancel_work_sync (&adapter->stat_work);zfcp_adapter_scsi_unregister (adapter);device_unregister (&adapter->generic_services);zfcp_sysfs_adapter_remove_files (&adapter->ccw_device->dev);
drivers/s390/scsi/zfcp_dbf.c +1 -1
@@ -268,7 +268,7 @@ void zfcp_hba_dbf_event_fsf_unsol(conststrncpy (rec->tag,"stat" ,ZFCP_DBF_TAG_SIZE );strncpy (rec->tag2, tag,ZFCP_DBF_TAG_SIZE ); - rec->u.status.failed = adapter->status_read_failed; + rec->u.status.failed =atomic_read (&adapter->stat_miss);if (status_buffer !=NULL ) { rec->u.status.status_type = status_buffer->status_type; rec->u.status.status_subtype = status_buffer->status_subtype;
drivers/s390/scsi/zfcp_def.h +2 -2
@@ -136,7 +136,6 @@ zfcp_address_to_sg(void *address, struct#define ZFCP_QTCB_VERSION FSF_QTCB_CURRENT_VERSION/* ATTENTION: value must not be used by hardware */ #define FSF_QTCB_UNSOLICITED_STATUS 0x6305 -#define ZFCP_STATUS_READ_FAILED_THRESHOLD 3#define ZFCP_STATUS_READS_RECOM FSF_STATUS_READS_RECOM/* Do 1st retry in 1 second, then double the timeout for each following retry */
@@ -759,7 +758,8 @@ struct zfcp_adapter {rwlock_t abort_lock;/* Protects against SCSI stack abort/command completion races */ -u16 status_read_failed;/* # failed status reads */ +atomic_t stat_miss;/* # missing status reads*/ +struct work_struct stat_work;atomic_t status;/* status of this adapter */ struct list_head erp_ready_head;/* error recovery for this adapter/devices */
drivers/s390/scsi/zfcp_fsf.c +3 -15
@@ -1029,21 +1029,9 @@ zfcp_fsf_status_read_handler(struct zfcp* FIXME: * allocation failure possible? (Is this code needed?) */ - retval =zfcp_fsf_status_read (adapter,0 ); -if (retval <0 ) { -ZFCP_LOG_INFO ("Failed to create unsolicited status read " -"request for the adapter %s.\n" , -zfcp_get_busid_by_adapter (adapter)); -/* temporary fix to avoid status read buffer shortage */ - adapter->status_read_failed++; -if ((ZFCP_STATUS_READS_RECOM - adapter->status_read_failed) - <ZFCP_STATUS_READ_FAILED_THRESHOLD ) { -ZFCP_LOG_INFO ("restart adapter %s due to status read " -"buffer shortage\n" , -zfcp_get_busid_by_adapter (adapter)); -zfcp_erp_adapter_reopen (adapter,0 ,103 , fsf_req); - } - } + +atomic_inc (&adapter->stat_miss); +schedule_work (&adapter->stat_work); out:return retval; }
drivers/s390/scsi/zfcp_erp.c +2 -17
@@ -2139,25 +2139,10 @@ static intzfcp_erp_adapter_strategy_open_fsf_statusread (struct zfcp_erp_action *erp_action) { -int retval =ZFCP_ERP_SUCCEEDED ; -int temp_ret;struct zfcp_adapter *adapter = erp_action->adapter; -int i; - adapter->status_read_failed =0 ; -for (i =0 ; i <ZFCP_STATUS_READS_RECOM ; i++) { - temp_ret =zfcp_fsf_status_read (adapter,ZFCP_WAIT_FOR_SBAL ); -if (temp_ret <0 ) { -ZFCP_LOG_INFO ("error: set-up of unsolicited status " -"notification failed on adapter %s\n" , -zfcp_get_busid_by_adapter (adapter)); - retval =ZFCP_ERP_FAILED ; - i--; -break ; - } - } - -return retval; +atomic_set (&adapter->stat_miss,16 ); +return zfcp_status_read_refill (adapter); }/*
drivers/s390/scsi/zfcp_ext.h +1 -0
@@ -92,6 +92,7 @@ extern void zfcp_fsf_start_timer(structextern void zfcp_erp_start_timer (struct zfcp_fsf_req *);extern void zfcp_fsf_req_dismiss_all (struct zfcp_adapter *);extern int zfcp_fsf_status_read (struct zfcp_adapter *,int ); +extern int zfcp_status_read_refill (struct zfcp_adapter *adapter);extern int zfcp_fsf_req_create (struct zfcp_adapter *,u32 ,int ,mempool_t *,unsigned long *,struct zfcp_fsf_req **); extern int zfcp_fsf_send_ct(struct zfcp_send_ct *, mempool_t *,
PATCH 2/11 zfcp: Fix mempool pointer for GID_PN request allocation
In reply to Christof SchmittFrom: Christof Schmitt <[email protected]>
When allocating memory for GID_PN nameserver requests, the allocation function stores the pointer to the mempool, but then overwrites the pointer via memset. Later, the wrong function to free the memory will be called, since this is based on the stored pointer.
Fix this by first initializing the struct and then storing the pointer.
Signed-off-by Christof Schmitt <[email protected]>
Signed-off-by Martin Peschke <[email protected]>
---
drivers/s390/scsi/zfcp_aux.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
drivers/s390/scsi/zfcp_aux.c +3 -6
@@ -1533,19 +1533,16 @@ zfcp_gid_pn_buffers_alloc(struct zfcp_gi {struct zfcp_gid_pn_data *data; -if (pool !=NULL ) { +if (pool) data =mempool_alloc (pool,GFP_ATOMIC ); -if (likely (data !=NULL )) { - data->ct.pool = pool; - } - }else { +else data =kmem_cache_alloc (zfcp_data.gid_pn_cache,GFP_ATOMIC ); - }if (NULL == data)return -ENOMEM ;memset (data,0 ,sizeof (*data)); + data->ct.pool = pool;sg_init_table (&data->req ,1 );sg_init_table (&data->resp ,1 ); data->ct.req = &data->req;
PATCH 3/11 zfcp: Fix fsf_status_read return code handling
In reply to Christof SchmittFrom: Heiko Carstens <[email protected]>
If allocation of a status buffer failed the function incorrectly returned 0 instead of -ENOMEM.
Signed-off-by Heiko Carstens <[email protected]>
Signed-off-by Martin Peschke <[email protected]>
---
drivers/s390/scsi/zfcp_fsf.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/s390/scsi/zfcp_fsf.c +3 -4
@@ -712,7 +712,7 @@ zfcp_fsf_status_read(struct zfcp_adapterstruct fsf_status_read_buffer *status_buffer;unsigned long lock_flags;volatile struct qdio_buffer_element *sbale; -int retval =0 ; +int retval;/* setup new FSF request */ retval = zfcp_fsf_req_create(adapter, FSF_QTCB_UNSOLICITED_STATUS,
@@ -731,12 +731,11 @@ zfcp_fsf_status_read(struct zfcp_adapter sbale[2 ].flags |=SBAL_FLAGS_LAST_ENTRY ; fsf_req->sbale_curr =2 ; + retval = -ENOMEM ; status_buffer =mempool_alloc (adapter->pool.data_status_read,GFP_ATOMIC ); -if (!status_buffer) { -ZFCP_LOG_NORMAL ("bug: could not get some buffer\n" ); +if (!status_buffer) goto failed_buf; - }memset (status_buffer,0 ,sizeof (struct fsf_status_read_buffer )); fsf_req->data = (unsigned long ) status_buffer;
PATCH 4/11 zfcp: Remove some sparse warnings
In reply to Christof SchmittFrom: Christof Schmitt <[email protected]>
Remove some sparse warnings by telling sparse that zfcp_req_create acquires the lock for the request queue.
Signed-off-by Christof Schmitt <[email protected]>
Signed-off-by Martin Peschke <[email protected]>
--- drivers/s390/scsi/zfcp_ext.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/s390/scsi/zfcp_ext.h +2 -1
@@ -94,7 +94,8 @@ extern void zfcp_fsf_req_dismiss_all(strextern int zfcp_fsf_status_read (struct zfcp_adapter *,int );extern int zfcp_status_read_refill (struct zfcp_adapter *adapter); extern int zfcp_fsf_req_create(struct zfcp_adapter *, u32, int, mempool_t *, -unsigned long *,struct zfcp_fsf_req **); + unsigned long *, struct zfcp_fsf_req **) + __acquires(adapter->request_queue.queue_lock);extern int zfcp_fsf_send_ct (struct zfcp_send_ct *,mempool_t *,struct zfcp_erp_action *);extern int zfcp_fsf_send_els (struct zfcp_send_els *);
PATCH 5/11 zfcp: Remove field sbal_last from trace record.
In reply to Christof SchmittFrom: Martin Peschke <[email protected]>
This field is not needed, because it designates an index with a fix offset from sbal_first. It's name is confusing anyway.
Signed-off-by Martin Peschke <[email protected]>
Signed-off-by Christof Schmitt <[email protected]>
--- drivers/s390/scsi/zfcp_dbf.c | 2 -- drivers/s390/scsi/zfcp_dbf.h | 1 - 2 files changed, 3 deletions(-)
drivers/s390/scsi/zfcp_dbf.c +0 -2
@@ -187,7 +187,6 @@ void zfcp_hba_dbf_event_fsf_response(str response->fsf_req_status = fsf_req->status; response->sbal_first = fsf_req->sbal_first; response->sbal_curr = fsf_req->sbal_curr; - response->sbal_last = fsf_req->sbal_last; response->pool = fsf_req->pool !=NULL ; response->erp_action = (unsigned long )fsf_req->erp_action;
@@ -356,7 +355,6 @@ static void zfcp_hba_dbf_view_response(czfcp_dbf_out (p,"fsf_req_status" ,"0x%08x" , r->fsf_req_status);zfcp_dbf_out (p,"sbal_first" ,"0x%02x" , r->sbal_first);zfcp_dbf_out (p,"sbal_curr" ,"0x%02x" , r->sbal_curr); -zfcp_dbf_out (p,"sbal_last" ,"0x%02x" , r->sbal_last);zfcp_dbf_out (p,"pool" ,"0x%02x" , r->pool);switch (r->fsf_command) {
drivers/s390/scsi/zfcp_dbf.h +0 -1
@@ -98,7 +98,6 @@ struct zfcp_hba_dbf_record_response {u32 fsf_req_status;u8 sbal_first;u8 sbal_curr; -u8 sbal_last;u8 pool;u64 erp_action;union {
PATCH 6/11 zfcp: Rename sbal_last.
In reply to Christof SchmittFrom: Martin Peschke <[email protected]>
sbal_last is confusing, as it is not the last one actually used, but just a limit. sbal_limit is a better name.
Signed-off-by Martin Peschke <[email protected]>
Signed-off-by Christof Schmitt <[email protected]>
--- drivers/s390/scsi/zfcp_def.h | 2 +- drivers/s390/scsi/zfcp_qdio.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-)
drivers/s390/scsi/zfcp_def.h +1 -1
@@ -851,7 +851,7 @@ struct zfcp_fsf_req {struct zfcp_adapter *adapter;/* adapter request belongs to */ u8 sbal_number;/* nr of SBALs free for use */ u8 sbal_first;/* first SBAL for this request */ -u8 sbal_last;/* last possible SBAL for +u8 sbal_limit;/* last possible SBAL for this reuest */ u8 sbal_curr;/* current SBAL during creation of request */
drivers/s390/scsi/zfcp_qdio.c +4 -4
@@ -432,9 +432,9 @@ zfcp_qdio_sbal_limit(struct zfcp_fsf_req {int count =atomic_read (&fsf_req->adapter->request_queue.free_count); count =min (count, max_sbals); - fsf_req->sbal_last = fsf_req->sbal_first; - fsf_req->sbal_last += (count -1 ); - fsf_req->sbal_last %=QDIO_MAX_BUFFERS_PER_Q ; + fsf_req->sbal_limit = fsf_req->sbal_first; + fsf_req->sbal_limit += (count -1 ); + fsf_req->sbal_limit %=QDIO_MAX_BUFFERS_PER_Q ; }/**
@@ -455,7 +455,7 @@ zfcp_qdio_sbal_chain(struct zfcp_fsf_req sbale->flags |=SBAL_FLAGS_LAST_ENTRY ;/* don't exceed last allowed SBAL */ -if (fsf_req->sbal_curr == fsf_req->sbal_last) +if (fsf_req->sbal_curr == fsf_req->sbal_limit)return NULL ;/* set chaining flag in first SBALE of current SBAL */
PATCH 7/11 zfcp: Rename sbal_curr to sbal_last.
In reply to Christof SchmittFrom: Martin Peschke <[email protected]>
sbal_last is more appropriate, because it matches sbal_first.
Signed-off-by Martin Peschke <[email protected]>
Signed-off-by Christof Schmitt <[email protected]>
--- drivers/s390/scsi/zfcp_dbf.c | 4 ++-- drivers/s390/scsi/zfcp_dbf.h | 2 +- drivers/s390/scsi/zfcp_def.h | 3 +-- drivers/s390/scsi/zfcp_fsf.c | 36 ++++++++++++++++++------------------ drivers/s390/scsi/zfcp_qdio.c | 16 ++++++++-------- 5 files changed, 30 insertions(+), 31 deletions(-)
drivers/s390/scsi/zfcp_dbf.h +1 -1
@@ -97,7 +97,7 @@ struct zfcp_hba_dbf_record_response {u8 fsf_status_qual[FSF_STATUS_QUALIFIER_SIZE ];u32 fsf_req_status;u8 sbal_first; -u8 sbal_curr; +u8 sbal_last;u8 pool;u64 erp_action;union {
drivers/s390/scsi/zfcp_def.h +1 -2
@@ -851,10 +851,9 @@ struct zfcp_fsf_req {struct zfcp_adapter *adapter;/* adapter request belongs to */ u8 sbal_number;/* nr of SBALs free for use */ u8 sbal_first;/* first SBAL for this request */ +u8 sbal_last;/* last SBAL for this request */ u8 sbal_limit;/* last possible SBAL for this reuest */ -u8 sbal_curr;/* current SBAL during creation -of request */ u8 sbale_curr;/* current SBALE during creation of request */ wait_queue_head_t completion_wq;/* can be used by a routine
drivers/s390/scsi/zfcp_qdio.c +8 -8
@@ -415,7 +415,7 @@ zfcp_qdio_sbale_resp(struct zfcp_fsf_reqvolatile struct qdio_buffer_element *zfcp_qdio_sbale_curr (struct zfcp_fsf_req *fsf_req) { -return zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_curr, +return zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_last, fsf_req->sbale_curr); }
@@ -443,7 +443,7 @@ zfcp_qdio_sbal_limit(struct zfcp_fsf_req* @fsf_req: zfcp_fsf_req to be processed * @sbtype: SBAL flags which have to be set in first SBALE of new SBAL * -* This function changes sbal_curr, sbale_curr, sbal_number of fsf_req. +* This function changes sbal_last, sbale_curr, sbal_number of fsf_req. */ static volatile struct qdio_buffer_element *zfcp_qdio_sbal_chain (struct zfcp_fsf_req *fsf_req,unsigned long sbtype)
@@ -455,16 +455,16 @@ zfcp_qdio_sbal_chain(struct zfcp_fsf_req sbale->flags |=SBAL_FLAGS_LAST_ENTRY ;/* don't exceed last allowed SBAL */ -if (fsf_req->sbal_curr == fsf_req->sbal_limit) +if (fsf_req->sbal_last == fsf_req->sbal_limit)return NULL ;/* set chaining flag in first SBALE of current SBAL */ - sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_curr,0 ); + sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_last,0 ); sbale->flags |=SBAL_FLAGS0_MORE_SBALS ;/* calculate index of next SBAL */ - fsf_req->sbal_curr++; - fsf_req->sbal_curr %=QDIO_MAX_BUFFERS_PER_Q ; + fsf_req->sbal_last++; + fsf_req->sbal_last %=QDIO_MAX_BUFFERS_PER_Q ;/* keep this requests number of SBALs up-to-date */ fsf_req->sbal_number++;
@@ -523,7 +523,7 @@ static inline int zfcp_qdio_sbals_wipe(struct zfcp_fsf_req *fsf_req) {return zfcp_qdio_sbals_zero (&fsf_req->adapter->request_queue, - fsf_req->sbal_first, fsf_req->sbal_curr); + fsf_req->sbal_first, fsf_req->sbal_last); }
@@ -601,7 +601,7 @@ zfcp_qdio_sbals_from_sg(struct zfcp_fsf_zfcp_qdio_sbal_limit (fsf_req, max_sbals);/* set storage-block type for current SBAL */ - sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_curr,0 ); + sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_last,0 ); sbale->flags |= sbtype;/* process all segements of scatter-gather list */
drivers/s390/scsi/zfcp_fsf.c +18 -18
@@ -726,7 +726,7 @@ zfcp_fsf_status_read(struct zfcp_adapter goto failed_req_create; } - sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_curr,0 ); + sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_last,0 ); sbale[0 ].flags |=SBAL_FLAGS0_TYPE_STATUS ; sbale[2 ].flags |=SBAL_FLAGS_LAST_ENTRY ; fsf_req->sbale_curr =2 ;
@@ -1075,7 +1075,7 @@ zfcp_fsf_abort_fcp_command(unsigned long &unit->status))) goto unit_blocked; - sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_curr,0 ); + sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_last,0 ); sbale[0 ].flags |=SBAL_FLAGS0_TYPE_READ ; sbale[1 ].flags |=SBAL_FLAGS_LAST_ENTRY ;
@@ -1295,7 +1295,7 @@ zfcp_fsf_send_ct(struct zfcp_send_ct *ct goto failed_req; } - sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_curr,0 ); + sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_last,0 );if (zfcp_use_one_sbal (ct->req, ct->req_count, ct->resp, ct->resp_count)){/* both request buffer and response buffer
@@ -1593,7 +1593,7 @@ zfcp_fsf_send_els(struct zfcp_send_els * goto port_blocked; } - sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_curr,0 ); + sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_last,0 );if (zfcp_use_one_sbal (els->req, els->req_count, els->resp, els->resp_count)){/* both request buffer and response buffer
@@ -1657,7 +1657,7 @@ zfcp_fsf_send_els(struct zfcp_send_els * fsf_req->qtcb->bottom.support.timeout =ZFCP_ELS_TIMEOUT ; fsf_req->data = (unsigned long ) els; - sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_curr,0 ); + sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_last,0 );zfcp_san_dbf_event_els_request (fsf_req);
@@ -1872,7 +1872,7 @@ zfcp_fsf_exchange_config_data(struct zfcreturn retval; } - sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_curr,0 ); + sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_last,0 ); sbale[0 ].flags |=SBAL_FLAGS0_TYPE_READ ; sbale[1 ].flags |=SBAL_FLAGS_LAST_ENTRY ;
@@ -1925,7 +1925,7 @@ zfcp_fsf_exchange_config_data_sync(strucreturn retval; } - sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_curr,0 ); + sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_last,0 ); sbale[0 ].flags |=SBAL_FLAGS0_TYPE_READ ; sbale[1 ].flags |=SBAL_FLAGS_LAST_ENTRY ;
@@ -2187,7 +2187,7 @@ zfcp_fsf_exchange_port_data(struct zfcp_return retval; } - sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_curr,0 ); + sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_last,0 ); sbale[0 ].flags |=SBAL_FLAGS0_TYPE_READ ; sbale[1 ].flags |=SBAL_FLAGS_LAST_ENTRY ;
@@ -2249,7 +2249,7 @@ zfcp_fsf_exchange_port_data_sync(structif (data) fsf_req->data = (unsigned long ) data; - sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_curr,0 ); + sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_last,0 ); sbale[0 ].flags |=SBAL_FLAGS0_TYPE_READ ; sbale[1 ].flags |=SBAL_FLAGS_LAST_ENTRY ;
@@ -2359,7 +2359,7 @@ zfcp_fsf_open_port(struct zfcp_erp_actio goto out; } - sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_curr,0 ); + sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_last,0 ); sbale[0 ].flags |=SBAL_FLAGS0_TYPE_READ ; sbale[1 ].flags |=SBAL_FLAGS_LAST_ENTRY ;
@@ -2591,7 +2591,7 @@ zfcp_fsf_close_port(struct zfcp_erp_acti goto out; } - sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_curr,0 ); + sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_last,0 ); sbale[0 ].flags |=SBAL_FLAGS0_TYPE_READ ; sbale[1 ].flags |=SBAL_FLAGS_LAST_ENTRY ;
@@ -2720,7 +2720,7 @@ zfcp_fsf_close_physical_port(struct zfcp goto out; } - sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_curr,0 ); + sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_last,0 ); sbale[0 ].flags |=SBAL_FLAGS0_TYPE_READ ; sbale[1 ].flags |=SBAL_FLAGS_LAST_ENTRY ;
@@ -2915,7 +2915,7 @@ zfcp_fsf_open_unit(struct zfcp_erp_actio goto out; } - sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_curr,0 ); + sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_last,0 ); sbale[0 ].flags |=SBAL_FLAGS0_TYPE_READ ; sbale[1 ].flags |=SBAL_FLAGS_LAST_ENTRY ;
@@ -3230,7 +3230,7 @@ zfcp_fsf_close_unit(struct zfcp_erp_acti goto out; } - sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_curr,0 ); + sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_last,0 ); sbale[0 ].flags |=SBAL_FLAGS0_TYPE_READ ; sbale[1 ].flags |=SBAL_FLAGS_LAST_ENTRY ;
@@ -3613,7 +3613,7 @@ zfcp_fsf_send_fcp_command_task_managemen fsf_req->qtcb->bottom.io.fcp_cmnd_length =sizeof (struct fcp_cmnd_iu ) +sizeof (fcp_dl_t); - sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_curr,0 ); + sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_last,0 ); sbale[0 ].flags |=SBAL_FLAGS0_TYPE_WRITE ; sbale[1 ].flags |=SBAL_FLAGS_LAST_ENTRY ;
@@ -4238,7 +4238,7 @@ zfcp_fsf_control_file(struct zfcp_adapte goto unlock_queue_lock; } - sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_curr,0 ); + sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_last,0 ); sbale[0 ].flags |= direction; bottom = &fsf_req->qtcb->bottom.support;
@@ -4590,14 +4590,14 @@ zfcp_fsf_req_create(struct zfcp_adapter } fsf_req->sbal_number =1 ; fsf_req->sbal_first = req_queue->free_index; - fsf_req->sbal_curr = req_queue->free_index; + fsf_req->sbal_last = req_queue->free_index; fsf_req->sbale_curr =1 ;if (likely (req_flags &ZFCP_REQ_AUTO_CLEANUP )) { fsf_req->status |=ZFCP_STATUS_FSFREQ_CLEANUP ; } - sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_curr,0 ); + sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_last,0 );/* setup common SBALE fields */ sbale[0 ].addr = (void *) fsf_req->req_id;
drivers/s390/scsi/zfcp_dbf.c +2 -2
@@ -186,7 +186,7 @@ void zfcp_hba_dbf_event_fsf_response(str fsf_status_qual, FSF_STATUS_QUALIFIER_SIZE); response->fsf_req_status = fsf_req->status; response->sbal_first = fsf_req->sbal_first; - response->sbal_curr = fsf_req->sbal_curr; + response->sbal_last = fsf_req->sbal_last; response->pool = fsf_req->pool !=NULL ; response->erp_action = (unsigned long )fsf_req->erp_action;
@@ -354,7 +354,7 @@ static void zfcp_hba_dbf_view_response(c FSF_STATUS_QUALIFIER_SIZE, 0, FSF_STATUS_QUALIFIER_SIZE);zfcp_dbf_out (p,"fsf_req_status" ,"0x%08x" , r->fsf_req_status);zfcp_dbf_out (p,"sbal_first" ,"0x%02x" , r->sbal_first); -zfcp_dbf_out (p,"sbal_curr" ,"0x%02x" , r->sbal_curr); +zfcp_dbf_out (p,"sbal_last" ,"0x%02x" , r->sbal_last);zfcp_dbf_out (p,"pool" ,"0x%02x" , r->pool);switch (r->fsf_command) {
PATCH 8/11 zfcp: Add information about interrupt to trace.
In reply to Christof SchmittFrom: Martin Peschke <[email protected]>
Store the index of the buffer in the inbound queue used to report request completion in trace record for request coompletion. This piece of information allows to better compare qdio and zfcp traces.
Signed-off-by Martin Peschke <[email protected]>
Signed-off-by Christof Schmitt <[email protected]>
--- drivers/s390/scsi/zfcp_dbf.c | 2 ++ drivers/s390/scsi/zfcp_dbf.h | 1 + drivers/s390/scsi/zfcp_def.h | 1 + drivers/s390/scsi/zfcp_qdio.c | 5 +++-- 4 files changed, 7 insertions(+), 2 deletions(-)
drivers/s390/scsi/zfcp_dbf.c +2 -0
@@ -187,6 +187,7 @@ void zfcp_hba_dbf_event_fsf_response(str response->fsf_req_status = fsf_req->status; response->sbal_first = fsf_req->sbal_first; response->sbal_last = fsf_req->sbal_last; + response->sbal_response = fsf_req->sbal_response; response->pool = fsf_req->pool !=NULL ; response->erp_action = (unsigned long )fsf_req->erp_action;
@@ -355,6 +356,7 @@ static void zfcp_hba_dbf_view_response(czfcp_dbf_out (p,"fsf_req_status" ,"0x%08x" , r->fsf_req_status);zfcp_dbf_out (p,"sbal_first" ,"0x%02x" , r->sbal_first);zfcp_dbf_out (p,"sbal_last" ,"0x%02x" , r->sbal_last); +zfcp_dbf_out (p,"sbal_response" ,"0x%02x" , r->sbal_response);zfcp_dbf_out (p,"pool" ,"0x%02x" , r->pool);switch (r->fsf_command) {
drivers/s390/scsi/zfcp_dbf.h +1 -0
@@ -98,6 +98,7 @@ struct zfcp_hba_dbf_record_response {u32 fsf_req_status;u8 sbal_first;u8 sbal_last; +u8 sbal_response;u8 pool;u64 erp_action;union {
drivers/s390/scsi/zfcp_def.h +1 -0
@@ -856,6 +856,7 @@ struct zfcp_fsf_req {this reuest */ u8 sbale_curr;/* current SBALE during creation of request */ +u8 sbal_response;/* SBAL used in interrupt */ wait_queue_head_t completion_wq;/* can be used by a routine to wait for completion */ volatile u32 status;/* status of this request */
drivers/s390/scsi/zfcp_qdio.c +3 -2
@@ -235,7 +235,7 @@ zfcp_qdio_request_handler(struct ccw_dev* zfcp_qdio_reqid_check - checks for valid reqids. */ static void zfcp_qdio_reqid_check (struct zfcp_adapter *adapter, -unsigned long req_id) +unsigned long req_id,int sbal) {struct zfcp_fsf_req *fsf_req;unsigned long flags;
@@ -255,6 +255,7 @@ static void zfcp_qdio_reqid_check(structatomic_dec (&adapter->reqs_active);spin_unlock_irqrestore (&adapter->req_list_lock, flags); + fsf_req->sbal_response = sbal;/* finish the FSF request */ zfcp_fsf_req_complete (fsf_req); }
@@ -321,7 +322,7 @@ zfcp_qdio_response_handler(struct ccw_de/* look for QDIO request identifiers in SB */ buffere = &buffer->element[buffere_index];zfcp_qdio_reqid_check (adapter, - (unsigned long ) buffere->addr); + (unsigned long ) buffere->addr, i);/* * A single used SBALE per inbound SBALE has been
PATCH 9/11 zfcp: Refine trace levels of some recovery related events.
In reply to Christof SchmittFrom: Martin Peschke <[email protected]>
This change better spreads trace levels of recovery related events. There was an overlap of traces for some recovery triggers and the processing of recovery actions.
Signed-off-by Martin Peschke <[email protected]>
Signed-off-by Christof Schmitt <[email protected]>
--- drivers/s390/scsi/zfcp_dbf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/s390/scsi/zfcp_dbf.c +2 -2
@@ -696,7 +696,7 @@ void zfcp_rec_dbf_event_thread(u8 id2, s r->u.thread.total = total; r->u.thread.ready = ready; r->u.thread.running = running; -debug_event (adapter->rec_dbf,5 , r,sizeof (*r)); +debug_event (adapter->rec_dbf,6 , r,sizeof (*r));spin_unlock_irqrestore (&adapter->rec_dbf_lock, flags); }
@@ -823,7 +823,7 @@ void zfcp_rec_dbf_event_action(u8 id2, s r->u.action.status = erp_action->status; r->u.action.step = erp_action->step; r->u.action.fsf_req = (unsigned long )erp_action->fsf_req; -debug_event (adapter->rec_dbf,4 , r,sizeof (*r)); +debug_event (adapter->rec_dbf,5 , r,sizeof (*r));spin_unlock_irqrestore (&adapter->rec_dbf_lock, flags); }
PATCH 10/11 zfcp: remove some __attribute__ ((packed))
In reply to Christof SchmittFrom: Martin Peschke <[email protected]>
There is no need to pack data structures which describe the contents of records in the new recovery trace.
lcrash currently depends on the binary format for the other traces, removing the packed attribute from all traces would break trace debugging with lcrash.
Signed-off-by Martin Peschke <[email protected]>
Signed-off-by Christof Schmitt <[email protected]>
--- drivers/s390/scsi/zfcp_dbf.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/s390/scsi/zfcp_dbf.h +5 -5
@@ -38,7 +38,7 @@ struct zfcp_rec_dbf_record_thread {u32 total;u32 ready;u32 running; -} __attribute__ ((packed)); +};struct zfcp_rec_dbf_record_target {u64 ref;
@@ -47,7 +47,7 @@ struct zfcp_rec_dbf_record_target {u64 wwpn;u64 fcp_lun;u32 erp_count; -} __attribute__ ((packed)); +};struct zfcp_rec_dbf_record_trigger {u8 want;
@@ -59,14 +59,14 @@ struct zfcp_rec_dbf_record_trigger {u64 action;u64 wwpn;u64 fcp_lun; -} __attribute__ ((packed)); +};struct zfcp_rec_dbf_record_action {u32 status;u32 step;u64 action;u64 fsf_req; -} __attribute__ ((packed)); +};struct zfcp_rec_dbf_record {u8 id;
@@ -77,7 +77,7 @@ struct zfcp_rec_dbf_record {struct zfcp_rec_dbf_record_target target;struct zfcp_rec_dbf_record_trigger trigger; } u; -} __attribute__ ((packed)); +};enum {ZFCP_REC_DBF_ID_ACTION ,
PATCH 11/11 zfcp: Fix sparse warning by providing new entry in dbf
In reply to Christof SchmittFrom: Christof Schmitt <[email protected]>
drivers/s390/scsi/zfcp_dbf.c:692:2: warning: context imbalance in 'zfcp_rec_dbf_event_thread' - different lock contexts for basic block
Replace the parameter indicating if the lock is held with a new entry function that only acquires the lock. This makes the lock handling more visible and removes the sparse warning.
Signed-off-by Christof Schmitt <[email protected]>
Signed-off-by Martin Peschke <[email protected]>
--- drivers/s390/scsi/zfcp_dbf.c | 23 +++++++++++++++++------ drivers/s390/scsi/zfcp_erp.c | 18 +++++++++--------- drivers/s390/scsi/zfcp_ext.h | 4 ++-- 3 files changed, 28 insertions(+), 17 deletions(-)
drivers/s390/scsi/zfcp_dbf.c +17 -6
@@ -670,24 +670,20 @@ static struct debug_view zfcp_rec_dbf_vi* zfcp_rec_dbf_event_thread - trace event related to recovery thread operation * @id2: identifier for event * @adapter: adapter -* @lock: non-zero value indicates that erp_lock has not yet been acquired +* This function assumes that the caller is holding erp_lock. */ -void zfcp_rec_dbf_event_thread (u8 id2,struct zfcp_adapter *adapter,int lock) +void zfcp_rec_dbf_event_thread (u8 id2,struct zfcp_adapter *adapter) {struct zfcp_rec_dbf_record *r = &adapter->rec_dbf_buf;unsigned long flags =0 ;struct list_head *entry;unsigned ready =0 , running =0 , total; -if (lock) -read_lock_irqsave (&adapter->erp_lock, flags);list_for_each (entry, &adapter->erp_ready_head) ready++;list_for_each (entry, &adapter->erp_running_head) running++; total = adapter->erp_total_count; -if (lock) -read_unlock_irqrestore (&adapter->erp_lock, flags);spin_lock_irqsave (&adapter->rec_dbf_lock, flags);memset (r,0 ,sizeof (*r));
@@ -700,6 +696,21 @@ void zfcp_rec_dbf_event_thread(u8 id2, sspin_unlock_irqrestore (&adapter->rec_dbf_lock, flags); } +/** +* zfcp_rec_dbf_event_thread - trace event related to recovery thread operation +* @id2: identifier for event +* @adapter: adapter +* This function assumes that the caller does not hold erp_lock. +*/ +void zfcp_rec_dbf_event_thread_lock (u8 id2,struct zfcp_adapter *adapter) +{ +unsigned long flags; + +read_lock_irqsave (&adapter->erp_lock, flags); +zfcp_rec_dbf_event_thread (id2, adapter); +read_unlock_irqrestore (&adapter->erp_lock, flags); +} + static void zfcp_rec_dbf_event_target(u8 id2, void *ref, struct zfcp_adapter *adapter, atomic_t *status, atomic_t *erp_count,
drivers/s390/scsi/zfcp_erp.c +9 -9
@@ -783,7 +783,7 @@ zfcp_erp_action_ready(struct zfcp_erp_aczfcp_erp_action_to_ready (erp_action);up (&adapter->erp_ready_sem); -zfcp_rec_dbf_event_thread (2 , adapter,0 ); +zfcp_rec_dbf_event_thread (2 , adapter); }/*
@@ -995,7 +995,7 @@ zfcp_erp_thread_kill(struct zfcp_adapteratomic_set_mask (ZFCP_STATUS_ADAPTER_ERP_THREAD_KILL , &adapter->status);up (&adapter->erp_ready_sem); -zfcp_rec_dbf_event_thread (2 , adapter,1 ); +zfcp_rec_dbf_event_thread_lock (2 , adapter);wait_event (adapter->erp_thread_wqh, !atomic_test_mask(ZFCP_STATUS_ADAPTER_ERP_THREAD_UP,
@@ -1050,9 +1050,9 @@ zfcp_erp_thread(void *data)* no action in 'ready' queue to be processed and * thread is not to be killed */ -zfcp_rec_dbf_event_thread (4 , adapter,1 ); +zfcp_rec_dbf_event_thread_lock (4 , adapter);down_interruptible (&adapter->erp_ready_sem); -zfcp_rec_dbf_event_thread (5 , adapter,1 ); +zfcp_rec_dbf_event_thread_lock (5 , adapter); }atomic_clear_mask (ZFCP_STATUS_ADAPTER_ERP_THREAD_UP , &adapter->status);
@@ -2062,9 +2062,9 @@ zfcp_erp_adapter_strategy_open_fsf_xconf* _must_ be the one belonging to the 'exchange config * data' request. */ -zfcp_rec_dbf_event_thread (6 , adapter,1 ); +zfcp_rec_dbf_event_thread_lock (6 , adapter);down (&adapter->erp_ready_sem); -zfcp_rec_dbf_event_thread (7 , adapter,1 ); +zfcp_rec_dbf_event_thread_lock (7 , adapter);if (erp_action->status &ZFCP_STATUS_ERP_TIMEDOUT ) { ZFCP_LOG_INFO("error: exchange of configuration data " "for adapter %s timed out\n",
@@ -2118,9 +2118,9 @@ zfcp_erp_adapter_strategy_open_fsf_xport } ret =ZFCP_ERP_SUCCEEDED ; -zfcp_rec_dbf_event_thread (8 , adapter,1 ); +zfcp_rec_dbf_event_thread_lock (8 , adapter);down (&adapter->erp_ready_sem); -zfcp_rec_dbf_event_thread (9 , adapter,1 ); +zfcp_rec_dbf_event_thread_lock (9 , adapter);if (erp_action->status &ZFCP_STATUS_ERP_TIMEDOUT ) {ZFCP_LOG_INFO ("error: exchange port data timed out (adapter " "%s)\n" ,zfcp_get_busid_by_adapter (adapter));
@@ -2876,7 +2876,7 @@ static int zfcp_erp_action_enqueue(int w/* finally put it into 'ready' queue and kick erp thread */ list_add_tail (&erp_action->list, &adapter->erp_ready_head);up (&adapter->erp_ready_sem); -zfcp_rec_dbf_event_thread (1 , adapter,0 ); +zfcp_rec_dbf_event_thread (1 , adapter); retval =0 ; out: zfcp_rec_dbf_event_trigger(id, ref, want, need, erp_action,
drivers/s390/scsi/zfcp_ext.h +2 -2
@@ -169,8 +169,8 @@ extern void zfcp_erp_port_access_changedextern void zfcp_erp_unit_access_changed (struct zfcp_unit *,u8 ,void *);/******************************** AUX ****************************************/ -extern void zfcp_rec_dbf_event_thread (u8 id,struct zfcp_adapter *adapter, -int lock); +extern void zfcp_rec_dbf_event_thread (u8 id,struct zfcp_adapter *adapter); +extern void zfcp_rec_dbf_event_thread_lock (u8 id,struct zfcp_adapter *adapter);extern void zfcp_rec_dbf_event_adapter (u8 id,void *ref,struct zfcp_adapter *);extern void zfcp_rec_dbf_event_port (u8 id,void *ref,struct zfcp_port *port);extern void zfcp_rec_dbf_event_unit (u8 id,void *ref,struct zfcp_unit *unit);