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.

Reply via email to