lkml.org 
[lkml]   [2009]   [Apr]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 5/7] swiotlb: (re)Create swiotlb_unmap_single
From
On Tue, 7 Apr 2009 12:32:12 -0500
Kumar Gala <galak@kernel.crashing.org> wrote:

>
> On Apr 7, 2009, at 12:22 PM, FUJITA Tomonori wrote:
>
> > On Tue, 7 Apr 2009 11:50:56 -0500
> > Kumar Gala <galak@kernel.crashing.org> wrote:
> >
> >>
> >> On Apr 7, 2009, at 11:37 AM, FUJITA Tomonori wrote:
> >>
> >>> On Tue, 7 Apr 2009 10:32:20 -0500
> >>> Kumar Gala <galak@kernel.crashing.org> wrote:
> >>>
> >>>>
> >>>> On Apr 7, 2009, at 4:09 AM, FUJITA Tomonori wrote:
> >>>>
> >>>>> On Tue, 7 Apr 2009 01:34:44 -0500
> >>>>> Kumar Gala <galak@kernel.crashing.org> wrote:
> >>>>>
> >>>>>>
> >>>>>> On Apr 6, 2009, at 9:24 PM, FUJITA Tomonori wrote:
> >>>>>>
> >>>>>>> On Fri, 3 Apr 2009 20:56:47 -0500
> >>>>>>> Becky Bruce <beckyb@kernel.crashing.org> wrote:
> >>>>>>>
> >>>>>>>> This mirrors the current swiotlb_sync_single() setup
> >>>>>>>> where the swiotlb_unmap_single() function is static to this
> >>>>>>>> file and contains the logic required to determine if we need
> >>>>>>>> to call actual sync_single. Previously, swiotlb_unmap_page
> >>>>>>>> and swiotlb_unmap_sg were duplicating very similar code.
> >>>>>>>> The duplicated code has also been reformatted for
> >>>>>>>> readability.
> >>>>>>>>
> >>>>>>>> Note that the swiotlb_unmap_sg code was previously doing
> >>>>>>>> a complicated comparison to determine if an addresses needed
> >>>>>>>> to be unmapped where a simple is_swiotlb_buffer() call
> >>>>>>>> would have sufficed.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Becky Bruce <beckyb@kernel.crashing.org>
> >>>>>>>> ---
> >>>>>>>> lib/swiotlb.c | 36 +++++++++++++++++++++++-------------
> >>>>>>>> 1 files changed, 23 insertions(+), 13 deletions(-)
> >>>>>>>>
> >>>>>>>> diff --git a/lib/swiotlb.c b/lib/swiotlb.c
> >>>>>>>> index af2ec25..602315b 100644
> >>>>>>>> --- a/lib/swiotlb.c
> >>>>>>>> +++ b/lib/swiotlb.c
> >>>>>>>
> >>>>>>> I don't think 'swiotlb_unmap_single' name is appropriate.
> >>>>>>>
> >>>>>>> swiotlb_unmap_single sounds like an exported function that
> >>>>>>> IOMMUs
> >>>>>>> can
> >>>>>>> use (and it was) however it should not be.
> >>>>>>
> >>>>>> What do you suggest we call it? __swiotlb_unmap_single.
> >>>>>
> >>>>> I think that __swiotlb_unmap_single is better because the name
> >>>>> implies
> >>>>> that it's an internal function. It's fine by me.
> >>>>>
> >>>>> If it is odd that __swiotlb_unmap_single() is just a wrapper
> >>>>> function
> >>>>> of unmap_single(), which does the real job to unmap a dma mapping,
> >>>>> it
> >>>>> might be another possible option to rename unmap_single to
> >>>>> do_unamp_single and use unmap_single.
> >>>>
> >>>> I think you lost me here. I'd prefer to just use
> >>>> __swiotlb_unmap_single at this point and get this code into the
> >>>> tree
> >>>> and work on such renaming after the fact (if that's ok).
> >>>
> >>> If you are rushing to merge this right now, the original patchset is
> >>> fine by me (I thought that you missed this merge window). I'll
> >>> rename
> >>> it later.
> >>
> >> We probably did, but one can never tell with these things. It seemed
> >> like Ingo merged and pushed some swiotlb changes late in the game
> >> for .
> >> 29
> >
> > Well, merging patches that have not been tested linux-next late is
> > what we should not do, I guess. I like to see Becky's patch in 2.6.30
> > because I have some swiotlb changes for 2.6.31 though.
>
> Same here. It makes it easier for us to work on the powerpc arch
> specific changes for .31 if we can get these into .30. What are you
> looking at for .31?

I need to finish the dma_mapping_ops cleanups and cross-arch
unification stuff:

http://marc.info/?l=linux-kernel&m=123827903216314&w=2

But the changes to swiotlb is minor. BTW, I have the patches for
powerpc too.


> Ingo, any comments on that?


\
 
 \ /
  Last update: 2009-04-07 20:21    [W:0.070 / U:0.188 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site