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 <nab@xxxxxxxxxxxxxxx>
>>>
>>> Cc: Juergen Gross <jgross@xxxxxxxx>
>>> Cc: Hannes Reinecke <hare@xxxxxxx>
>>> Cc: David Vrabel <david.vrabel@xxxxxxxxxx>
>>> Signed-off-by: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx>
>>> ---
>>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
