> 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-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
