Message ID | 20100120152832.06a905a5@nehalam |
---|---|
State | Rejected, archived |
Headers | show |
From: Stephen Hemminger <shemminger@vyatta.com> Date: Wed, 20 Jan 2010 15:28:32 -0800 > This should fix the dma-debug API code (and documentation), to > avoid false positives when sync is done on a partial map. > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> ... > --- a/lib/dma-debug.c 2010-01-20 15:22:55.919519883 -0800 > +++ b/lib/dma-debug.c 2010-01-20 15:26:31.648895638 -0800 > @@ -285,11 +285,9 @@ static struct dma_debug_entry *hash_buck > } > > /* > - * If we have multiple matches but no perfect-fit, just return > - * NULL. > + * If we have multiple matches but no perfect-fit > + * return best value and let caller deal with it. > */ > - ret = (matches == 1) ? ret : NULL; > - > return ret; > } > I think you have to enforce a perfect-fit match for the non-SYNC cases. check_sync() can pass in 'true' for a new bool argument "partial_ok" added to hash_bucket_find() whereas check_unmap() can pass 'false'. get_nr_mapped_entries() should pass 'false' as well. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 20 Jan 2010 16:18:05 -0800 (PST) David Miller <davem@davemloft.net> wrote: > From: Stephen Hemminger <shemminger@vyatta.com> > Date: Wed, 20 Jan 2010 15:28:32 -0800 > > > This should fix the dma-debug API code (and documentation), to > > avoid false positives when sync is done on a partial map. > > > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> > ... > > --- a/lib/dma-debug.c 2010-01-20 15:22:55.919519883 -0800 > > +++ b/lib/dma-debug.c 2010-01-20 15:26:31.648895638 -0800 > > @@ -285,11 +285,9 @@ static struct dma_debug_entry *hash_buck > > } > > > > /* > > - * If we have multiple matches but no perfect-fit, just return > > - * NULL. > > + * If we have multiple matches but no perfect-fit > > + * return best value and let caller deal with it. > > */ > > - ret = (matches == 1) ? ret : NULL; > > - > > return ret; > > } > > > > I think you have to enforce a perfect-fit match for the > non-SYNC cases. > > check_sync() can pass in 'true' for a new bool argument "partial_ok" > added to hash_bucket_find() whereas check_unmap() can pass 'false'. > > get_nr_mapped_entries() should pass 'false' as well. check_unmap has checks for different size, different type, call_ents, direction etc. The code for get_nr_mapped_entries() is the only question; it is used by unmap_sg() and probably needs perfect match only. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Stephen Hemminger <shemminger@vyatta.com> Date: Wed, 20 Jan 2010 16:34:10 -0800 > check_unmap has checks for different size, different type, > call_ents, direction etc. > > The code for get_nr_mapped_entries() is the only question; > it is used by unmap_sg() and probably needs perfect match only. Ok. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 20, 2010 at 04:34:10PM -0800, Stephen Hemminger wrote: > On Wed, 20 Jan 2010 16:18:05 -0800 (PST) > David Miller <davem@davemloft.net> wrote: > > > From: Stephen Hemminger <shemminger@vyatta.com> > > Date: Wed, 20 Jan 2010 15:28:32 -0800 > > > > > This should fix the dma-debug API code (and documentation), to > > > avoid false positives when sync is done on a partial map. > > > > > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> > > ... > > > --- a/lib/dma-debug.c 2010-01-20 15:22:55.919519883 -0800 > > > +++ b/lib/dma-debug.c 2010-01-20 15:26:31.648895638 -0800 > > > @@ -285,11 +285,9 @@ static struct dma_debug_entry *hash_buck > > > } > > > > > > /* > > > - * If we have multiple matches but no perfect-fit, just return > > > - * NULL. > > > + * If we have multiple matches but no perfect-fit > > > + * return best value and let caller deal with it. > > > */ > > > - ret = (matches == 1) ? ret : NULL; > > > - > > > return ret; > > > } > > > > > > > I think you have to enforce a perfect-fit match for the > > non-SYNC cases. > > > > check_sync() can pass in 'true' for a new bool argument "partial_ok" > > added to hash_bucket_find() whereas check_unmap() can pass 'false'. > > > > get_nr_mapped_entries() should pass 'false' as well. > > check_unmap has checks for different size, different type, > call_ents, direction etc. > > The code for get_nr_mapped_entries() is the only question; > it is used by unmap_sg() and probably needs perfect match only. > Hmm... After looking at it more, mainly dma_sync_single_range() implementations, I didn't find any reasons for these comments in DMA-API.txt, but even if I missed something, this patch definitely shows these lib/dma-debug warnings are buggy if they depend on the number of mapped entries (while otherwise ref->size < entry->size isn't reported). Btw, it seems code should try to return minimum entry->size for such a check. Thanks Stephen, now I feel 99.9% safe ;-) Sorry for mistrust, David! Jarek P. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Jarek Poplawski <jarkao2@gmail.com> Date: Thu, 21 Jan 2010 12:44:23 +0000 > Sorry for mistrust, David! That's twice in the past week Jarek. One more and you will be in real troubles :-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- a/Documentation/DMA-API.txt 2010-01-20 15:17:01.390143729 -0800 +++ b/Documentation/DMA-API.txt 2010-01-20 15:18:48.967875255 -0800 @@ -377,9 +377,10 @@ void pci_dma_sync_sg(struct pci_dev *hwdev, struct scatterlist *sg, int nelems, int direction) -Synchronise a single contiguous or scatter/gather mapping. All the -parameters must be the same as those passed into the single mapping -API. +Synchronise a single contiguous or scatter/gather mapping. The +device and handle must be the same as those passed into the single mapping +API. The size can be less than the original mapping if only part +of the mapping needs to be accessed. Notes: You must do this: --- a/lib/dma-debug.c 2010-01-20 15:22:55.919519883 -0800 +++ b/lib/dma-debug.c 2010-01-20 15:26:31.648895638 -0800 @@ -285,11 +285,9 @@ static struct dma_debug_entry *hash_buck } /* - * If we have multiple matches but no perfect-fit, just return - * NULL. + * If we have multiple matches but no perfect-fit + * return best value and let caller deal with it. */ - ret = (matches == 1) ? ret : NULL; - return ret; }
This should fix the dma-debug API code (and documentation), to avoid false positives when sync is done on a partial map. Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html