Messages in this thread | | | Date | Fri, 14 Aug 2020 07:42:41 +0200 | From | Christoph Hellwig <> | Subject | Re: [PATCH] dma-debug: fix debug_dma_assert_idle(), use rcu_read_lock() |
| |
On Thu, Aug 13, 2020 at 12:02:41PM -0700, Linus Torvalds wrote: > Yeah, that's ridiculously expensive, and serializes things for no good reason. > > Your patch looks obviously correct to me (Christoph?),
It also looks correct to me.
> but it also > makes me go "why are we doing this in the first place"? > > Because it looks to me like > (a) the debug check is wrong > (b) this is left-over from early debugging > > In particular, I don't see why we couldn't do a COW on a page that is > under writeback at the same time. We're not changing the page that is > doing DMA.
Yes. We don't need to check for a DMA to the device, but a DMA from the device while under DMA obviously is bogus. But then again you'd need to try really hard to do that.
> In fact, the whole "COW with DMA" makes me feel like the real bug may > have been due that whole "ambiguous COW" thing, which was fixed in > 17839856fd58 ("gup: document and work around "COW can break either > way" issue") > > That debug thing goes back almost 7 years, and I don't think it has > caught anything in those seven years, but I could be wrong. > > The commit that adds it does talk about a bug, but that code was > removed entirely eventually. And google shows no hits for > debug_dma_assert_idle() since - until your email. > > So my gut feel is that we should remove the check entirely, although > your patch does seem like a big improvement. > > Christoph?
The whole thing predates my involvement with the code, but I defintively think the patch from Hugh is a major improvement. But I would also have no problem with just removing it entirely.
| |