Messages in this thread |  | | | Date | Wed, 17 Oct 2007 20:20:25 +0200 | | From | Jens Axboe <> | | Subject | Re: [bug] block subsystem related crash with latest -git |
On Wed, Oct 17 2007, Linus Torvalds wrote: > > > On Wed, 17 Oct 2007, Jens Axboe wrote: > > > > > > - remove the "memset()" you had added earlier. It's bogus. It cannot be > > > the right thing. If the sg list wasn't initialized correctly much > > > earlier, trying to initialize it late is pointless - it contains crap. > > > > It's required to clear output members (like dma_len and so on), since > > some of the IOMU code really wants that initialized. > > No it's NOT! > > The whole point here is that the sg had *already* better be cleared. > > If it wasn't cleared before, that's a bug regardless of anything else. So > a memset() is guaranteed to be either a no-op or hiding another bug! > > So yes, those members had better be zero. It's just that they had better > be zero long before that memset! The memset should have been done when > allocating the SG list.
For the chain elements - yes, definitely! But we also want to clear dma mapping output values, at least sparc64 wants that. You could argue that the IOMMU code should be fixed up, but I don't think we should mix the two.
So we need the memset() in blk_rq_map_sg() AS WELL AS the initial sg table clear. I'm not arguing about the latter, we clearly do need that.
> > If you prefer the old next_sg approach to my alternative, that is fine > > with me. But we do need the memset(). > > Really? Explain why. Entering that code with a non-initialized SG list is > a bug to begin with.
Not for the output members, they will be filled AFTER blk_rq_map_sg() returns and someone use iommu_map_sg() to re-iterate the sglist and map the pages appropriately.
-- Jens Axboe
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |