On Wed, Oct 10, 2012 at 05:46:35PM -0700, Nicholas A. Bellinger wrote: > Mmmm, the unprotected use of ->dev_reservation_flags to check for legacy > reservations within target_check_reservation looks wrong to me.. > > Both target_scsi2_reservation_[reserve,release] already hold > ->dev_reservation_lock, so we'd also need to obtain the same lock here > with your patch to prevent different process context from issuing > RESERVE/RELEASE and potentially hitting the wrong reservations code-path > when multiple initiators try to switch between the two modes. > > I certainly like the cleanups + simplifications in this patch, but this > will need to be addressed before merging. So NACK on this for the > moment. It is racy, but the race is not in any way new in this patch, the check just moved from core_scsi3_pr_reservation_check to target_check_reservation as part of the cleanup. I'll send you a patch ontop of the series to fix it up, which will be a lot easier than respinning it and also keep unrelated changes apart. > > --nab > > > > > -- > To unsubscribe from this list: send the line "unsubscribe target-devel" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html ---end quoted text--- -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html