[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 13/21] scsi: do not call send_command directl
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v3 13/21] scsi: do not call send_command directly |
Date: |
Tue, 24 May 2011 15:05:11 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.15) Gecko/20101027 Fedora/3.0.10-1.fc12 Thunderbird/3.0.10 |
Am 20.05.2011 19:43, schrieb Paolo Bonzini:
> On 05/20/2011 06:04 PM, Christoph Hellwig wrote:
>>> -void scsi_req_enqueue(SCSIRequest *req)
>>> +int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf)
>>> {
>>> + int32_t rc;
>>> assert(!req->enqueued);
>>> scsi_req_ref(req);
>>> req->enqueued = true;
>>> QTAILQ_INSERT_TAIL(&req->dev->requests, req, next);
>>> +
>>> + /* Make sure the request doesn't disappear under send_command's feet.
>>> */
>>> + scsi_req_ref(req);
>>> + rc = req->dev->info->send_command(req, buf);
>>> + scsi_req_unref(req);
>>> + return rc;
>>
>> How would it disappear given that we grabbed another reference just before?
>
> Welcome to the wonderful world of nested callbacks. :(
>
> Suppose send_command completes a request. scsi_req_complete then
> dequeues it (which undoes the reference above), and calls the device who
> owned it. The device who owned the request then presumably NULLs a
> pointer and drops the last reference. The request is then freed.
Maybe the callback should be done from a BH then? It sounds like this
could cause more bugs than what you're fixing here.
Kevin
- Re: [Qemu-devel] [PATCH v3 14/21] scsi: introduce scsi_req_new, (continued)
[Qemu-devel] [PATCH v3 16/21] scsi: introduce scsi_req_get_buf, Paolo Bonzini, 2011/05/17
[Qemu-devel] [PATCH v3 15/21] scsi: introduce scsi_req_kick, Paolo Bonzini, 2011/05/17
[Qemu-devel] [PATCH v3 19/21] scsi: make write_data return void, Paolo Bonzini, 2011/05/17
[Qemu-devel] [PATCH v3 18/21] scsi-disk: add data direction checking, Paolo Bonzini, 2011/05/17