Hello, Jens, James. Jens Axboe wrote: >> With the original patch, I have to run through the whole of libsas and >> scsi_transport_sas doing >> >> s/data_len/raw_data_len/ >> >> With your update it looks like I have to run through them all doing >> >> s/data_len/data_len - extra_len/ blk_rq_raw_data_len() should do. >> which is even worse. Can't we put things back to a point where data_len >> means exactly that and extra_len means how much we have spare on the >> end, so you know you can DMA up to data_len + extra_len if need be? >> >> That way we don't have to sweep through every block driver altering the >> way it uses data_len. If SMP is broken because it needs start address alignment but not padding to align the size, what should be done is to make that exact requirement visible to the block layer. Say, blk_queue_dma_start_alignment() or maybe change blk_queue_dma_alignment() such that it only indicates start address alignment and add blk_queue_dma_size_alignment() for drivers which require size to be aligned too. I think those are few. I think the decision which value rq->data_len represents comes down to which size is used more in low level drivers because no matter which way we choose we'll have to update some of the drivers which expects the other thing from rq->data_len. blk_rq_raw_data_len() is needed iff a driver needs dummy buffers attached at the end and still needs to know the original request size which isn't the common case. > Fully agree. The reason why I think it's so ugly is that you have to > keep these two seperate variables in sync. The burning was just one bug, > there will be others... The posted modification isn't too bad as the maintenance of the two variables is at places where the nasty things happen. I think what rq->data_len should represent when seen from LLDs is more important and please note that if SMP is broken because it simply doesn't require 512byte size alignment, it's a different issue. As long as both raw_data_len and data_len are accessible, I'm okay either way. My biggest reluctance is against breaking sum(sg) == rq->data_len. I think this can lead to much more subtle problems such as programming the controller w/ wrong bytes count and wrapped-around resid calculation. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
