From f9eca022760079c0556cb311e3aa9091a3921a31 Mon Sep 17 00:00:00 2001 From: Steffen Maier Date: Thu, 8 Nov 2018 15:44:45 +0100 Subject: scsi: zfcp: drop duplicate fsf_command from zfcp_fsf_req which is also in QTCB header Status read buffers (SRBs, unsolicited notifications) never use a QTCB [zfcp_fsf_req_create()]. zfcp_fsf_req_send() already uses this to distinguish SRBs from other FSF request types. We can re-use this method in zfcp_fsf_req_complete(). Introduce a helper function to make the check for req->qtcb less magic. SRBs always are FSF_QTCB_UNSOLICITED_STATUS, so we can hard-code this for the two trace functions dealing with SRBs. All other FSF request types have a QTCB and we can get the fsf_command from there. zfcp_dbf_hba_fsf_response() and thus zfcp_dbf_hba_fsf_res() are only called for non-SRB requests so it's safe to dereference the QTCB [zfcp_fsf_req_complete() returns early on SRB, else calls zfcp_fsf_protstatus_eval() which calls zfcp_dbf_hba_fsf_response()]. In zfcp_scsi_forget_cmnd() we guard the QTCB dereference with a preceding NULL check and rely on boolean shortcut evaluation. As a side effect, this causes an alignment hole which we can close in a later patch after having cleaned up all fields of struct zfcp_fsf_req. Before: $ pahole -C zfcp_fsf_req drivers/s390/scsi/zfcp.ko ... u32 status; /* 136 4 */ u32 fsf_command; /* 140 4 */ struct fsf_qtcb * qtcb; /* 144 8 */ ... After: $ pahole -C zfcp_fsf_req drivers/s390/scsi/zfcp.ko ... u32 status; /* 136 4 */ /* XXX 4 bytes hole, try to pack */ struct fsf_qtcb * qtcb; /* 144 8 */ ... Signed-off-by: Steffen Maier Reviewed-by: Benjamin Block Signed-off-by: Martin K. Petersen --- drivers/s390/scsi/zfcp_dbf.c | 8 ++++---- drivers/s390/scsi/zfcp_dbf.h | 4 ++-- drivers/s390/scsi/zfcp_def.h | 7 +++++-- drivers/s390/scsi/zfcp_fsf.c | 14 ++++++-------- drivers/s390/scsi/zfcp_scsi.c | 4 +++- 5 files changed, 20 insertions(+), 17 deletions(-) (limited to 'drivers/s390/scsi') diff --git a/drivers/s390/scsi/zfcp_dbf.c b/drivers/s390/scsi/zfcp_dbf.c index 3b368fcf13f4..d20977bb27a4 100644 --- a/drivers/s390/scsi/zfcp_dbf.c +++ b/drivers/s390/scsi/zfcp_dbf.c @@ -81,7 +81,7 @@ void zfcp_dbf_hba_fsf_res(char *tag, int level, struct zfcp_fsf_req *req) rec->id = ZFCP_DBF_HBA_RES; rec->fsf_req_id = req->req_id; rec->fsf_req_status = req->status; - rec->fsf_cmd = req->fsf_command; + rec->fsf_cmd = q_head->fsf_command; rec->fsf_seq_no = req->seq_no; rec->u.res.req_issued = req->issued; rec->u.res.prot_status = q_pref->prot_status; @@ -94,7 +94,7 @@ void zfcp_dbf_hba_fsf_res(char *tag, int level, struct zfcp_fsf_req *req) memcpy(rec->u.res.fsf_status_qual, &q_head->fsf_status_qual, FSF_STATUS_QUALIFIER_SIZE); - if (req->fsf_command != FSF_QTCB_FCP_CMND) { + if (q_head->fsf_command != FSF_QTCB_FCP_CMND) { rec->pl_len = q_head->log_length; zfcp_dbf_pl_write(dbf, (char *)q_pref + q_head->log_start, rec->pl_len, "fsf_res", req->req_id); @@ -127,7 +127,7 @@ void zfcp_dbf_hba_fsf_uss(char *tag, struct zfcp_fsf_req *req) rec->id = ZFCP_DBF_HBA_USS; rec->fsf_req_id = req->req_id; rec->fsf_req_status = req->status; - rec->fsf_cmd = req->fsf_command; + rec->fsf_cmd = FSF_QTCB_UNSOLICITED_STATUS; if (!srb) goto log; @@ -174,7 +174,7 @@ void zfcp_dbf_hba_bit_err(char *tag, struct zfcp_fsf_req *req) rec->id = ZFCP_DBF_HBA_BIT; rec->fsf_req_id = req->req_id; rec->fsf_req_status = req->status; - rec->fsf_cmd = req->fsf_command; + rec->fsf_cmd = FSF_QTCB_UNSOLICITED_STATUS; memcpy(&rec->u.be, &sr_buf->payload.bit_error, sizeof(struct fsf_bit_error_payload)); diff --git a/drivers/s390/scsi/zfcp_dbf.h b/drivers/s390/scsi/zfcp_dbf.h index d116c07ed77a..b4438713d1cc 100644 --- a/drivers/s390/scsi/zfcp_dbf.h +++ b/drivers/s390/scsi/zfcp_dbf.h @@ -339,8 +339,8 @@ void zfcp_dbf_hba_fsf_response(struct zfcp_fsf_req *req) zfcp_dbf_hba_fsf_resp_suppress(req) ? 5 : 1, req); - } else if ((req->fsf_command == FSF_QTCB_OPEN_PORT_WITH_DID) || - (req->fsf_command == FSF_QTCB_OPEN_LUN)) { + } else if ((qtcb->header.fsf_command == FSF_QTCB_OPEN_PORT_WITH_DID) || + (qtcb->header.fsf_command == FSF_QTCB_OPEN_LUN)) { zfcp_dbf_hba_fsf_resp("fs_open", 4, req); } else if (qtcb->header.log_length) { diff --git a/drivers/s390/scsi/zfcp_def.h b/drivers/s390/scsi/zfcp_def.h index 572debf2f528..d65adb0ae9f1 100644 --- a/drivers/s390/scsi/zfcp_def.h +++ b/drivers/s390/scsi/zfcp_def.h @@ -277,7 +277,6 @@ static inline u64 zfcp_scsi_dev_lun(struct scsi_device *sdev) * @qdio_req: qdio queue related values * @completion: used to signal the completion of the request * @status: status of the request - * @fsf_command: FSF command issued * @qtcb: associated QTCB * @seq_no: sequence number of this request * @data: private data @@ -294,7 +293,6 @@ struct zfcp_fsf_req { struct zfcp_qdio_req qdio_req; struct completion completion; u32 status; - u32 fsf_command; struct fsf_qtcb *qtcb; u32 seq_no; void *data; @@ -311,4 +309,9 @@ int zfcp_adapter_multi_buffer_active(struct zfcp_adapter *adapter) return atomic_read(&adapter->status) & ZFCP_STATUS_ADAPTER_MB_ACT; } +static inline bool zfcp_fsf_req_is_status_read_buffer(struct zfcp_fsf_req *req) +{ + return req->qtcb == NULL; +} + #endif /* ZFCP_DEF_H */ diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c index 62311bd2df03..07b86375b461 100644 --- a/drivers/s390/scsi/zfcp_fsf.c +++ b/drivers/s390/scsi/zfcp_fsf.c @@ -84,13 +84,13 @@ static void zfcp_fsf_class_not_supp(struct zfcp_fsf_req *req) void zfcp_fsf_req_free(struct zfcp_fsf_req *req) { if (likely(req->pool)) { - if (likely(req->qtcb)) + if (likely(!zfcp_fsf_req_is_status_read_buffer(req))) mempool_free(req->qtcb, req->adapter->pool.qtcb_pool); mempool_free(req, req->pool); return; } - if (likely(req->qtcb)) + if (likely(!zfcp_fsf_req_is_status_read_buffer(req))) kmem_cache_free(zfcp_fsf_qtcb_cache, req->qtcb); kfree(req); } @@ -393,7 +393,7 @@ static void zfcp_fsf_protstatus_eval(struct zfcp_fsf_req *req) */ static void zfcp_fsf_req_complete(struct zfcp_fsf_req *req) { - if (unlikely(req->fsf_command == FSF_QTCB_UNSOLICITED_STATUS)) { + if (unlikely(zfcp_fsf_req_is_status_read_buffer(req))) { zfcp_fsf_status_read_handler(req); return; } @@ -710,7 +710,6 @@ static struct zfcp_fsf_req *zfcp_fsf_req_create(struct zfcp_qdio *qdio, init_completion(&req->completion); req->adapter = adapter; - req->fsf_command = fsf_cmd; req->req_id = adapter->req_no; if (likely(fsf_cmd != FSF_QTCB_UNSOLICITED_STATUS)) { @@ -729,10 +728,10 @@ static struct zfcp_fsf_req *zfcp_fsf_req_create(struct zfcp_qdio *qdio, req->qtcb->prefix.req_seq_no = adapter->fsf_req_seq_no; req->qtcb->prefix.req_id = req->req_id; req->qtcb->prefix.ulp_info = 26; - req->qtcb->prefix.qtcb_type = fsf_qtcb_type[req->fsf_command]; + req->qtcb->prefix.qtcb_type = fsf_qtcb_type[fsf_cmd]; req->qtcb->prefix.qtcb_version = FSF_QTCB_CURRENT_VERSION; req->qtcb->header.req_handle = req->req_id; - req->qtcb->header.fsf_command = req->fsf_command; + req->qtcb->header.fsf_command = fsf_cmd; } zfcp_qdio_req_init(adapter->qdio, &req->qdio_req, req->req_id, sbtype, @@ -745,7 +744,6 @@ static int zfcp_fsf_req_send(struct zfcp_fsf_req *req) { struct zfcp_adapter *adapter = req->adapter; struct zfcp_qdio *qdio = adapter->qdio; - int with_qtcb = (req->qtcb != NULL); int req_id = req->req_id; zfcp_reqlist_add(adapter->req_list, req); @@ -761,7 +759,7 @@ static int zfcp_fsf_req_send(struct zfcp_fsf_req *req) } /* Don't increase for unsolicited status */ - if (with_qtcb) + if (!zfcp_fsf_req_is_status_read_buffer(req)) adapter->fsf_req_seq_no++; adapter->req_no++; diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c index a8efcb330bc1..82dbe2677fc1 100644 --- a/drivers/s390/scsi/zfcp_scsi.c +++ b/drivers/s390/scsi/zfcp_scsi.c @@ -226,7 +226,9 @@ static void zfcp_scsi_forget_cmnd(struct zfcp_fsf_req *old_req, void *data) (struct zfcp_scsi_req_filter *)data; /* already aborted - prevent side-effects - or not a SCSI command */ - if (old_req->data == NULL || old_req->fsf_command != FSF_QTCB_FCP_CMND) + if (old_req->data == NULL || + zfcp_fsf_req_is_status_read_buffer(old_req) || + old_req->qtcb->header.fsf_command != FSF_QTCB_FCP_CMND) return; /* (tmf_scope == FCP_TMF_TGT_RESET || tmf_scope == FCP_TMF_LUN_RESET) */ -- cgit v1.2.3-59-g8ed1b