Skip to content

Commit

Permalink
[vioscsi] SendSRB minor refactor
Browse files Browse the repository at this point in the history
The virtqueue_add_buf() routine will return 0 on SUCCESS or otherwise
a negative number, usually -28 (ENOSPC), i.e no space for buffer. An
inline comment is added for edification. Refactored virtqueue_add_buf()
handling to only process the SRB if virtqueue_add_buf() returns SUCCESS.
Presently, other positive return codes would be processed.

Variable renames:
res to add_buffer_req_status
index to vq_req_idx
MessageID to MessageId
 -- Also cleaned up variable init table

New definitions:
VQ_ADD_BUFFER_SUCCESS is defined in vioscsi\helper.h header file.
 -- Used to initialize add_buffer_req_status variable. (Credit: @JonKohler)

To ensure valid data is reported we also move the trace event on
failure to above the call to CompleteRequest() and wrap the line for
improved readability.
 -- Add add_buffer_req_status content to trace output on error (Credit: @JonKohler)

Signed-off-by: benyamin-codez <115509179+benyamin-codez@users.noreply.github.com>
  • Loading branch information
benyamin-codez authored and vrozenfe committed Oct 14, 2024
1 parent bd6caf9 commit c9ed8eb
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 14 deletions.
32 changes: 18 additions & 14 deletions vioscsi/helper.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,11 @@ SendSRB(
STOR_LOCK_HANDLE LockHandle = { 0 };
ULONG status = STOR_STATUS_SUCCESS;
UCHAR ScsiStatus = SCSISTAT_GOOD;
ULONG MessageID;
int res = 0;
ULONG MessageId;
INT add_buffer_req_status = VQ_ADD_BUFFER_SUCCESS;
PREQUEST_LIST element;
ULONG index;
ULONG vq_req_idx;

ENTER_FN_SRB();

if (!Srb)
Expand Down Expand Up @@ -93,8 +94,8 @@ ENTER_FN_SRB();
return;
}

MessageID = QUEUE_TO_MESSAGE(QueueNumber);
index = QueueNumber - VIRTIO_SCSI_REQUEST_QUEUE_0;
MessageId = QUEUE_TO_MESSAGE(QueueNumber);
vq_req_idx = QueueNumber - VIRTIO_SCSI_REQUEST_QUEUE_0;

if (adaptExt->reset_in_progress) {
RhelDbgPrint(TRACE_LEVEL_FATAL, " Reset is in progress, completing SRB 0x%p with SRB_STATUS_BUS_RESET.\n", Srb);
Expand All @@ -103,27 +104,30 @@ ENTER_FN_SRB();
return;
}

VioScsiVQLock(DeviceExtension, MessageID, &LockHandle, FALSE);
VioScsiVQLock(DeviceExtension, MessageId, &LockHandle, FALSE);
SET_VA_PA();
res = virtqueue_add_buf(adaptExt->vq[QueueNumber],
srbExt->psgl,
srbExt->out, srbExt->in,
&srbExt->cmd, va, pa);
add_buffer_req_status = virtqueue_add_buf(adaptExt->vq[QueueNumber],
srbExt->psgl,
srbExt->out, srbExt->in,
&srbExt->cmd, va, pa);

if (res >= 0) {
if (add_buffer_req_status == VQ_ADD_BUFFER_SUCCESS) {
notify = virtqueue_kick_prepare(adaptExt->vq[QueueNumber]);
element = &adaptExt->processing_srbs[index];
element = &adaptExt->processing_srbs[vq_req_idx];
InsertTailList(&element->srb_list, &srbExt->list_entry);
element->srb_cnt++;
} else {
// virtqueue_add_buf() returned -28 (ENOSPC), i.e. no space for buffer, or some other error
ScsiStatus = SCSISTAT_QUEUE_FULL;
SRB_SET_SRB_STATUS(Srb, SRB_STATUS_BUSY);
SRB_SET_SCSI_STATUS(Srb, ScsiStatus);
StorPortBusy(DeviceExtension, 10);
RhelDbgPrint(TRACE_LEVEL_WARNING,
" Could not put an SRB into a VQ due to error %s (%i). To be completed with SRB_STATUS_BUSY. QueueNumber = %lu, SRB = 0x%p, Lun = %d, TimeOut = %d.\n",
(add_buffer_req_status == -ENOSPC) ? "ENOSPC" : "UNKNOWN", add_buffer_req_status, QueueNumber, srbExt->Srb, SRB_LUN(Srb), Srb->TimeOutValue);
CompleteRequest(DeviceExtension, Srb);
RhelDbgPrint(TRACE_LEVEL_FATAL, " Could not put an SRB into a VQ, so complete it with SRB_STATUS_BUSY. QueueNumber = %d, SRB = 0x%p, Lun = %d, TimeOut = %d.\n", QueueNumber, srbExt->Srb, SRB_LUN(Srb), Srb->TimeOutValue);
}
VioScsiVQUnlock(DeviceExtension, MessageID, &LockHandle, FALSE);
VioScsiVQUnlock(DeviceExtension, MessageId, &LockHandle, FALSE);
if (notify){
virtqueue_notify(adaptExt->vq[QueueNumber]);
}
Expand Down
2 changes: 2 additions & 0 deletions vioscsi/helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@
#define CACHE_LINE_SIZE 64
#define ROUND_TO_CACHE_LINES(Size) (((ULONG_PTR)(Size) + CACHE_LINE_SIZE - 1) & ~(CACHE_LINE_SIZE - 1))

#define VQ_ADD_BUFFER_SUCCESS 0

#include <srbhelper.h>

// Note: SrbGetCdbLength is defined in srbhelper.h
Expand Down

0 comments on commit c9ed8eb

Please sign in to comment.