Re: [RFC 00/17] DRM: fix struct sg_table nents vs. orig_nents misuse

2020-04-28 Thread Robin Murphy

On 2020-04-28 4:32 pm, Daniel Vetter wrote:

On Tue, Apr 28, 2020 at 04:02:57PM +0200, Christoph Hellwig wrote:

On Tue, Apr 28, 2020 at 03:19:48PM +0200, Marek Szyprowski wrote:

1. introduce a dma_{map,sync,unmap}_sgtable() wrappers, which will use
a proper sg_table entries and call respective DMA-mapping functions
and adapt current code to it


That sounds reasonable to me.  Those could be pretty trivial wrappers.




2. rename nents and orig_nents to nr_pages, nr_dmas to clearly state
which one refers to which part of the scatterlist; I'm open for
other names for those entries


nr_cpu_ents and nr_dma_ents might be better names, but it still would be
a whole lot of churn for little gain.  I think just good wrappers like
suggested above might be more helpful.


I guess long-term we could aim for both? I.e. roll out better wrappers
first, once that's soaked through the tree, rename the last offenders.


Yes, that's what I was thinking too - most of these uses are just 
passing them in and out of the DMA APIs, and thus would be subsumed into 
the wrappers anyway, then in the relatively few remaining places where 
the table is actually iterated for one reason or the other, renaming 
would stand to help review and maintenance in terms of making it far 
more obvious when the implementation and the intent don't match.


Robin.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC 00/17] DRM: fix struct sg_table nents vs. orig_nents misuse

2020-04-28 Thread Daniel Vetter
On Tue, Apr 28, 2020 at 04:02:57PM +0200, Christoph Hellwig wrote:
> On Tue, Apr 28, 2020 at 03:19:48PM +0200, Marek Szyprowski wrote:
> > 1. introduce a dma_{map,sync,unmap}_sgtable() wrappers, which will use
> >a proper sg_table entries and call respective DMA-mapping functions
> >and adapt current code to it
> 
> That sounds reasonable to me.  Those could be pretty trivial wrappers.
> 
> >
> > 
> > 2. rename nents and orig_nents to nr_pages, nr_dmas to clearly state
> >which one refers to which part of the scatterlist; I'm open for
> >other names for those entries
> 
> nr_cpu_ents and nr_dma_ents might be better names, but it still would be
> a whole lot of churn for little gain.  I think just good wrappers like
> suggested above might be more helpful.

I guess long-term we could aim for both? I.e. roll out better wrappers
first, once that's soaked through the tree, rename the last offenders.

Personally I like nr_cpu_ents and nr_dma_ents, that's about as clear as it
gets.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel