Messages in this thread | | | Date | Fri, 20 Mar 2015 10:10:24 +0100 (CET) | From | Sebastian Ott <> | Subject | Re: [PATCH] lib/dma-debug: fix bucket_find_contain |
| |
On Thu, 19 Mar 2015, Andrew Morton wrote: > On Mon, 16 Mar 2015 19:24:29 +0100 (CET) Sebastian Ott <sebott@linux.vnet.ibm.com> wrote: > > > bucket_find_contain will search the bucket list for a dma_debug_entry. When > > the entry isn't found it needs to search other buckets too, since only the > > start address of a dma range is hashed (which might be in a different bucket). > > > > A copy of the dma_debug_entry is used to get the previous hash bucket but when > > its list is searched the original dma_debug_entry is to be used not its > > modified copy. > > > > This fixes false "device driver tries to sync DMA memory it has not allocated" > > warnings. > > The changelog doesn't help me understand how serious this is. A 4.0 > thing? -stable?
Hm, this is broken since quite some time and nobody noticed or cared. I think it doesn't need to be in stable. Also without the patch you'll just get some false positive debug_dma warnings.
> > > --- a/lib/dma-debug.c > > +++ b/lib/dma-debug.c > > @@ -361,7 +361,7 @@ static struct dma_debug_entry *bucket_fi > > unsigned int range = 0; > > > > while (range <= max_range) { > > - entry = __hash_bucket_find(*bucket, &index, containing_match); > > + entry = __hash_bucket_find(*bucket, ref, containing_match); > > > > if (entry) > > return entry; > > This code just got more confusing :( Are you sure we aren't supposed to > use `ref' in bucket_find_contain()'s call to get_hash_bucket()?
The only caller of bucket_find_contain uses get_hash_bucket(ref).
I suspect that in most cases the dma_handle used for dma_sync will lead to the same bucket as the handle used for dma_map (and we get a hit in the first __hash_bucket_find).
But if a large dma mapping is created it could span more than one bucket. So when dma_sync(X) is called it's possible we didn't find the entry in the bucket related to X. We use index to get the previous bucket but we still need to search this bucket for X.
When we would use get_hash_bucket(ref) again we would search the bucket which we've already searched in the first run.
Regards, Sebastian
| |