From 64f39c9a426744b4586938ceedbea14189d708d9 Mon Sep 17 00:00:00 2001 From: Sebastian Sumpf Date: Wed, 2 Dec 2015 15:39:57 +0100 Subject: [PATCH] usb_drv: Remove signal dispatching from storage Do not call 'wait_and_dispatch_one_signal' in storage back end. This causes a race of the EP with the main thread. --- repos/dde_linux/lib/mk/usb.inc | 1 - repos/dde_linux/run/usb_storage.run | 2 +- repos/dde_linux/src/lib/usb/include/lx_emul.h | 1 + .../dde_linux/src/lib/usb/storage/storage.cc | 42 +++++++++++++------ 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/repos/dde_linux/lib/mk/usb.inc b/repos/dde_linux/lib/mk/usb.inc index e8c98a01c3..4720427d50 100644 --- a/repos/dde_linux/lib/mk/usb.inc +++ b/repos/dde_linux/lib/mk/usb.inc @@ -27,7 +27,6 @@ CC_WARN = -Wall -Wno-unused-variable -Wno-uninitialized \ CC_C_OPT += -Wno-unused-but-set-variable -Wno-pointer-sign -Wno-unused-label -CXX_OPT = -fpermissive CC_CXX_OPT = -fpermissive # diff --git a/repos/dde_linux/run/usb_storage.run b/repos/dde_linux/run/usb_storage.run index de8d6bbe42..b149f49b8b 100644 --- a/repos/dde_linux/run/usb_storage.run +++ b/repos/dde_linux/run/usb_storage.run @@ -109,7 +109,7 @@ catch { exec sh -c $cmd } # append qemu_args "-drive if=none,id=disk,file=$disk_image" append qemu_args { \ --m 64 -nographic -M pc \ +-m 128 -nographic -M pc \ -device usb-ehci,id=ehci \ -device usb-storage,bus=ehci.0,drive=disk \ -boot order=d } diff --git a/repos/dde_linux/src/lib/usb/include/lx_emul.h b/repos/dde_linux/src/lib/usb/include/lx_emul.h index d21880c62a..1580bdd731 100644 --- a/repos/dde_linux/src/lib/usb/include/lx_emul.h +++ b/repos/dde_linux/src/lib/usb/include/lx_emul.h @@ -1156,6 +1156,7 @@ struct work_struct { work_func_t func; struct list_head entry; unsigned pending; + void *data; }; struct delayed_work { diff --git a/repos/dde_linux/src/lib/usb/storage/storage.cc b/repos/dde_linux/src/lib/usb/storage/storage.cc index 6994b72a6e..c5b7231d11 100644 --- a/repos/dde_linux/src/lib/usb/storage/storage.cc +++ b/repos/dde_linux/src/lib/usb/storage/storage.cc @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -88,6 +89,11 @@ class Storage_device : public Genode::List::Element, PDBG("PACKET: phys: %lx block: %llu count: %zu %s", phys, block_nr, block_count, read ? "read" : "write"); + /* check if we can call queuecommand */ + struct us_data *us = (struct us_data *) _sdev->host->hostdata; + if (us->srb != NULL) + throw Request_congestion(); + struct scsi_cmnd *cmnd = _scsi_alloc_command(); cmnd->cmnd[0] = read ? READ_10 : WRITE_10; @@ -118,10 +124,7 @@ class Storage_device : public Genode::List::Element, cmnd->request = &req; /* send command to host driver */ - while((_sdev->host->hostt->queuecommand(_sdev->host, cmnd)) != 0) { - Server::wait_and_dispatch_one_signal(); - __wait_event(); - } + _sdev->host->hostt->queuecommand(_sdev->host, cmnd); } public: @@ -182,6 +185,20 @@ struct Factory : Block::Driver_factory static Storage_device *device = nullptr; +static work_struct delayed; + + +extern "C" void ack_packet(work_struct *work) +{ + Block::Packet_descriptor *packet = + static_cast(work->data); + + if (verbose) + PDBG("ACK packet for block: %llu", packet->block_number()); + + device->ack_packet(*packet); + Genode::destroy(Genode::env()->heap(), packet); +} void scsi_add_device(struct scsi_device *sdev) @@ -196,6 +213,7 @@ void scsi_add_device(struct scsi_device *sdev) * XXX move to 'main' */ if (!announce) { + PREPARE_WORK(&delayed, ack_packet); static Block::Root root(_signal->ep(), env()->heap(), factory); env()->parent()->announce(_signal->ep().rpc_ep().manage(&root)); announce = true; @@ -205,15 +223,15 @@ void scsi_add_device(struct scsi_device *sdev) void Storage_device::_async_done(struct scsi_cmnd *cmnd) { - Block::Packet_descriptor *packet = - static_cast(cmnd->packet); + /* + * Schedule packet ack, because we are called here in USB storage thread + * context, the from code that will clear the command queue later, so we + * cannot send the next packet from here + */ + delayed.data = cmnd->packet; + schedule_work(&delayed); - if (verbose) - PDBG("ACK packet for block: %llu status: %d", - packet->block_number(), cmnd->result); - - device->ack_packet(*packet); - Genode::destroy(Genode::env()->heap(), packet); scsi_free_buffer(cmnd); _scsi_free_command(cmnd); + }