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
<SCSI.h>, 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

@@ -94,17 +94,18 @@
 
 
 #include <KernelExport.h>
+
+#ifdef _KERNEL_MODE
+#include <condition_variable.h>
 #include <device_manager.h>
+#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;
+	// notified after asynchronous execution of request
-	// initialised by alloc_ccb, can be replaced for action but
+	ConditionVariable completion_cond;
-	// must be restored before returning via free_ccb
-	sem_id		completion_sem;
 
+#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

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;
 }

src/add-ons/kernel/bus_managers/scsi/ccb.cpp

@@ -1,7 +1,8 @@
 /*
-** Copyright 2002/03, Thomas Kurschel. All rights reserved.
+ * Copyright 2002-2003, Thomas Kurschel. All rights reserved.
-** Distributed under the terms of the MIT License.
+ * 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 <slab/Slab.h>
 
 
-// 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
+static object_cache* sCcbPool = NULL;
-// it must be at least 1 for normal use and 1 for stand-by autosense request
-#define CCB_NUM_MAX 128
 
 
 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;
+	object_cache_free(sCcbPool, ccb, 0);
-
-	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);
 }
 
 
 status_t
-scsi_init_ccb_alloc(scsi_bus_info *bus)
+init_ccb_alloc()
 {
-	// initially, we want no CCB allocated as the path_id of
+	sCcbPool = create_object_cache("scsi ccb", sizeof(scsi_ccb), 0, NULL, NULL, NULL);
-	// the bus is not ready yet so the CCB cannot be initialized
+	if (sCcbPool == NULL)
-	// 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)
 		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);
 }
-

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);
+status_t init_ccb_alloc();
-void scsi_uninit_ccb_alloc(scsi_bus_info *bus);
+void uninit_ccb_alloc();
 
 
 // devices.c

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);

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

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);