From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH-v2 11/12] xen-scsiback: Convert to percpu_ida tag allocation Date: Wed, 27 Jan 2016 11:57:39 +0100 Message-ID: <56A8A2A3.30407@suse.com> References: <1453709466-6308-1-git-send-email-nab@daterainc.com> <1453709466-6308-12-git-send-email-nab@daterainc.com> <56A7401E.7070005@suse.com> <1453876090.6746.123.camel@haakon3.risingtidesystems.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1453876090.6746.123.camel@haakon3.risingtidesystems.com> Sender: target-devel-owner@vger.kernel.org To: "Nicholas A. Bellinger" Cc: "Nicholas A. Bellinger" , target-devel , linux-scsi , Christoph Hellwig , Hannes Reinecke , Mike Christie , Sagi Grimberg , Andy Grover , Sebastian Andrzej Siewior , Andrzej Pietrasiewicz , Chris Boot , David Vrabel List-Id: linux-scsi@vger.kernel.org On 27/01/16 07:28, Nicholas A. Bellinger wrote: > On Tue, 2016-01-26 at 10:45 +0100, Juergen Gross wrote: >> On 25/01/16 09:11, Nicholas A. Bellinger wrote: >>> From: Nicholas Bellinger >>> >>> Cc: Juergen Gross >>> Cc: Hannes Reinecke >>> Cc: David Vrabel >>> Signed-off-by: Nicholas Bellinger >>> --- >>> drivers/xen/xen-scsiback.c | 163 ++++++++++++++++++++++++--------------------- >>> 1 file changed, 87 insertions(+), 76 deletions(-) >>> >>> diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c >>> index 594f8a7..640fb22 100644 >>> --- a/drivers/xen/xen-scsiback.c >>> +++ b/drivers/xen/xen-scsiback.c >>> @@ -190,7 +190,6 @@ module_param_named(max_buffer_pages, scsiback_max_buffer_pages, int, 0644); >>> MODULE_PARM_DESC(max_buffer_pages, >>> "Maximum number of free pages to keep in backend buffer"); >>> >>> -static struct kmem_cache *scsiback_cachep; >>> static DEFINE_SPINLOCK(free_pages_lock); >>> static int free_pages_num; >>> static LIST_HEAD(scsiback_free_pages); >>> @@ -322,7 +321,8 @@ static void scsiback_free_translation_entry(struct kref *kref) >>> } >>> >>> static void scsiback_do_resp_with_sense(char *sense_buffer, int32_t result, >>> - uint32_t resid, struct vscsibk_pend *pending_req) >>> + uint32_t resid, struct vscsibk_pend *pending_req, >>> + uint16_t rqid) >>> { >>> struct vscsiif_response *ring_res; >>> struct vscsibk_info *info = pending_req->info; >> >> pending_req might be NULL now, so this will panic the system. >> > > Thanks for the review. > > Added the following to propagate up original *info into > scsiback_do_resp_with_sense() to address the early pending_req > failure case. > > https://git.kernel.org/cgit/linux/kernel/git/nab/target-pending.git/commit/?h=queue-next&id=5873f22a9b7c7aa16ff9a85074a07b739f1d06a5 Hmm, wouldn't it make more sense to split scsiback_do_resp_with_sense() into two functions now? Something like: diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c index ad4eb10..0d71467 100644 --- a/drivers/xen/xen-scsiback.c +++ b/drivers/xen/xen-scsiback.c @@ -321,11 +321,10 @@ static void scsiback_free_translation_entry(struct kref *kref) kfree(entry); } -static void scsiback_do_resp_with_sense(char *sense_buffer, int32_t result, - uint32_t resid, struct vscsibk_pend *pending_req) +static void scsiback_send_response(struct vscsibk_info *info, + char *sense_buffer, int32_t result, uint32_t resid, uint16_t rqid) { struct vscsiif_response *ring_res; - struct vscsibk_info *info = pending_req->info; int notify; struct scsi_sense_hdr sshdr; unsigned long flags; @@ -337,7 +336,7 @@ static void scsiback_do_resp_with_sense(char *sense_buffer, int32_t result, info->ring.rsp_prod_pvt++; ring_res->rslt = result; - ring_res->rqid = pending_req->rqid; + ring_res->rqid = rqid; if (sense_buffer != NULL && scsi_normalize_sense(sense_buffer, VSCSIIF_SENSE_BUFFERSIZE, @@ -357,6 +356,13 @@ static void scsiback_do_resp_with_sense(char *sense_buffer, int32_t result, if (notify) notify_remote_via_irq(info->irq); +} + +static void scsiback_do_resp_with_sense(char *sense_buffer, int32_t result, + uint32_t resid, struct vscsibk_pend *pending_req) +{ + scsiback_send_response(pending_req->info, sense_buffer, result, + resid, pending_req->rqid); if (pending_req->v2p) kref_put(&pending_req->v2p->kref, And then call scsiback_send_response() directly in case pending_req is NULL. Juergen