[haiku-commits] haiku: hrev53452 - src/add-ons/kernel/busses/scsi/virtio src/add-ons/kernel/drivers/network/virtio src/add-ons/kernel/bus_managers/virtio headers/private/virtio src/add-ons/kernel/drivers/disk/virtual/virtio_block

  • From: waddlesplash <waddlesplash@xxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Mon, 2 Sep 2019 10:30:38 -0400 (EDT)

hrev53452 adds 5 changesets to branch 'master'
old head: 03b52202d15def20288e2b5d0d5f930e75d167f7
new head: 66d6afec5a89464880e7793ad1b791cbee1efd8d
overview: 
https://git.haiku-os.org/haiku/log/?qt=range&q=66d6afec5a89+%5E03b52202d15d

----------------------------------------------------------------------------

d2fecacd0f94: virtio_scsi: Whitespace and line length cleanup only.
  
  Change-Id: I12efb32c9222b9397bc44be927f37700e030e89a
  Reviewed-on: https://review.haiku-os.org/c/haiku/+/1811
  Reviewed-by: Jérôme Duval <jerome.duval@xxxxxxxxx>

85fbdab0a61b: virtio_scsi: Remove unused fExpectsInterrupt and its spinlock.
  
  The value is never actually checked. The use of a condition variable
  makes it redundant since it will already either have a waiting entry,
  or not.
  
  Change-Id: Iafaecb7f9e56a2cf8932e05b82aad2b350fbce1f
  Reviewed-on: https://review.haiku-os.org/c/haiku/+/1812
  Reviewed-by: Jérôme Duval <jerome.duval@xxxxxxxxx>

26c0b5e70101: virtio_scsi: Fix theoretical use of possibly modified fCCB.
  
  As soon as the lock is released, fCCB may change due to a new Start().
  With the current use of the request class this could not happen, so
  this is purely theoretical.
  
  Change-Id: I6caee7f904f1864621aeef088e2bd611eb3b0a1f
  Reviewed-on: https://review.haiku-os.org/c/haiku/+/1813
  Reviewed-by: Adrien Destugues <pulkomandy@xxxxxxxxx>

9a2911ca8c15: virtio: Rework queue_dequeue to return a boolean.
  
  It previously returned the cookie directly, which made it impossible
  to distinguish between a NULL cookie and the function not having
  anything to dequeue. This lead to some code setting a cookie that was
  not actually used.
  
  Return the dequeue status as a boolean and provide the cookie with an
  optionally handed in pointer instead and adjust all users.
  
  Change-Id: Iaac1726ac4bc7ae42bb96b8f0915852b6def5822
  Reviewed-on: https://review.haiku-os.org/c/haiku/+/1814
  Reviewed-by: Jérôme Duval <jerome.duval@xxxxxxxxx>
  Reviewed-by: waddlesplash <waddlesplash@xxxxxxxxx>

66d6afec5a89: virtio_scsi: Abort requests on timeout.
  
  Previously the CCB would never complete when a timeout occured, leading
  to all further IO stopping.
  
  Abort the request by setting the CCB status to aborted and handing the
  CCB back to the SCSI layer. That one will then handle the error (and
  possibly retrying). This is similar to how such errors are handled in
  AHCI and the ATA stack.
  
  Since it is now possible that we receive a completion interrupt for an
  already aborted request, we need to keep track of what request the
  interrupts belong to and only notify when the current one completes. As
  there currently can't be multiple requests in flight, a simple counter
  is used.
  
  Change-Id: Ib80e146605efd2f81123803f424cc7f66f52a6c8
  Reviewed-on: https://review.haiku-os.org/c/haiku/+/1815
  Reviewed-by: waddlesplash <waddlesplash@xxxxxxxxx>

                                            [ Michael Lotz <mmlr@xxxxxxxx> ]

----------------------------------------------------------------------------

11 files changed, 74 insertions(+), 74 deletions(-)
headers/private/virtio/virtio.h                  |  3 +-
.../bus_managers/virtio/VirtioBalloonDevice.cpp  |  5 +--
.../kernel/bus_managers/virtio/VirtioModule.cpp  |  6 +--
.../kernel/bus_managers/virtio/VirtioPrivate.h   |  3 +-
.../kernel/bus_managers/virtio/VirtioQueue.cpp   | 11 +++--
.../kernel/busses/random/VirtioRNGDevice.cpp     |  4 +-
.../busses/scsi/virtio/VirtioSCSIController.cpp  | 42 ++++++++------------
.../busses/scsi/virtio/VirtioSCSIPrivate.h       | 14 ++++---
.../busses/scsi/virtio/VirtioSCSIRequest.cpp     | 20 ++++++++--
.../disk/virtual/virtio_block/virtio_block.cpp   |  2 +-
.../kernel/drivers/network/virtio/virtio_net.cpp | 38 +++++++-----------

############################################################################

Commit:      d2fecacd0f94bd1f4f418ca613c902fae2a5e8b2
URL:         https://git.haiku-os.org/haiku/commit/?id=d2fecacd0f94
Author:      Michael Lotz <mmlr@xxxxxxxx>
Date:        Sun Sep  1 00:10:31 2019 UTC
Committer:   waddlesplash <waddlesplash@xxxxxxxxx>
Commit-Date: Mon Sep  2 14:30:29 2019 UTC

virtio_scsi: Whitespace and line length cleanup only.

Change-Id: I12efb32c9222b9397bc44be927f37700e030e89a
Reviewed-on: https://review.haiku-os.org/c/haiku/+/1811
Reviewed-by: Jérôme Duval <jerome.duval@xxxxxxxxx>

----------------------------------------------------------------------------

diff --git a/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIPrivate.h 
b/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIPrivate.h
index 467532c917..b6455cf99b 100644
--- a/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIPrivate.h
+++ b/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIPrivate.h
@@ -22,7 +22,7 @@
 #      define TRACE(x...) ;
 #endif
 #define ERROR(x...)                    dprintf("\33[33mvirtio_scsi:\33[0m " x)
-#define CALLED()                       TRACE("CALLED %s\n", 
__PRETTY_FUNCTION__)
+#define CALLED()                       TRACE("CALLED %s\n", 
__PRETTY_FUNCTION__)
 
 extern device_manager_info* gDeviceManager;
 extern scsi_for_sim_interface *gSCSI;
@@ -65,14 +65,16 @@ private:
        static  void                            _RequestCallback(void* 
driverCookie,
                                                                        void 
*cookie);
                        void                            _RequestInterrupt();
-       static  void                            _EventCallback(void 
*driverCookie, void *cookie);
-                       void                            _EventInterrupt(struct 
virtio_scsi_event* event);
+       static  void                            _EventCallback(void 
*driverCookie,
+                                                                       void 
*cookie);
+                       void                            _EventInterrupt(
+                                                                       struct 
virtio_scsi_event* event);
        static  void                            _RescanChildBus(void *cookie);
 
                        void                            _SubmitEvent(uint32 
event);
 
                        device_node*            fNode;
-                       scsi_bus                        fBus;
+                       scsi_bus                        fBus;
 
                        virtio_device_interface* fVirtio;
                        virtio_device*          fVirtioDevice;
diff --git a/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIRequest.cpp 
b/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIRequest.cpp
index a7d5d39de7..079a3f4262 100644
--- a/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIRequest.cpp
+++ b/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIRequest.cpp
@@ -109,8 +109,8 @@ VirtioSCSIRequest::Finish(bool resubmit)
        } else if (fStatus == SCSI_REQ_CMP && fResponse->status != 0
                && HasSense()) {
                // when the request completed and has set sense
-       // data, report this to the scsi stack by setting
-       // CHECK CONDITION status
+               // data, report this to the scsi stack by setting
+               // CHECK CONDITION status
                TRACE("setting check condition\n");
 
                fCCB->subsys_status = SCSI_REQ_CMP_ERR;

############################################################################

Commit:      85fbdab0a61b6418c4cd9933f3de165c1250f9cf
URL:         https://git.haiku-os.org/haiku/commit/?id=85fbdab0a61b
Author:      Michael Lotz <mmlr@xxxxxxxx>
Date:        Sat Aug 31 23:38:37 2019 UTC
Committer:   waddlesplash <waddlesplash@xxxxxxxxx>
Commit-Date: Mon Sep  2 14:30:29 2019 UTC

virtio_scsi: Remove unused fExpectsInterrupt and its spinlock.

The value is never actually checked. The use of a condition variable
makes it redundant since it will already either have a waiting entry,
or not.

Change-Id: Iafaecb7f9e56a2cf8932e05b82aad2b350fbce1f
Reviewed-on: https://review.haiku-os.org/c/haiku/+/1812
Reviewed-by: Jérôme Duval <jerome.duval@xxxxxxxxx>

----------------------------------------------------------------------------

diff --git a/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIController.cpp 
b/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIController.cpp
index 8cf4cb3912..d27b9b5a53 100644
--- a/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIController.cpp
+++ b/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIController.cpp
@@ -41,7 +41,6 @@ VirtioSCSIController::VirtioSCSIController(device_node *node)
 {
        CALLED();
 
-       B_INITIALIZE_SPINLOCK(&fInterruptLock);
        fInterruptCondition.Init(this, "virtio scsi transfer");
 
        if (gSCSI->alloc_dpc(&fEventDPC) != B_OK)
@@ -231,11 +230,7 @@ VirtioSCSIController::ExecuteRequest(scsi_ccb *ccb)
        }
        fRequest->FillRequest(inCount, outCount, entries);
 
-       {
-               InterruptsSpinLocker locker(fInterruptLock);
-               fExpectsInterrupt = true;
-               fInterruptCondition.Add(&fInterruptConditionEntry);
-       }
+       fInterruptCondition.Add(&fInterruptConditionEntry);
 
        fVirtio->queue_request_v(fRequestVirtioQueue, entries,
                outCount, inCount, NULL);
@@ -243,11 +238,6 @@ VirtioSCSIController::ExecuteRequest(scsi_ccb *ccb)
        result = fInterruptConditionEntry.Wait(B_RELATIVE_TIMEOUT,
                fRequest->Timeout());
 
-       {
-               InterruptsSpinLocker locker(fInterruptLock);
-               fExpectsInterrupt = false;
-       }
-
        if (result != B_OK)
                return result;
 
@@ -295,7 +285,6 @@ VirtioSCSIController::_RequestCallback(void* driverCookie, 
void* cookie)
 void
 VirtioSCSIController::_RequestInterrupt()
 {
-       SpinLocker locker(fInterruptLock);
        fInterruptCondition.NotifyAll();
 }
 
diff --git a/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIPrivate.h 
b/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIPrivate.h
index b6455cf99b..78413e4d3a 100644
--- a/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIPrivate.h
+++ b/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIPrivate.h
@@ -91,10 +91,8 @@ private:
 
                        VirtioSCSIRequest*      fRequest;
 
-                       spinlock                        fInterruptLock;
                        ConditionVariable       fInterruptCondition;
                        ConditionVariableEntry fInterruptConditionEntry;
-                       bool                            fExpectsInterrupt;
 
                        scsi_dpc_cookie         fEventDPC;
                        struct virtio_scsi_event 
fEventBuffers[VIRTIO_SCSI_NUM_EVENTS];

############################################################################

Commit:      26c0b5e7010131d7bc8aa7a08065ddfc1e575295
URL:         https://git.haiku-os.org/haiku/commit/?id=26c0b5e70101
Author:      Michael Lotz <mmlr@xxxxxxxx>
Date:        Sat Aug 31 23:51:55 2019 UTC
Committer:   waddlesplash <waddlesplash@xxxxxxxxx>
Commit-Date: Mon Sep  2 14:30:29 2019 UTC

virtio_scsi: Fix theoretical use of possibly modified fCCB.

As soon as the lock is released, fCCB may change due to a new Start().
With the current use of the request class this could not happen, so
this is purely theoretical.

Change-Id: I6caee7f904f1864621aeef088e2bd611eb3b0a1f
Reviewed-on: https://review.haiku-os.org/c/haiku/+/1813
Reviewed-by: Adrien Destugues <pulkomandy@xxxxxxxxx>

----------------------------------------------------------------------------

diff --git a/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIRequest.cpp 
b/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIRequest.cpp
index 079a3f4262..ff4244b6b3 100644
--- a/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIRequest.cpp
+++ b/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIRequest.cpp
@@ -126,12 +126,13 @@ VirtioSCSIRequest::Finish(bool resubmit)
                }
        }
 
+       scsi_ccb *ccb = fCCB;
        mutex_unlock(&fLock);
 
        if (resubmit)
-               gSCSI->resubmit(fCCB);
+               gSCSI->resubmit(ccb);
        else
-               gSCSI->finished(fCCB, 1);
+               gSCSI->finished(ccb, 1);
 
        TRACE("VirtioSCSIRequest::Finish() done\n");
 

############################################################################

Commit:      9a2911ca8c155932fdd097fbe6c596ed2755305b
URL:         https://git.haiku-os.org/haiku/commit/?id=9a2911ca8c15
Author:      Michael Lotz <mmlr@xxxxxxxx>
Date:        Sun Sep  1 21:00:51 2019 UTC
Committer:   waddlesplash <waddlesplash@xxxxxxxxx>
Commit-Date: Mon Sep  2 14:30:29 2019 UTC

virtio: Rework queue_dequeue to return a boolean.

It previously returned the cookie directly, which made it impossible
to distinguish between a NULL cookie and the function not having
anything to dequeue. This lead to some code setting a cookie that was
not actually used.

Return the dequeue status as a boolean and provide the cookie with an
optionally handed in pointer instead and adjust all users.

Change-Id: Iaac1726ac4bc7ae42bb96b8f0915852b6def5822
Reviewed-on: https://review.haiku-os.org/c/haiku/+/1814
Reviewed-by: Jérôme Duval <jerome.duval@xxxxxxxxx>
Reviewed-by: waddlesplash <waddlesplash@xxxxxxxxx>

----------------------------------------------------------------------------

diff --git a/headers/private/virtio/virtio.h b/headers/private/virtio/virtio.h
index d53d1c2623..cb0c1760e6 100644
--- a/headers/private/virtio/virtio.h
+++ b/headers/private/virtio/virtio.h
@@ -134,7 +134,8 @@ typedef struct {
 
        uint16 (*queue_size)(virtio_queue queue);
 
-       void* (*queue_dequeue)(virtio_queue queue, uint32* _usedLength);
+       bool (*queue_dequeue)(virtio_queue queue, void** _cookie,
+               uint32* _usedLength);
 
 } virtio_device_interface;
 
diff --git a/src/add-ons/kernel/bus_managers/virtio/VirtioBalloonDevice.cpp 
b/src/add-ons/kernel/bus_managers/virtio/VirtioBalloonDevice.cpp
index d106f5bec9..e9d48112ed 100644
--- a/src/add-ons/kernel/bus_managers/virtio/VirtioBalloonDevice.cpp
+++ b/src/add-ons/kernel/bus_managers/virtio/VirtioBalloonDevice.cpp
@@ -188,14 +188,13 @@ VirtioBalloonDevice::_Thread()
 
                // alloc or release
                TRACE("queue request\n");
-               status_t result = fVirtio->queue_request(queue, &fEntry, NULL, 
-                       queue);
+               status_t result = fVirtio->queue_request(queue, &fEntry, NULL, 
NULL);
                if (result != B_OK) {
                        ERROR("queueing failed (%s)\n", strerror(result));
                        return result;
                }
 
-               while (fVirtio->queue_dequeue(queue, NULL) == NULL) {
+               while (!fVirtio->queue_dequeue(queue, NULL, NULL)) {
                        TRACE("wait for response\n");
                        queueConditionEntry.Wait(B_CAN_INTERRUPT);
                }
diff --git a/src/add-ons/kernel/bus_managers/virtio/VirtioModule.cpp 
b/src/add-ons/kernel/bus_managers/virtio/VirtioModule.cpp
index 753980e487..4ad32a019d 100644
--- a/src/add-ons/kernel/bus_managers/virtio/VirtioModule.cpp
+++ b/src/add-ons/kernel/bus_managers/virtio/VirtioModule.cpp
@@ -184,11 +184,11 @@ virtio_queue_size(virtio_queue _queue)
 }
 
 
-void*
-virtio_queue_dequeue(virtio_queue _queue, uint32* _usedLength)
+bool
+virtio_queue_dequeue(virtio_queue _queue, void** _cookie, uint32* _usedLength)
 {
        VirtioQueue *queue = (VirtioQueue *)_queue;
-       return queue->Dequeue(_usedLength);
+       return queue->Dequeue(_cookie, _usedLength);
 }
 
 
diff --git a/src/add-ons/kernel/bus_managers/virtio/VirtioPrivate.h 
b/src/add-ons/kernel/bus_managers/virtio/VirtioPrivate.h
index 9b4da99a14..5a1b7dbc3a 100644
--- a/src/add-ons/kernel/bus_managers/virtio/VirtioPrivate.h
+++ b/src/add-ons/kernel/bus_managers/virtio/VirtioPrivate.h
@@ -131,7 +131,8 @@ public:
                        void                            EnableInterrupt();
                        void                            DisableInterrupt();
 
-                       void*                           Dequeue(uint32* 
_usedLength = NULL);
+                       bool                            Dequeue(void** _cookie 
= NULL,
+                                                                       uint32* 
_usedLength = NULL);
 
 private:
                        void                            UpdateAvailable(uint16 
index);
diff --git a/src/add-ons/kernel/bus_managers/virtio/VirtioQueue.cpp 
b/src/add-ons/kernel/bus_managers/virtio/VirtioQueue.cpp
index 0220ab9385..aa5e07f35a 100644
--- a/src/add-ons/kernel/bus_managers/virtio/VirtioQueue.cpp
+++ b/src/add-ons/kernel/bus_managers/virtio/VirtioQueue.cpp
@@ -245,13 +245,13 @@ VirtioQueue::Interrupt()
 }
 
 
-void*
-VirtioQueue::Dequeue(uint32* _usedLength)
+bool
+VirtioQueue::Dequeue(void** _cookie, uint32* _usedLength)
 {
        TRACE("Dequeue() fRingUsedIndex: %u\n", fRingUsedIndex);
 
        if (fRingUsedIndex == fRing.used->idx)
-               return NULL;
+               return false;
 
        uint16 usedIndex = fRingUsedIndex++ & (fRingSize - 1);
        TRACE("Dequeue() usedIndex: %u\n", usedIndex);
@@ -261,6 +261,9 @@ VirtioQueue::Dequeue(uint32* _usedLength)
                *_usedLength = element->len;
 
        void* cookie = fDescriptors[descriptorIndex]->Cookie();
+       if (_cookie != NULL)
+               *_cookie = cookie;
+
        uint16 size = fDescriptors[descriptorIndex]->Size();
        if (size == 0)
                panic("VirtioQueue::Dequeue() size is zero\n");
@@ -283,7 +286,7 @@ VirtioQueue::Dequeue(uint32* _usedLength)
        fRingHeadIndex = descriptorIndex;
        TRACE("Dequeue() fRingHeadIndex: %u\n", fRingHeadIndex);
 
-       return cookie;
+       return true;
 }
 
 
diff --git a/src/add-ons/kernel/busses/random/VirtioRNGDevice.cpp 
b/src/add-ons/kernel/busses/random/VirtioRNGDevice.cpp
index f431e5b5ea..5c80c1ea84 100644
--- a/src/add-ons/kernel/busses/random/VirtioRNGDevice.cpp
+++ b/src/add-ons/kernel/busses/random/VirtioRNGDevice.cpp
@@ -95,7 +95,7 @@ VirtioRNGDevice::Read(void* _buffer, size_t* _numBytes)
                        fInterruptCondition.Add(&fInterruptConditionEntry);
                }
                status_t result = fVirtio->queue_request(fVirtioQueue, NULL, 
&fEntry,
-                       this);
+                       NULL);
                if (result != B_OK) {
                        ERROR("queueing failed (%s)\n", strerror(result));
                        return result;
@@ -131,7 +131,7 @@ VirtioRNGDevice::_RequestCallback(void* driverCookie, void* 
cookie)
 {
        VirtioRNGDevice* device = (VirtioRNGDevice*)driverCookie;
 
-       while (device->fVirtio->queue_dequeue(device->fVirtioQueue, NULL) != 
NULL)
+       while (device->fVirtio->queue_dequeue(device->fVirtioQueue, NULL, NULL))
                ;
 
        device->_RequestInterrupt();
diff --git a/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIController.cpp 
b/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIController.cpp
index d27b9b5a53..59fa80b6c4 100644
--- a/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIController.cpp
+++ b/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIController.cpp
@@ -275,7 +275,7 @@ VirtioSCSIController::_RequestCallback(void* driverCookie, 
void* cookie)
        VirtioSCSIController* controller = (VirtioSCSIController*)driverCookie;
 
        while (controller->fVirtio->queue_dequeue(
-                       controller->fRequestVirtioQueue, NULL) != NULL) {
+                       controller->fRequestVirtioQueue, NULL, NULL)) {
        }
 
        controller->_RequestInterrupt();
@@ -296,12 +296,9 @@ VirtioSCSIController::_EventCallback(void* driverCookie, 
void* cookie)
        CALLED();
        VirtioSCSIController* controller = (VirtioSCSIController*)driverCookie;
 
-       while (true) {
-               virtio_scsi_event* event = (virtio_scsi_event*)
-                       
controller->fVirtio->queue_dequeue(controller->fEventVirtioQueue,
-                               NULL);
-               if (event == NULL)
-                       break;
+       virtio_scsi_event* event = NULL;
+       while (controller->fVirtio->queue_dequeue(controller->fEventVirtioQueue,
+                       (void**)&event, NULL)) {
                controller->_EventInterrupt(event);
        }
 }
diff --git 
a/src/add-ons/kernel/drivers/disk/virtual/virtio_block/virtio_block.cpp 
b/src/add-ons/kernel/drivers/disk/virtual/virtio_block/virtio_block.cpp
index 65149d1886..d16f054172 100644
--- a/src/add-ons/kernel/drivers/disk/virtual/virtio_block/virtio_block.cpp
+++ b/src/add-ons/kernel/drivers/disk/virtual/virtio_block/virtio_block.cpp
@@ -189,7 +189,7 @@ virtio_block_callback(void* driverCookie, void* cookie)
        virtio_block_driver_info* info = (virtio_block_driver_info*)cookie;
 
        // consume all queued elements
-       while (info->virtio->queue_dequeue(info->virtio_queue, NULL) != NULL)
+       while (info->virtio->queue_dequeue(info->virtio_queue, NULL, NULL))
                ;
 
        release_sem_etc(info->sem_cb, 1, B_DO_NOT_RESCHEDULE);
diff --git a/src/add-ons/kernel/drivers/network/virtio/virtio_net.cpp 
b/src/add-ons/kernel/drivers/network/virtio/virtio_net.cpp
index 336487292f..d6af769a6f 100644
--- a/src/add-ons/kernel/drivers/network/virtio/virtio_net.cpp
+++ b/src/add-ons/kernel/drivers/network/virtio/virtio_net.cpp
@@ -171,26 +171,15 @@ get_feature_name(uint32 feature)
 static status_t
 virtio_net_drain_queues(virtio_net_driver_info* info)
 {
-       while (true) {
-               BufInfo* buf = (BufInfo*)info->virtio->queue_dequeue(
-                       info->txQueues[0], NULL);
-               if (buf == NULL)
-                       break;
+       BufInfo* buf = NULL;
+       while (info->virtio->queue_dequeue(info->txQueues[0], (void**)&buf, 
NULL))
                info->txFreeList.Add(buf);
-       }
 
-       while (true) {
-               BufInfo* buf = (BufInfo*)info->virtio->queue_dequeue(
-                       info->rxQueues[0], NULL);
-               if (buf == NULL)
-                       break;
-       }
+       while (info->virtio->queue_dequeue(info->rxQueues[0], NULL, NULL))
+               ;
 
-       while (true) {
-               BufInfo* buf = info->rxFullList.RemoveHead();
-               if (buf == NULL)
-                       break;
-       }
+       while (info->rxFullList.RemoveHead() != NULL)
+               ;
 
        return B_OK;
 }
@@ -565,10 +554,11 @@ virtio_net_read(void* cookie, off_t pos, void* buffer, 
size_t* _length)
                mutex_lock(&info->rxLock);
                while (info->rxDone != -1) {
                        uint32 usedLength = 0;
-                       BufInfo* buf = (BufInfo*)info->virtio->queue_dequeue(
-                               info->rxQueues[0], &usedLength);
-                       if (buf == NULL)
+                       BufInfo* buf = NULL;
+                       if (!info->virtio->queue_dequeue(info->rxQueues[0], 
(void**)&buf,
+                                       &usedLength) || buf == NULL) {
                                break;
+                       }
 
                        buf->rxUsedLength = usedLength;
                        info->rxFullList.Add(buf);
@@ -622,10 +612,12 @@ virtio_net_write(void* cookie, off_t pos, const void* 
buffer,
 
                mutex_lock(&info->txLock);
                while (info->txDone != -1) {
-                       BufInfo* buf = (BufInfo*)info->virtio->queue_dequeue(
-                               info->txQueues[0], NULL);
-                       if (buf == NULL)
+                       BufInfo* buf = NULL;
+                       if (!info->virtio->queue_dequeue(info->txQueues[0], 
(void**)&buf,
+                                       NULL) || buf == NULL) {
                                break;
+                       }
+
                        info->txFreeList.Add(buf);
                }
        }

############################################################################

Revision:    hrev53452
Commit:      66d6afec5a89464880e7793ad1b791cbee1efd8d
URL:         https://git.haiku-os.org/haiku/commit/?id=66d6afec5a89
Author:      Michael Lotz <mmlr@xxxxxxxx>
Date:        Sat Aug 31 23:57:52 2019 UTC
Committer:   waddlesplash <waddlesplash@xxxxxxxxx>
Commit-Date: Mon Sep  2 14:30:29 2019 UTC

virtio_scsi: Abort requests on timeout.

Previously the CCB would never complete when a timeout occured, leading
to all further IO stopping.

Abort the request by setting the CCB status to aborted and handing the
CCB back to the SCSI layer. That one will then handle the error (and
possibly retrying). This is similar to how such errors are handled in
AHCI and the ATA stack.

Since it is now possible that we receive a completion interrupt for an
already aborted request, we need to keep track of what request the
interrupts belong to and only notify when the current one completes. As
there currently can't be multiple requests in flight, a simple counter
is used.

Change-Id: Ib80e146605efd2f81123803f424cc7f66f52a6c8
Reviewed-on: https://review.haiku-os.org/c/haiku/+/1815
Reviewed-by: waddlesplash <waddlesplash@xxxxxxxxx>

----------------------------------------------------------------------------

diff --git a/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIController.cpp 
b/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIController.cpp
index 59fa80b6c4..5082f1342f 100644
--- a/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIController.cpp
+++ b/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIController.cpp
@@ -37,6 +37,7 @@ VirtioSCSIController::VirtioSCSIController(device_node *node)
        fVirtioDevice(NULL),
        fStatus(B_NO_INIT),
        fRequest(NULL),
+       fCurrentRequest(0),
        fEventDPC(NULL)
 {
        CALLED();
@@ -230,16 +231,20 @@ VirtioSCSIController::ExecuteRequest(scsi_ccb *ccb)
        }
        fRequest->FillRequest(inCount, outCount, entries);
 
+       atomic_add(&fCurrentRequest, 1);
        fInterruptCondition.Add(&fInterruptConditionEntry);
 
        fVirtio->queue_request_v(fRequestVirtioQueue, entries,
-               outCount, inCount, NULL);
+               outCount, inCount, (void *)(addr_t)fCurrentRequest);
 
        result = fInterruptConditionEntry.Wait(B_RELATIVE_TIMEOUT,
                fRequest->Timeout());
 
-       if (result != B_OK)
+       if (result != B_OK) {
+               ERROR("wait failed with status: %#" B_PRIx32 "\n", result);
+               fRequest->Abort();
                return result;
+       }
 
        return fRequest->Finish(false);
 }
@@ -273,11 +278,6 @@ VirtioSCSIController::_RequestCallback(void* driverCookie, 
void* cookie)
 {
        CALLED();
        VirtioSCSIController* controller = (VirtioSCSIController*)driverCookie;
-
-       while (controller->fVirtio->queue_dequeue(
-                       controller->fRequestVirtioQueue, NULL, NULL)) {
-       }
-
        controller->_RequestInterrupt();
 }
 
@@ -285,7 +285,11 @@ VirtioSCSIController::_RequestCallback(void* driverCookie, 
void* cookie)
 void
 VirtioSCSIController::_RequestInterrupt()
 {
-       fInterruptCondition.NotifyAll();
+       void* cookie = NULL;
+       while (fVirtio->queue_dequeue(fRequestVirtioQueue, &cookie, NULL)) {
+               if ((int32)(addr_t)cookie == atomic_get(&fCurrentRequest))
+                       fInterruptCondition.NotifyAll();
+       }
 }
 
 
diff --git a/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIPrivate.h 
b/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIPrivate.h
index 78413e4d3a..a7afe88552 100644
--- a/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIPrivate.h
+++ b/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIPrivate.h
@@ -91,6 +91,7 @@ private:
 
                        VirtioSCSIRequest*      fRequest;
 
+                       int32                           fCurrentRequest;
                        ConditionVariable       fInterruptCondition;
                        ConditionVariableEntry fInterruptConditionEntry;
 
@@ -123,6 +124,7 @@ public:
                                                                        { 
return fCCB->data_length > 0; }
 
                        status_t                        Finish(bool resubmit);
+                       void                            Abort();
 
                        // SCSI stuff
                        status_t                        Start(scsi_ccb *ccb);
diff --git a/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIRequest.cpp 
b/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIRequest.cpp
index ff4244b6b3..49071609e7 100644
--- a/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIRequest.cpp
+++ b/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIRequest.cpp
@@ -140,6 +140,17 @@ VirtioSCSIRequest::Finish(bool resubmit)
 }
 
 
+void
+VirtioSCSIRequest::Abort()
+{
+       scsi_ccb *ccb = fCCB;
+       mutex_unlock(&fLock);
+
+       ccb->subsys_status = SCSI_REQ_ABORTED;
+       gSCSI->finished(ccb, 1);
+}
+
+
 void
 VirtioSCSIRequest::RequestSense()
 {


Other related posts:

  • » [haiku-commits] haiku: hrev53452 - src/add-ons/kernel/busses/scsi/virtio src/add-ons/kernel/drivers/network/virtio src/add-ons/kernel/bus_managers/virtio headers/private/virtio src/add-ons/kernel/drivers/disk/virtual/virtio_block - waddlesplash