Re: [PATCH] staging: android: ion: Initialize dma_address of new sg list
On Fri, Feb 16, 2018 at 09:57:12AM -0800, Liam Mark wrote:
> On Fri, 16 Feb 2018, Greg KH wrote:
>
> > On Fri, Feb 09, 2018 at 10:16:56PM -0800, Liam Mark wrote:
> > > Fix the dup_sg_table function to initialize the dma_address of the new
> > > sg list entries instead of the source dma_address entries.
> > >
> > > Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
> >
> > So this should be sent to the stable trees as well, right?
> >
> > Please add that when you resend...
>
> My understanding was that I should only send this to the stable branch if
> it fixes a real bug.
>
> Both myself and Laura can't see any actual problems that result from this
> issue, the change is more to help future proof the code.
>
> My commit message wasn't clear about this and could have mislead people
> into thinking there was a bug.
> I will update the commit message to make it clear that this issue doesn't
> currently result in any problem.
>
> Do you still want me to send it to the stable trees?
If it's not a bugfix, no need to. But please clean up your changelog
text as it implies that it is a bugfix...
thanks,
greg k-h
Re: [PATCH] staging: android: ion: Initialize dma_address of new sg list
On Fri, Feb 16, 2018 at 09:57:12AM -0800, Liam Mark wrote:
> On Fri, 16 Feb 2018, Greg KH wrote:
>
> > On Fri, Feb 09, 2018 at 10:16:56PM -0800, Liam Mark wrote:
> > > Fix the dup_sg_table function to initialize the dma_address of the new
> > > sg list entries instead of the source dma_address entries.
> > >
> > > Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
> >
> > So this should be sent to the stable trees as well, right?
> >
> > Please add that when you resend...
>
> My understanding was that I should only send this to the stable branch if
> it fixes a real bug.
>
> Both myself and Laura can't see any actual problems that result from this
> issue, the change is more to help future proof the code.
>
> My commit message wasn't clear about this and could have mislead people
> into thinking there was a bug.
> I will update the commit message to make it clear that this issue doesn't
> currently result in any problem.
>
> Do you still want me to send it to the stable trees?
If it's not a bugfix, no need to. But please clean up your changelog
text as it implies that it is a bugfix...
thanks,
greg k-h
Re: [PATCH] staging: android: ion: Initialize dma_address of new sg list
On Fri, 16 Feb 2018, Greg KH wrote:
> On Fri, Feb 09, 2018 at 10:16:56PM -0800, Liam Mark wrote:
> > Fix the dup_sg_table function to initialize the dma_address of the new
> > sg list entries instead of the source dma_address entries.
> >
> > Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
>
> So this should be sent to the stable trees as well, right?
>
> Please add that when you resend...
My understanding was that I should only send this to the stable branch if
it fixes a real bug.
Both myself and Laura can't see any actual problems that result from this
issue, the change is more to help future proof the code.
My commit message wasn't clear about this and could have mislead people
into thinking there was a bug.
I will update the commit message to make it clear that this issue doesn't
currently result in any problem.
Do you still want me to send it to the stable trees?
Thanks,
Liam
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH] staging: android: ion: Initialize dma_address of new sg list
On Fri, 16 Feb 2018, Greg KH wrote:
> On Fri, Feb 09, 2018 at 10:16:56PM -0800, Liam Mark wrote:
> > Fix the dup_sg_table function to initialize the dma_address of the new
> > sg list entries instead of the source dma_address entries.
> >
> > Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
>
> So this should be sent to the stable trees as well, right?
>
> Please add that when you resend...
My understanding was that I should only send this to the stable branch if
it fixes a real bug.
Both myself and Laura can't see any actual problems that result from this
issue, the change is more to help future proof the code.
My commit message wasn't clear about this and could have mislead people
into thinking there was a bug.
I will update the commit message to make it clear that this issue doesn't
currently result in any problem.
Do you still want me to send it to the stable trees?
Thanks,
Liam
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH] staging: android: ion: Initialize dma_address of new sg list
On Thu, 15 Feb 2018, Laura Abbott wrote:
> On 02/12/2018 01:25 PM, Liam Mark wrote:
> >
> > On Mon, 12 Feb 2018, Dan Carpenter wrote:
> >
> > > On Fri, Feb 09, 2018 at 10:16:56PM -0800, Liam Mark wrote:
> > > > Fix the dup_sg_table function to initialize the dma_address of the new
> > > > sg list entries instead of the source dma_address entries.
> > > >
> > > > Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
> > > > Signed-off-by: Liam Mark
> > >
> > > How did you find this bug? What are the user visible effects of this
> > > bug? I'm probably going to ask you to send a v2 patch with a new
> > > changelog depending on the answers to these questions.
> >
> > I noticed the bug when reading through the code, I haven?t seen any
> > visible effects of this bug.
> >
> > From looking at the code I don?t see any obvious ways that this bug
> would
> > introduce any issues with the current code base.
> >
> > Liam
> >
>
> Given the way we duplicate the sg_list this should be harmless but you
> are right it's cleaner if we initialize the new list. You can add my
> Ack when you update with a new change log clarifying this.
>
> Thanks,
> Laura
>
Will do.
Thanks,
Liam
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH] staging: android: ion: Initialize dma_address of new sg list
On Thu, 15 Feb 2018, Laura Abbott wrote:
> On 02/12/2018 01:25 PM, Liam Mark wrote:
> >
> > On Mon, 12 Feb 2018, Dan Carpenter wrote:
> >
> > > On Fri, Feb 09, 2018 at 10:16:56PM -0800, Liam Mark wrote:
> > > > Fix the dup_sg_table function to initialize the dma_address of the new
> > > > sg list entries instead of the source dma_address entries.
> > > >
> > > > Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
> > > > Signed-off-by: Liam Mark
> > >
> > > How did you find this bug? What are the user visible effects of this
> > > bug? I'm probably going to ask you to send a v2 patch with a new
> > > changelog depending on the answers to these questions.
> >
> > I noticed the bug when reading through the code, I haven?t seen any
> > visible effects of this bug.
> >
> > From looking at the code I don?t see any obvious ways that this bug
> would
> > introduce any issues with the current code base.
> >
> > Liam
> >
>
> Given the way we duplicate the sg_list this should be harmless but you
> are right it's cleaner if we initialize the new list. You can add my
> Ack when you update with a new change log clarifying this.
>
> Thanks,
> Laura
>
Will do.
Thanks,
Liam
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH] staging: android: ion: Initialize dma_address of new sg list
On Fri, Feb 09, 2018 at 10:16:56PM -0800, Liam Mark wrote:
> Fix the dup_sg_table function to initialize the dma_address of the new
> sg list entries instead of the source dma_address entries.
>
> Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
So this should be sent to the stable trees as well, right?
Please add that when you resend...
thanks,
greg k-h
Re: [PATCH] staging: android: ion: Initialize dma_address of new sg list
On Fri, Feb 09, 2018 at 10:16:56PM -0800, Liam Mark wrote:
> Fix the dup_sg_table function to initialize the dma_address of the new
> sg list entries instead of the source dma_address entries.
>
> Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
So this should be sent to the stable trees as well, right?
Please add that when you resend...
thanks,
greg k-h
Re: [PATCH] staging: android: ion: Initialize dma_address of new sg list
On 02/12/2018 01:25 PM, Liam Mark wrote:
On Mon, 12 Feb 2018, Dan Carpenter wrote:
On Fri, Feb 09, 2018 at 10:16:56PM -0800, Liam Mark wrote:
Fix the dup_sg_table function to initialize the dma_address of the new
sg list entries instead of the source dma_address entries.
Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
Signed-off-by: Liam Mark
How did you find this bug? What are the user visible effects of this
bug? I'm probably going to ask you to send a v2 patch with a new
changelog depending on the answers to these questions.
I noticed the bug when reading through the code, I haven’t seen any
visible effects of this bug.
From looking at the code I don’t see any obvious ways that this bug would
introduce any issues with the current code base.
Liam
Given the way we duplicate the sg_list this should be harmless but you
are right it's cleaner if we initialize the new list. You can add my
Ack when you update with a new change log clarifying this.
Thanks,
Laura
Re: [PATCH] staging: android: ion: Initialize dma_address of new sg list
On 02/12/2018 01:25 PM, Liam Mark wrote:
On Mon, 12 Feb 2018, Dan Carpenter wrote:
On Fri, Feb 09, 2018 at 10:16:56PM -0800, Liam Mark wrote:
Fix the dup_sg_table function to initialize the dma_address of the new
sg list entries instead of the source dma_address entries.
Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
Signed-off-by: Liam Mark
How did you find this bug? What are the user visible effects of this
bug? I'm probably going to ask you to send a v2 patch with a new
changelog depending on the answers to these questions.
I noticed the bug when reading through the code, I haven’t seen any
visible effects of this bug.
From looking at the code I don’t see any obvious ways that this bug would
introduce any issues with the current code base.
Liam
Given the way we duplicate the sg_list this should be harmless but you
are right it's cleaner if we initialize the new list. You can add my
Ack when you update with a new change log clarifying this.
Thanks,
Laura
Re: [PATCH] staging: android: ion: Initialize dma_address of new sg list
On Mon, 12 Feb 2018, Dan Carpenter wrote:
> On Fri, Feb 09, 2018 at 10:16:56PM -0800, Liam Mark wrote:
> > Fix the dup_sg_table function to initialize the dma_address of the new
> > sg list entries instead of the source dma_address entries.
> >
> > Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
> > Signed-off-by: Liam Mark
>
> How did you find this bug? What are the user visible effects of this
> bug? I'm probably going to ask you to send a v2 patch with a new
> changelog depending on the answers to these questions.
I noticed the bug when reading through the code, I haven’t seen any
visible effects of this bug.
>From looking at the code I don’t see any obvious ways that this bug would
introduce any issues with the current code base.
Liam
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH] staging: android: ion: Initialize dma_address of new sg list
On Mon, 12 Feb 2018, Dan Carpenter wrote:
> On Fri, Feb 09, 2018 at 10:16:56PM -0800, Liam Mark wrote:
> > Fix the dup_sg_table function to initialize the dma_address of the new
> > sg list entries instead of the source dma_address entries.
> >
> > Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
> > Signed-off-by: Liam Mark
>
> How did you find this bug? What are the user visible effects of this
> bug? I'm probably going to ask you to send a v2 patch with a new
> changelog depending on the answers to these questions.
I noticed the bug when reading through the code, I haven’t seen any
visible effects of this bug.
>From looking at the code I don’t see any obvious ways that this bug would
introduce any issues with the current code base.
Liam
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH] staging: android: ion: Initialize dma_address of new sg list
On Fri, Feb 09, 2018 at 10:16:56PM -0800, Liam Mark wrote:
> Fix the dup_sg_table function to initialize the dma_address of the new
> sg list entries instead of the source dma_address entries.
>
> Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
> Signed-off-by: Liam Mark
How did you find this bug? What are the user visible effects of this
bug? I'm probably going to ask you to send a v2 patch with a new
changelog depending on the answers to these questions.
regards,
dan carpenter
Re: [PATCH] staging: android: ion: Initialize dma_address of new sg list
On Fri, Feb 09, 2018 at 10:16:56PM -0800, Liam Mark wrote:
> Fix the dup_sg_table function to initialize the dma_address of the new
> sg list entries instead of the source dma_address entries.
>
> Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
> Signed-off-by: Liam Mark
How did you find this bug? What are the user visible effects of this
bug? I'm probably going to ask you to send a v2 patch with a new
changelog depending on the answers to these questions.
regards,
dan carpenter
[PATCH] staging: android: ion: Initialize dma_address of new sg list
Fix the dup_sg_table function to initialize the dma_address of the new
sg list entries instead of the source dma_address entries.
Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
Signed-off-by: Liam Mark
---
drivers/staging/android/ion/ion.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index f480885e346b..3ace3a0d9210 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -197,7 +197,7 @@ static struct sg_table *dup_sg_table(struct sg_table *table)
new_sg = new_table->sgl;
for_each_sg(table->sgl, sg, table->nents, i) {
memcpy(new_sg, sg, sizeof(*sg));
- sg->dma_address = 0;
+ new_sg->dma_address = 0;
new_sg = sg_next(new_sg);
}
--
1.8.5.2
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
[PATCH] staging: android: ion: Initialize dma_address of new sg list
Fix the dup_sg_table function to initialize the dma_address of the new
sg list entries instead of the source dma_address entries.
Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
Signed-off-by: Liam Mark
---
drivers/staging/android/ion/ion.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index f480885e346b..3ace3a0d9210 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -197,7 +197,7 @@ static struct sg_table *dup_sg_table(struct sg_table *table)
new_sg = new_table->sgl;
for_each_sg(table->sgl, sg, table->nents, i) {
memcpy(new_sg, sg, sizeof(*sg));
- sg->dma_address = 0;
+ new_sg->dma_address = 0;
new_sg = sg_next(new_sg);
}
--
1.8.5.2
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
