> This has the potential of leaving a big fat ugly hole in the middle of > scsi_cmnd. I would suggest of *just* moving the sense_buffer array to be > the *first member* of struct scsi_cmnd. The command itself is already cache > aligned, allocated by the proper flags to it's slab. And put a fat comment > near it's definition. The hole would only be present on non coherent architectures though. We also still need the padding after the sense_buffer. But yeah, moving the sense buffer to first position would diminish the size of the hole I suppose. > This is until a proper fix is sent. I have in my Q a proposition for a > more prominent solution, which I will send next month. Do to short comings > in the sense handling and optimizations, but should definitely cover this > problem. > > The code should have time to be discussed and tested, so it is only 2.6.26 > material. For the duration of the 2.6.25 kernel we can live with a reorder > of scsi_cmnd members, if it solves such a grave bug for some ARCHs. Re-oreder isn't enough. We need both re-order and the __dma_buffer annotation to ensure proper padding. Some archs have 64 or even 128 bytes cache line size. I agree that in the long run, a better solution should be done involving reworking the way the sense buffer is allocated. I started modifying the EH code to kmalloc it, but it gets very messy with the current interface between drivers and EH. I still have some patches around I can send, though some drivers need fixing (error handling on kmalloc failure). James seem to have a better idea of pre-allocating sense buffers per host based on how many they can have in flight which I haven't looked at. Ben. > Boaz > ---- > [RFD below] > My proposed solution will be has follows: > > 1. Since the scsi protocol mandates an immediate REQUEST_SENSE after an error > in effect the Q is frozen until the REQUEST_SENSE command returns. > > 2. The scsi-ml needs the sense buffer for its normal operation, independent > from the ULD's request of the sence_buffer or not at request->sense. But > in effect, 90% of all scsi-requests come with ULD's allocated buffer for > sense, that is copied to, on command completion. > > 3. 99% of all commands complete successfully, so if an optimization is > proposed for the successful case, sacrificing a few cycles for the error > case than thats a good thing. > > My suggestion is to have a per Q, driver-overridable, sense buffer that is > DMAed/written to by drivers. At the end of the REQUEST_SENSE command one > of 2 things will be done. Either copy the sense to the ULD's supplied buffer, > or if not available, allocate one from a dedicated mem_cache pool. I think that's pretty much was James was proposing indeed. > So we are completely saving 92 bytes from scsi_cmnd by replacing the buffer > with a pointer. We can always read the sense into a per Q buffer. And 10% of > %1 of the times we will need to allocate a sense buffer from a dedicated mem_cache > I would say thats a nice optimization. > > The changes to scsi_error/scsi_cmnd and friends, is pretty strait forward. But > it depends on a conversion of 4/5 drivers to the new scsi_eh API for > REQUEST_SENSE. I have only converted these drivers that interfered with the accessors > effort + 1 other places. But there are a few more places that are not converted. > Once done. The implementation can easily change with no affect on drivers. > > The reason I've started with this work is because the SCSI standard permits up > to 260 bytes of sense, which you guest right, is needed by the OSD command set. > > Boaz - 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
