Stefan Richter wrote: > I wrote on 2005-12-09: > >> Jody McIntyre wrote: > > ... > >>> Do you still want this if Jens' patch is applied to the SCSI subsystem? >>> Extra checks are nice, but extra code is not :) It's up to you... > > ... > >> scsi_prep_fn() is not the only path which may set the transfer >> direction. Unless *all* paths are guaranteed to send us a proper >> direction, we still need the patch for sbp2_create_command_orb(). >> >> If all *known* paths are fixed, we could consider to override a bad >> transfer direction silently like e.g. usb_storage does. But we should >> not leave sbp2_create_command_orb() as fragile as it currently is. >> >> I wonder if I know all paths where the direction could be set. > > > I learned now of many more points which set the transfer direction. It > can apparently even come from userspace. > >> I also wonder if there could ever be something else execpt DMA_NONE, >> DMA_TO_DEVICE, or DMA_FROM_DEVICE being passed to sbp2. If it is >> guaranteed that scsi_cmnd.sc_data_direction is one of these three, we >> should remove ieee1394/sbp2.h::sbp2scsi_direction_table[]. > > > AFAIU it is still possibly that DMA_BIDIRECTIONAL is passed down. > However I think now that we should delete sbp2scsi_direction_table > anyway and simply reject DMA_BIDIRECTIONAL. This is what a few other > SCSI low-level drivers do, and I think it is appropriate for a low-level > driver to fail such commands instead of converting them to a "known" > direction. > > Or am I missing something? SCSI does define various bidirectional commands, mostly in OSD (Object Storage) and a couple in SBC (for disks, RAID related). Linux does support not them yet. About the time when Linux supports command lengths greater than 16 bytes, it will also need to support bidirectional data transfers. Perhaps bidirectional data transfers would be implemented by two scatter gather lists. Doug Gilbert - : 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
