On 05/31/2011 12:08 AM, Nicholas A. Bellinger wrote: > On Fri, 2011-05-27 at 12:06 -0700, Andy Grover wrote: >> Don't set task->task_sg_num until task_sg is successfully alloced, to >> prevent it from possibly being out of sync. >> >> Simplify loop counting sgs needed. Handle possible (?) case of offset >> greater than se_mem->se_len. Remove some cluttering debugprints. Reduce >> local variable usage. >> >> Do not set termination bit in padded-1 entry in the padded_sg case. If >> something barfs on an extra zeroed sg, (will happen if chain-capable sgs >> don't actually get chained) then it's broken. >> >> Remove unneeded parens. >> >> Signed-off-by: Andy Grover <agrover@xxxxxxxxxx> >> --- > > So I am not exactly sure there is an issue here with task->task_sg_num > assignment in the failure case, as nothing depends AFAICT upon > task->task_sg_num in the release path before an possible > transport_init_task_sg() failure.. Yeah I was just being paranoid.. > However, my main concern is the removal of sg_mark_end() calls to > signify the end of task->task_sg[] memory for individual task I/O > dispatch. As you have noticed this looks OK with existing code wrt to > backend *_map_task_sg() and *_do_task() using for_each_sg() w/ > task->task_sg_num, but am wondering if it really makes sense to remove > this for the actual scatterlist dispatch for all backends..? Is there > any possible code below TCM backend drivers where HW drivers can pose an > issue..? > > I am happy to merge this for tcm v4.1 along with patch #11 if this is > really safe to do, but will defer for the moment until we can get a few > more ACKs on these two.. Every sg[] that is initialized by sg_init_table (or sg_init_one) should have sg_mark_end called on its last element[1] so I think we're ok, or am I misunderstanding the issue you're getting at? Regards -- Andy [1] http://lxr.linux.no/linux+v2.6.38/lib/scatterlist.c#L85 -- 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
