On Tue, Feb 15, 2011 at 4:42 AM, FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx> wrote: > On Mon, 14 Feb 2011 12:50:40 +0100 > Bart Van Assche <bvanassche@xxxxxxx> wrote: > >> - send_rsp() sends back an SRP response with req_lim_delta = 1 before >> srp_iu_put() is invoked. I think this is a race condition: it can >> happen that the SRP initiator has received an SRP_RSP and sends a new >> SRP_CMD before srp_iu_put() was invoked. On a heavily loaded system >> that can trigger a queue overflow in the target. > > Yeah, we had better to handle such case better. > > But I don't think that we hit a critical event such as crash, memory > corruption, etc, with the current code. > > In such case, srp_iu_get return NULL, so the target ignores the > request, then eventually the initiator recovers. Sorry but I do not agree that hitting this race is harmless. If this race gets triggered the credit associated with the lost SRP information unit will never be returned to the initiator. If this happens frequently enough (INITIAL_SRP_LIMIT times), the initiator will run out of credits and will lock up forever. > Probably, we should set the queue size to INITIAL_SRP_LIMIT + 1. With the current implementation of the SCSI storage target core that's probably sufficient. But if that core is ever modified such that multiple SCSI commands can be processed concurrently, that limit wouldn't be sufficient anymore because then the described race condition could be triggered in multiple threads simultaneously - at least in theory. An alternative solution is to modify send_iu() such that srp_iu_put() is invoked after all relevant data has been copied via h_copy_rdma() and to the CRQ and before h_send_crq() is invoked. That approach has e.g. been implemented in this patch: http://www.spinics.net/lists/linux-scsi/msg49105.html. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
