[PATCH RFT] mips: dma-mapping: support debug_dma_mapping_error

[PATCH RFT] mips: dma-mapping: support debug_dma_mapping_error

[Shuah Khan]

Add support for debug_dma_mapping_error() call to avoid warning from
debug_dma_unmap() interface when it checks for mapping error checked
status. Without this patch, device driver failed to check map error
warning is generated.

Signed-off-by: Shuah Khan <shuah.khan@hp.com>
---
 arch/mips/include/asm/dma-mapping.h |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/mips/include/asm/dma-mapping.h b/arch/mips/include/asm/dma-mapping.h
index be39a12..0929b08 100644
--- a/arch/mips/include/asm/dma-mapping.h
+++ b/arch/mips/include/asm/dma-mapping.h
@@ -40,6 +40,7 @@ static inline int dma_supported(struct device *dev, u64 mask)
 static inline int dma_mapping_error(struct device *dev, u64 mask)
 {
 	struct dma_map_ops *ops = get_dma_ops(dev);
+	debug_dma_mapping_error(dev, mask);
 	return ops->mapping_error(dev, mask);
 }
 
-- 
1.7.9.5

[PATCH RFT RESEND linux-next] mips: dma-mapping: support debug_dma_mapping_error

[Shuah Khan]

Add support for debug_dma_mapping_error() call to avoid warning from
debug_dma_unmap() interface when it checks for mapping error checked
status. Without this patch, device driver failed to check map error
warning is generated.

Signed-off-by: Shuah Khan <shuah.khan@hp.com>
---
 arch/mips/include/asm/dma-mapping.h |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/mips/include/asm/dma-mapping.h b/arch/mips/include/asm/dma-mapping.h
index be39a12..006b43e 100644
--- a/arch/mips/include/asm/dma-mapping.h
+++ b/arch/mips/include/asm/dma-mapping.h
@@ -40,6 +40,8 @@ static inline int dma_supported(struct device *dev, u64 mask)
 static inline int dma_mapping_error(struct device *dev, u64 mask)
 {
 	struct dma_map_ops *ops = get_dma_ops(dev);
+
+	debug_dma_mapping_error(dev, mask);
 	return ops->mapping_error(dev, mask);
 }
 
-- 
1.7.9.5

Re: [PATCH RFT RESEND linux-next] mips: dma-mapping: support debug_dma_mapping_error

[David Daney]

On 10/26/2012 09:01 AM, Shuah Khan wrote:
> Add support for debug_dma_mapping_error() call to avoid warning from
> debug_dma_unmap() interface when it checks for mapping error checked
> status. Without this patch, device driver failed to check map error
> warning is generated.
>
> Signed-off-by: Shuah Khan <shuah.khan@hp.com>
> ---
>   arch/mips/include/asm/dma-mapping.h |    2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/arch/mips/include/asm/dma-mapping.h b/arch/mips/include/asm/dma-mapping.h
> index be39a12..006b43e 100644
> --- a/arch/mips/include/asm/dma-mapping.h
> +++ b/arch/mips/include/asm/dma-mapping.h
> @@ -40,6 +40,8 @@ static inline int dma_supported(struct device *dev, u64 mask)
>   static inline int dma_mapping_error(struct device *dev, u64 mask)
>   {
>   	struct dma_map_ops *ops = get_dma_ops(dev);
> +
> +	debug_dma_mapping_error(dev, mask);
>   	return ops->mapping_error(dev, mask);
>   }
>
>

Although this is a start, I don't think it is sufficient.

As far as I can tell, there are many missing calls to debug_dma_*() in 
the various MIPS commone and sub-architecture DMA code.

Really you (or someone) needs to look at *all* the functions in 
arch/mips/asm/dma-mapping.h, and arch/mips/mm/dma-default.c and find 
places missing a debug_dma_*().

Thanks,
David Daney

Re: [PATCH RFT RESEND linux-next] mips: dma-mapping: support debug_dma_mapping_error

[Shuah Khan]

On Fri, 2012-10-26 at 09:45 -0700, David Daney wrote:
> On 10/26/2012 09:01 AM, Shuah Khan wrote:
> > Add support for debug_dma_mapping_error() call to avoid warning from
> > debug_dma_unmap() interface when it checks for mapping error checked
> > status. Without this patch, device driver failed to check map error
> > warning is generated.
> >
> > Signed-off-by: Shuah Khan <shuah.khan@hp.com>
> > ---
> >   arch/mips/include/asm/dma-mapping.h |    2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/mips/include/asm/dma-mapping.h b/arch/mips/include/asm/dma-mapping.h
> > index be39a12..006b43e 100644
> > --- a/arch/mips/include/asm/dma-mapping.h
> > +++ b/arch/mips/include/asm/dma-mapping.h
> > @@ -40,6 +40,8 @@ static inline int dma_supported(struct device *dev, u64 mask)
> >   static inline int dma_mapping_error(struct device *dev, u64 mask)
> >   {
> >   	struct dma_map_ops *ops = get_dma_ops(dev);
> > +
> > +	debug_dma_mapping_error(dev, mask);
> >   	return ops->mapping_error(dev, mask);
> >   }
> >
> >
> 
> Although this is a start, I don't think it is sufficient.
> 
> As far as I can tell, there are many missing calls to debug_dma_*() in 
> the various MIPS commone and sub-architecture DMA code.
> 
> Really you (or someone) needs to look at *all* the functions in 
> arch/mips/asm/dma-mapping.h, and arch/mips/mm/dma-default.c and find 
> places missing a debug_dma_*().

Is it correct to assume that this patch is not needed on MIPS until
debug_dma interfaces get added to MIPS common and sub-architecture DMA
code. 

When I didn't see dma_map_page() in arch/mips/include/asm/dma-mapping.h
defined, and just an extern, I incorrectly assumed, it is getting picked
up from <asm-generic/dma-mapping-common.h>, hence the need for this
patch in the first place.

-- Shuah

Re: [PATCH RFT RESEND linux-next] mips: dma-mapping: support debug_dma_mapping_error

[David Daney]

On 10/26/2012 10:06 AM, Shuah Khan wrote:
> On Fri, 2012-10-26 at 09:45 -0700, David Daney wrote:
>> On 10/26/2012 09:01 AM, Shuah Khan wrote:
>>> Add support for debug_dma_mapping_error() call to avoid warning from
>>> debug_dma_unmap() interface when it checks for mapping error checked
>>> status. Without this patch, device driver failed to check map error
>>> warning is generated.
>>>
>>> Signed-off-by: Shuah Khan <shuah.khan@hp.com>
>>> ---
>>>    arch/mips/include/asm/dma-mapping.h |    2 ++
>>>    1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/mips/include/asm/dma-mapping.h b/arch/mips/include/asm/dma-mapping.h
>>> index be39a12..006b43e 100644
>>> --- a/arch/mips/include/asm/dma-mapping.h
>>> +++ b/arch/mips/include/asm/dma-mapping.h
>>> @@ -40,6 +40,8 @@ static inline int dma_supported(struct device *dev, u64 mask)
>>>    static inline int dma_mapping_error(struct device *dev, u64 mask)
>>>    {
>>>    	struct dma_map_ops *ops = get_dma_ops(dev);
>>> +
>>> +	debug_dma_mapping_error(dev, mask);
>>>    	return ops->mapping_error(dev, mask);
>>>    }
>>>
>>>
>>
>> Although this is a start, I don't think it is sufficient.
>>
>> As far as I can tell, there are many missing calls to debug_dma_*() in
>> the various MIPS commone and sub-architecture DMA code.
>>
>> Really you (or someone) needs to look at *all* the functions in
>> arch/mips/asm/dma-mapping.h, and arch/mips/mm/dma-default.c and find
>> places missing a debug_dma_*().
>
> Is it correct to assume that this patch is not needed on MIPS until
> debug_dma interfaces get added to MIPS common and sub-architecture DMA
> code.

No, you have a false predicate here.

debug_dma_* *is* already mostly added.  The problem is that it is 
incomplete.  That is the nature of the problem.  Your patch makes it 
slightly better, but doesn't fully fix the problems.


>
> When I didn't see dma_map_page() in arch/mips/include/asm/dma-mapping.h
> defined, and just an extern, I incorrectly assumed, it is getting picked
> up from <asm-generic/dma-mapping-common.h>, hence the need for this
> patch in the first place.
>
> -- Shuah
>
>
>

Re: [PATCH RFT RESEND linux-next] mips: dma-mapping: support debug_dma_mapping_error

[Shuah Khan]

On Fri, 2012-10-26 at 13:07 -0700, David Daney wrote:
> On 10/26/2012 10:06 AM, Shuah Khan wrote:
> > On Fri, 2012-10-26 at 09:45 -0700, David Daney wrote:
> >> On 10/26/2012 09:01 AM, Shuah Khan wrote:
> >>> Add support for debug_dma_mapping_error() call to avoid warning from
> >>> debug_dma_unmap() interface when it checks for mapping error checked
> >>> status. Without this patch, device driver failed to check map error
> >>> warning is generated.
> >>>
> >>> Signed-off-by: Shuah Khan <shuah.khan@hp.com>
> >>> ---
> >>>    arch/mips/include/asm/dma-mapping.h |    2 ++
> >>>    1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/arch/mips/include/asm/dma-mapping.h b/arch/mips/include/asm/dma-mapping.h
> >>> index be39a12..006b43e 100644
> >>> --- a/arch/mips/include/asm/dma-mapping.h
> >>> +++ b/arch/mips/include/asm/dma-mapping.h
> >>> @@ -40,6 +40,8 @@ static inline int dma_supported(struct device *dev, u64 mask)
> >>>    static inline int dma_mapping_error(struct device *dev, u64 mask)
> >>>    {
> >>>    	struct dma_map_ops *ops = get_dma_ops(dev);
> >>> +
> >>> +	debug_dma_mapping_error(dev, mask);
> >>>    	return ops->mapping_error(dev, mask);
> >>>    }
> >>>
> >>>
> >>
> >> Although this is a start, I don't think it is sufficient.
> >>
> >> As far as I can tell, there are many missing calls to debug_dma_*() in
> >> the various MIPS commone and sub-architecture DMA code.
> >>
> >> Really you (or someone) needs to look at *all* the functions in
> >> arch/mips/asm/dma-mapping.h, and arch/mips/mm/dma-default.c and find
> >> places missing a debug_dma_*().
> >
> > Is it correct to assume that this patch is not needed on MIPS until
> > debug_dma interfaces get added to MIPS common and sub-architecture DMA
> > code.
> 
> No, you have a false predicate here.
> 
> debug_dma_* *is* already mostly added.  The problem is that it is 
> incomplete.  That is the nature of the problem.  Your patch makes it 
> slightly better, but doesn't fully fix the problems.

Got it. Thanks. I would volunteer to look at fixing all the problems,
but unfortunately I don't have a MIPS box handy for a thorough testing
that would be necessary for such a change.

-- Shuah
> 
> 
> >
> > When I didn't see dma_map_page() in arch/mips/include/asm/dma-mapping.h
> > defined, and just an extern, I incorrectly assumed, it is getting picked
> > up from <asm-generic/dma-mapping-common.h>, hence the need for this
> > patch in the first place.
> >
> > -- Shuah
> >
> >
> >
> 

Re: [PATCH RFT RESEND linux-next] mips: dma-mapping: support debug_dma_mapping_error

[David Daney]

On 10/26/2012 02:51 PM, Shuah Khan wrote:

>>>> On 10/26/2012 09:01 AM, Shuah Khan wrote:
>>>>> Add support for debug_dma_mapping_error() call to avoid warning from
>>>>> debug_dma_unmap() interface when it checks for mapping error checked
>>>>> status. Without this patch, device driver failed to check map error
>>>>> warning is generated.

I'm confused.

Your claim that a 'warning is generated' seems to be in conflict with...


[...]
> Got it. Thanks. I would volunteer to look at fixing all the problems,
> but unfortunately I don't have a MIPS box handy

This statement that you don't have hardware that exhibits the problem.

How was the patch tested?  How do you even know there is a problem?

David Daney

Re: [PATCH RFT RESEND linux-next] mips: dma-mapping: support debug_dma_mapping_error

[Shuah Khan]

On Fri, 2012-10-26 at 15:31 -0700, David Daney wrote:
> On 10/26/2012 02:51 PM, Shuah Khan wrote:
> 
> >>>> On 10/26/2012 09:01 AM, Shuah Khan wrote:
> >>>>> Add support for debug_dma_mapping_error() call to avoid warning from
> >>>>> debug_dma_unmap() interface when it checks for mapping error checked
> >>>>> status. Without this patch, device driver failed to check map error
> >>>>> warning is generated.
> 
> I'm confused.
> 
> Your claim that a 'warning is generated' seems to be in conflict with...
> 
> 
> [...]
> > Got it. Thanks. I would volunteer to look at fixing all the problems,
> > but unfortunately I don't have a MIPS box handy
> 
> This statement that you don't have hardware that exhibits the problem.
> 
> How was the patch tested?  How do you even know there is a problem?

I enhanced the existing dma debug interfaces to add an interface to
debug missing dma_mapping_error() checks. That went into linux-next.
With this patch, when dma_map_page() and dma_map_single() are debugged
with a call to debug_dma_map_page() and the corresponding
dma_mapping_error() interface doesn't call debug_dma_mapping_error()
interface, a warning will be generated. I have been sending changes to
arch specific dma_mapping_error() routines to add this debug interface.
That is why I marked this patch RFT requesting testing.

Thanks,
-- Shuah

Re: [PATCH RFT RESEND linux-next] mips: dma-mapping: support debug_dma_mapping_error

[Shuah Khan]

On Tue, 2012-10-30 at 13:58 -0600, Shuah Khan wrote:
> On Fri, 2012-10-26 at 15:31 -0700, David Daney wrote:
> > On 10/26/2012 02:51 PM, Shuah Khan wrote:
> > 
> > >>>> On 10/26/2012 09:01 AM, Shuah Khan wrote:
> > >>>>> Add support for debug_dma_mapping_error() call to avoid warning from
> > >>>>> debug_dma_unmap() interface when it checks for mapping error checked
> > >>>>> status. Without this patch, device driver failed to check map error
> > >>>>> warning is generated.
> > 
> > I'm confused.
> > 
> > Your claim that a 'warning is generated' seems to be in conflict with...
> > 
> > 
> > [...]
> > > Got it. Thanks. I would volunteer to look at fixing all the problems,
> > > but unfortunately I don't have a MIPS box handy
> > 
> > This statement that you don't have hardware that exhibits the problem.
> > 
> > How was the patch tested?  How do you even know there is a problem?
> 
> I enhanced the existing dma debug interfaces to add an interface to
> debug missing dma_mapping_error() checks. That went into linux-next.
> With this patch, when dma_map_page() and dma_map_single() are debugged
> with a call to debug_dma_map_page() and the corresponding
> dma_mapping_error() interface doesn't call debug_dma_mapping_error()
> interface, a warning will be generated. I have been sending changes to
> arch specific dma_mapping_error() routines to add this debug interface.
> That is why I marked this patch RFT requesting testing.


Marek,

This one is for mips to go through your tree with the other arch patches
for debug_dma_mapping_error().

Thanks,
-- Shuah