On Mon, 2017-02-27 at 15:28 -0800, James Smart wrote: > I'd like to announce the availability of the Broadcom (Emulex) FC Target > driver - efct. Hello James, Sorry but I have not yet had the time to do a full review. But I would like to already share the following feedback: * efct_io_find_tgt_io() looks up an I/O request in a list. The lookup is protected by a spinlock but callers are expected to increase the I/O request reference count after the lock has been released. This looks wrong to me and most likely means that there are race conditions related to I/O request lookup. Have you considered to move the kref_get() call into efct_io_find_tgt_io() such that it can occur with the spinlock held? * If an ABTS is received then efct_process_abts() (indirectly) calls target_submit_tmr(). That last function may or may not decide to call efct_lio_queue_status(), depending on whether or not the TAS bit has been set. efct_lio_queue_status() sends back a SCSI response to the initiator (see also __transport_check_aborted_status() and transport_send_task_abort()). Is it allowed by the FC spec to send back a SCSI response after an ABTS has been received? Is that SCSI response sent before or after the BLS ABTS response? Thanks, Bart.
