From 7746364088560b850a16ca95ee6441cf50774d6f Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Tue, 5 Nov 2024 13:43:13 -0500 Subject: [PATCH] SCSI: Use an object_cache for the CCB pool. Same as for the scatter/gather pool. This also changes CCBs to use condition variables instead of semaphores for completion, which means they're now C++-only. A few C files still include , but none use CCBs directly, so this works out fine. A quick check with a compile benchmark didn't show a performance regression. --- headers/os/drivers/bus/SCSI.h | 26 +++---- .../kernel/bus_managers/scsi/busses.cpp | 16 ++-- src/add-ons/kernel/bus_managers/scsi/ccb.cpp | 74 ++++++------------- .../kernel/bus_managers/scsi/scsi_internal.h | 8 +- .../kernel/bus_managers/scsi/scsi_io.cpp | 14 ++-- .../kernel/busses/scsi/ahci/ahci_defs.h | 1 - src/add-ons/kernel/generic/scsi_periph/io.cpp | 5 +- 7 files changed, 56 insertions(+), 88 deletions(-) diff --git a/headers/os/drivers/bus/SCSI.h b/headers/os/drivers/bus/SCSI.h index 2dfe0eac4b..be3a8ca26e 100644 --- a/headers/os/drivers/bus/SCSI.h +++ b/headers/os/drivers/bus/SCSI.h @@ -94,17 +94,18 @@ #include + +#ifdef _KERNEL_MODE +#include #include +#endif -#define SCSI_MAX_CDB_SIZE 16 // max size of cdb -#define SCSI_MAX_SENSE_SIZE 64 // max size of sense data - -// bus/device handle typedef struct scsi_bus_info *scsi_bus; typedef struct scsi_device_info *scsi_device; +#if defined(__cplusplus) && defined(_KERNEL_MODE) // structure of one scsi i/o CCB (command control block) typedef struct scsi_ccb { struct scsi_ccb *next, *prev; // internal @@ -117,11 +118,10 @@ typedef struct scsi_ccb { uchar target_lun; // Target LUN number uint32 flags; // Flags for operation of the subsystem - // released once after asynchronous execution of request; - // initialised by alloc_ccb, can be replaced for action but - // must be restored before returning via free_ccb - sem_id completion_sem; + // notified after asynchronous execution of request + ConditionVariable completion_cond; +#define SCSI_MAX_CDB_SIZE 16 uint8 cdb[SCSI_MAX_CDB_SIZE]; // command data block uchar cdb_length; // length of command in bytes int64 sort; // value of command to sort on (<0 means n/a) @@ -134,6 +134,7 @@ typedef struct scsi_ccb { int32 data_resid; // data transfer residual length: 2's comp void *io_operation; +#define SCSI_MAX_SENSE_SIZE 64 uchar sense[SCSI_MAX_SENSE_SIZE]; // autosense data uchar sense_resid; // autosense resid length: 2's comp @@ -154,6 +155,9 @@ typedef struct scsi_ccb { uint16 orig_sg_count; uint32 orig_data_length; } scsi_ccb; +#else +typedef struct scsi_ccb scsi_ccb; +#endif // Defines for the subsystem status field @@ -291,15 +295,11 @@ typedef struct { typedef struct scsi_device_interface { driver_module_info info; - // get CCB - // warning: if pool of CCBs is exhausted, this call is delayed until a - // CCB is freed, so don't try to allocate more then one CCB at once! scsi_ccb *(*alloc_ccb)(scsi_device device); - // free CCB void (*free_ccb)(scsi_ccb *ccb); // execute command asynchronously - // when it's finished, the semaphore of the ccb is released + // when it's finished, the condvar of the ccb is released // you must provide a S/G list if data_len != 0 void (*async_io)(scsi_ccb *ccb); // execute command synchronously diff --git a/src/add-ons/kernel/bus_managers/scsi/busses.cpp b/src/add-ons/kernel/bus_managers/scsi/busses.cpp index 14a09c6310..16ca1c94b4 100644 --- a/src/add-ons/kernel/bus_managers/scsi/busses.cpp +++ b/src/add-ons/kernel/bus_managers/scsi/busses.cpp @@ -135,10 +135,6 @@ scsi_create_bus(device_node *node, uint8 path_id) mutex_init(&bus->mutex, "scsi_bus_mutex"); spinlock_irq_init(&bus->dpc_lock); - res = scsi_init_ccb_alloc(bus); - if (res < B_OK) - goto err2; - bus->service_thread = spawn_kernel_thread(scsi_service_threadproc, "scsi_bus_service", BUS_SERVICE_PRIORITY, bus); @@ -152,8 +148,6 @@ scsi_create_bus(device_node *node, uint8 path_id) return bus; err1: - scsi_uninit_ccb_alloc(bus); -err2: mutex_destroy(&bus->mutex); delete_sem(bus->start_service); err4: @@ -167,20 +161,17 @@ err6: static status_t scsi_destroy_bus(scsi_bus_info *bus) { - int32 retcode; - // noone is using this bus now, time to clean it up bus->shutting_down = true; release_sem(bus->start_service); + status_t retcode; wait_for_thread(bus->service_thread, &retcode); delete_sem(bus->start_service); mutex_destroy(&bus->mutex); delete_sem(bus->scan_lun_lock); - scsi_uninit_ccb_alloc(bus); - return B_OK; } @@ -305,6 +296,10 @@ static status_t scsi_bus_module_init(void) { SHOW_FLOW0(4, ""); + + status_t status = init_ccb_alloc(); + if (status != B_OK) + return status; return init_temp_sg(); } @@ -314,6 +309,7 @@ scsi_bus_module_uninit(void) { SHOW_INFO0(4, ""); + uninit_ccb_alloc(); uninit_temp_sg(); return B_OK; } diff --git a/src/add-ons/kernel/bus_managers/scsi/ccb.cpp b/src/add-ons/kernel/bus_managers/scsi/ccb.cpp index 2695b17fb4..40a4d76816 100644 --- a/src/add-ons/kernel/bus_managers/scsi/ccb.cpp +++ b/src/add-ons/kernel/bus_managers/scsi/ccb.cpp @@ -1,7 +1,8 @@ /* -** Copyright 2002/03, Thomas Kurschel. All rights reserved. -** Distributed under the terms of the MIT License. -*/ + * Copyright 2002-2003, Thomas Kurschel. All rights reserved. + * Copyright 2024, Haiku, Inc. All rights reserved. + * Distributed under the terms of the MIT License. + */ /* Part of Open SCSI bus manager @@ -14,24 +15,24 @@ #include "scsi_internal.h" +#include -// ccb are relatively large, so don't make it too small to not waste memory -#define CCB_CHUNK_SIZE 16*1024 -// maximum number of CCBs - probably, we want to make that editable -// it must be at least 1 for normal use and 1 for stand-by autosense request -#define CCB_NUM_MAX 128 +static object_cache* sCcbPool = NULL; scsi_ccb * scsi_alloc_ccb(scsi_device_info *device) { - scsi_ccb *ccb; - SHOW_FLOW0( 3, "" ); - ccb = (scsi_ccb *)locked_pool->alloc(device->bus->ccb_pool); + scsi_ccb* ccb = (scsi_ccb*)object_cache_alloc(sCcbPool, 0); + ccb->completion_cond.Init(ccb, "scsi ccb"); + + ccb->bus = device->bus; + ccb->path_id = device->bus->path_id; + ccb->state = SCSI_STATE_FINISHED; ccb->device = device; ccb->target_id = device->target_id; @@ -57,59 +58,26 @@ scsi_free_ccb(scsi_ccb *ccb) if (ccb->state != SCSI_STATE_FINISHED) panic("Tried to free ccb that's still in use (state %d)\n", ccb->state); - ccb->state = SCSI_STATE_FREE; - - locked_pool->free(ccb->bus->ccb_pool, ccb); -} - - -static status_t -ccb_low_alloc_hook(void *block, void *arg) -{ - scsi_ccb *ccb = (scsi_ccb *)block; - scsi_bus_info *bus = (scsi_bus_info *)arg; - status_t res; - - ccb->bus = bus; - ccb->path_id = bus->path_id; - ccb->state = SCSI_STATE_FREE; - - if ((res = ccb->completion_sem = create_sem(0, "ccb_sem")) < 0) - return res; - - return B_OK; -} - - -static void -ccb_low_free_hook(void *block, void *arg) -{ - scsi_ccb *ccb = (scsi_ccb *)block; - - delete_sem(ccb->completion_sem); + object_cache_free(sCcbPool, ccb, 0); } status_t -scsi_init_ccb_alloc(scsi_bus_info *bus) +init_ccb_alloc() { - // initially, we want no CCB allocated as the path_id of - // the bus is not ready yet so the CCB cannot be initialized - // correctly - bus->ccb_pool = locked_pool->create(sizeof(scsi_ccb), sizeof(uint32) - 1, 0, - CCB_CHUNK_SIZE, CCB_NUM_MAX, 0, "scsi_ccb_pool", B_CONTIGUOUS, - ccb_low_alloc_hook, ccb_low_free_hook, bus); - - if (bus->ccb_pool == NULL) + sCcbPool = create_object_cache("scsi ccb", sizeof(scsi_ccb), 0, NULL, NULL, NULL); + if (sCcbPool == NULL) return B_NO_MEMORY; + // it must be at least 1 for normal use and 1 for stand-by autosense request + object_cache_set_minimum_reserve(sCcbPool, 2); + return B_OK; } void -scsi_uninit_ccb_alloc(scsi_bus_info *bus) +uninit_ccb_alloc() { - locked_pool->destroy(bus->ccb_pool); + delete_object_cache(sCcbPool); } - diff --git a/src/add-ons/kernel/bus_managers/scsi/scsi_internal.h b/src/add-ons/kernel/bus_managers/scsi/scsi_internal.h index a17a6d4872..f0ca8d6792 100644 --- a/src/add-ons/kernel/bus_managers/scsi/scsi_internal.h +++ b/src/add-ons/kernel/bus_managers/scsi/scsi_internal.h @@ -110,8 +110,6 @@ typedef struct scsi_bus_info { struct scsi_device_info *waiting_devices; // devices ready to receive requests - locked_pool_cookie ccb_pool; // ccb pool (one per bus) - device_node *node; // pnp node of bus struct dma_params dma_params; // dma restrictions of controller @@ -210,7 +208,7 @@ enum { // state of ccb enum { - SCSI_STATE_FREE = 0, + SCSI_STATE_INVALID = 0, SCSI_STATE_INWORK = 1, SCSI_STATE_QUEUED = 2, SCSI_STATE_SENT = 3, @@ -238,8 +236,8 @@ uchar scsi_inquiry_path(scsi_bus bus, scsi_path_inquiry *inquiry_data); scsi_ccb *scsi_alloc_ccb(scsi_device_info *device); void scsi_free_ccb(scsi_ccb *ccb); -status_t scsi_init_ccb_alloc(scsi_bus_info *bus); -void scsi_uninit_ccb_alloc(scsi_bus_info *bus); +status_t init_ccb_alloc(); +void uninit_ccb_alloc(); // devices.c diff --git a/src/add-ons/kernel/bus_managers/scsi/scsi_io.cpp b/src/add-ons/kernel/bus_managers/scsi/scsi_io.cpp index b1073f7b5b..4179556255 100644 --- a/src/add-ons/kernel/bus_managers/scsi/scsi_io.cpp +++ b/src/add-ons/kernel/bus_managers/scsi/scsi_io.cpp @@ -194,7 +194,7 @@ finish_autosense(scsi_device_info *device) } // inform peripheral driver - release_sem_etc(orig_request->completion_sem, 1, 0/*B_DO_NOT_RESCHEDULE*/); + orig_request->completion_cond.NotifyAll(); } @@ -325,7 +325,7 @@ scsi_request_finished(scsi_ccb *request, uint num_requests) else { // tell peripheral driver about completion if (!do_autosense) - release_sem_etc(request->completion_sem, 1, 0/*B_DO_NOT_RESCHEDULE*/); + request->completion_cond.NotifyAll(); } } @@ -483,7 +483,7 @@ err2: if (request->buffered) scsi_release_dma_buffer(request); err: - release_sem(request->completion_sem); + request->completion_cond.NotifyAll(B_ERROR); return; } @@ -508,8 +508,12 @@ scsi_sync_io(scsi_ccb *request) } } + ConditionVariableEntry entry; + request->completion_cond.Add(&entry); + scsi_async_io(request); - acquire_sem(request->completion_sem); + + entry.Wait(); if (tmp_sg) cleanup_temp_sg(request); @@ -567,7 +571,7 @@ scsi_abort(scsi_ccb *req_to_abort) scsi_release_dma_buffer(req_to_abort); // tell peripheral driver about - release_sem_etc(req_to_abort->completion_sem, 1, 0/*B_DO_NOT_RESCHEDULE*/); + req_to_abort->completion_cond.NotifyAll(B_CANCELED); if (start_retry) release_sem(bus->start_service); diff --git a/src/add-ons/kernel/busses/scsi/ahci/ahci_defs.h b/src/add-ons/kernel/busses/scsi/ahci/ahci_defs.h index 511d5784e1..9900a8f9b6 100644 --- a/src/add-ons/kernel/busses/scsi/ahci/ahci_defs.h +++ b/src/add-ons/kernel/busses/scsi/ahci/ahci_defs.h @@ -332,7 +332,6 @@ extern scsi_for_sim_interface *gSCSI; #define LO32(val) ((uint32)(val)) #define HI32(val) ((uint32)(((uint64)(val)) >> 32)) -#define ASSERT(expr) if (expr) {} else panic("%s", #expr) #define PCI_VENDOR_INTEL 0x8086 #define PCI_VENDOR_JMICRON 0x197b diff --git a/src/add-ons/kernel/generic/scsi_periph/io.cpp b/src/add-ons/kernel/generic/scsi_periph/io.cpp index 6bce83f90d..8f3c9ac508 100644 --- a/src/add-ons/kernel/generic/scsi_periph/io.cpp +++ b/src/add-ons/kernel/generic/scsi_periph/io.cpp @@ -342,9 +342,12 @@ read_write(scsi_periph_device_info *device, scsi_ccb *request, //if (status != B_OK) // return status; + ConditionVariableEntry entry; + request->completion_cond.Add(&entry); + device->scsi->async_io(request); - acquire_sem(request->completion_sem); + entry.Wait(); // ask generic peripheral layer what to do now res = periph_check_error(device, request);