2012-10-25 23:36:38

by Shuah Khan

[permalink] [raw]
Subject: [PATCH RFT] mips: dma-mapping: support debug_dma_mapping_error

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 <[email protected]>
---
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



2012-10-26 16:01:43

by Shuah Khan

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

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 <[email protected]>
---
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


2012-10-26 16:45:27

by David Daney

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

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 <[email protected]>
> ---
> 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

2012-10-26 17:06:49

by Shuah Khan

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

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 <[email protected]>
> > ---
> > 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

2012-10-26 20:07:09

by David Daney

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

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 <[email protected]>
>>> ---
>>> 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
>
>
>

2012-10-26 21:51:10

by Shuah Khan

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

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 <[email protected]>
> >>> ---
> >>> 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
> >
> >
> >
>

2012-10-26 22:31:42

by David Daney

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

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

2012-10-30 19:58:54

by Shuah Khan

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

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

2012-11-15 18:21:27

by Shuah Khan

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

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