Quoting Robin Murphy (2019-07-22 11:13:49) > Hi Chris, > > On 20/07/2019 19:08, Chris Wilson wrote: > > Since the cached32_node is allowed to be advanced above dma_32bit_pfn > > (to provide a shortcut into the limited range), we need to be careful to > > remove the to be freed node if it is the cached32_node. > > Thanks for digging in - just to confirm my understanding, the > problematic situation is where both 32-bit and 64-bit nodes have been > allocated, the topmost 32-bit node is freed, then the lowest 64-bit node > is freed, which leaves the pointer dangling because the second free > fails the "free->pfn_hi < iovad->dma_32bit_pfn" check. Does that match > your reasoning? Yes. Once cached32_node is set to the right of dma_32bit_pfn it fails to be picked up in the next cycle through __cached_rbnode_delete_update should cached32_node be the next victim. > Assuming I haven't completely misread the problem, I can't think of a > more appropriate way to fix it, so; > > Reviewed-by: Robin Murphy <robin.murphy@xxxxxxx> > > I could swear I did consider that corner case at some point, but since > Leizhen and I rewrote this stuff about 5 times between us I'm not > entirely surprised such a subtlety could have got lost again along the way. I admit to being impressed that rb_prev() does not appear high in the profiles -- though I guess that is more an artifact of the scary layers of caching above it. -Chris
