Hello, Vladislav.
Sorry about the delay.
Vladislav Bolkhovitin wrote:This patch reimplements scsi_execute_async(). In the new version
it's a lot less hackish and also has additional features. Namely:
:-)
1. Possibility to insert commands both in tail and in head of the queue.
2. Possibility to explicitly specify if the last SG element has
space for padding.
This patch based on the previous patches posted by Tejun
Heo. Comparing to them it has the following improvements:
1. It uses BIOs chaining instead of kmalloc()ing the whole bio.
2. It uses SGs chaining instead of kmalloc()ing one big SG in case
if direct mapping failed (e.g. because of DMA alignment or padding).
3. If direct mapping failed, if possible, it copies only the last SG
element, not the whole SG.
4. When needed, copy_page() is used instead of memcpy() to copy the
whole pages.
Also this patch adds and exports functions sg_copy() and
sg_copy_elem(), which cop one SG to another and one SG element to
another respectively.
At the moment SCST is the only user of this functionality. It needs
it, because its target drivers, which are, basically, SCSI drivers,
can deal only with SGs, not with BIOs. But, according the latest
discussions, there are other potential users for of this
functionality, so I'm sending this patch in a hope that it will be
also useful for them and eventually will be merged in the mainline
kernel.
This patch requires previously sent patch with subject "[PATCH]:
Rename REQ_COPY_USER to more descriptive
REQ_HAS_TAIL_SPACE_FOR_PADDING".
The original patchset was focused more on unifying user and kernel and
user sg mapping handling. It seems you implemented the kernel part
completely separately. Wouldn't it be better to unify where possible?
Or is there some fundamental reason that can't be done that I missed?
Also, code organization-wise, I think good part of the posted code
belongs to bio.c.
The tail-only copying is nice but I'm not entirely sure whether such
full-blown approach is necessary. The tail padding was added
primarily for dumb ATAPI devices which want to transfer more bytes
than requested. Having extra space at the end makes the driver's job
much easier as it can just overflow into the area. Some controller do
have "drain after this" flag in the dma table but some simply can't
handle such situations properly without explicit overflow area.
So, being the horrid hack it is and highly unlikely to be used in
performance sensitive path, I think it would be better to keep the
implementation simple and dull. It just isn't something worth
investing complexity over. Of course, if you have a use case which
requires high performance tail padding, it's a different story.