On Thu, Oct 11 2007, Tejun Heo wrote: > Jens Axboe wrote: > > This is the old ata_sg_is_last: > > > > static inline int > > ata_sg_is_last(struct scatterlist *sg, struct ata_queued_cmd *qc) > > { > > if (sg == &qc->pad_sgent) > > return 1; > > if (qc->pad_len) > > return 0; > > if (((sg - qc->__sg) + 1) == qc->n_elem) > > return 1; > > return 0; > > } > > > > and the new one: > > > > static inline int > > ata_sg_is_last(struct scatterlist *sg, struct ata_queued_cmd *qc) > > { > > if (sg == &qc->pad_sgent) > > return 1; > > if (qc->pad_len) > > return 0; > > if (qc->n_iter == qc->n_elem) > > return 1; > > return 0; > > } > > > > ->n_iter is how ata_qc_next_sg() walks over the sglist, I don't > > understand your reference to why depending on that during iteration > > would be bad? > > Because that makes ata_sg iterator macros have hidden side effects > (nothing in the interface suggests it can't be nested and when somebody > actually nests it, finding what went wrong can be pretty difficult). I > think it would be better to have explicit ata_sg_iter passed around if > sg itself isn't enough to walk the sg list.
I think it's implicit in the interface, since you don't pass an sgtable in. But it's not a big deal to me, if you want it changed, go ahead :-) > > So we could add a test for sg_last() there, but that would turn sg table > > iteration into an O(N^2) operation for those drivers that use > > ata_sg_is_last() with chained sg tables. I'd much rather just get rid of > > ata_sg_is_last(), it's only used to mark end-of-table entries for > > hardware. That logic can be performed cheaper. > > Yeap, it can be removed but having "is this the last one?" test is just > nicer to low level drivers. With ata_sg_iter, I think we can do it in > O(N) by looking up and caching the next entry. Sure, of you could just lookup sg_last() in the beginning of the loop. Still seems a bit silly to me just to keep the ata_sg_is_last() interface, since it can be done for free basically by maintaining an sglast element in the loop that you then use when the loop is done to set your SG_END marker (or whatever the driver needs). -- Jens Axboe - 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/