Re: [Intel-gfx] [PATCH] dma_fence_array: Fix PENDING_ERROR leak in dma_fence_array_signaled()
(Switching to my @linux.intel.com address)
Quoting Christian König (2021-11-29 14:55:37)
> Am 29.11.21 um 13:46 schrieb Thomas Hellström:
> > On Mon, 2021-11-29 at 13:33 +0100, Christian König wrote:
> >> Am 29.11.21 um 13:23 schrieb Thomas Hellström:
> >>> Hi, Christian,
> >>>
> >>> On Mon, 2021-11-29 at 09:21 +0100, Christian König wrote:
> Am 29.11.21 um 08:35 schrieb Thomas Hellström:
> > If a dma_fence_array is reported signaled by a call to
> > dma_fence_is_signaled(), it may leak the PENDING_ERROR status.
> >
> > Fix this by clearing the PENDING_ERROR status if we return true
> > in
> > dma_fence_array_signaled().
> >
> > Fixes: 1f70b8b812f3 ("dma-fence: Propagate errors to dma-fence-
> > array container")
> > Cc: linaro-mm-...@lists.linaro.org
> > Cc: Christian König
> > Cc: Chris Wilson
> > Signed-off-by: Thomas Hellström
> >
> Reviewed-by: Christian König
> >>> How are the dma-buf / dma-fence patches typically merged? If i915
> >>> is
> >>> the only fence->error user, could we take this through drm-intel to
> >>> avoid a backmerge for upcoming i915 work?
> >> Well that one here looks like a bugfix to me, so either through
> >> drm-misc-fixes ore some i915 -fixes branch sounds fine to me.
> >>
> >> If you have any new development based on that a backmerge of the -
> >> fixes
> >> into your -next branch is unavoidable anyway.
> > Ok, I'll check with Joonas if I can take it through
> > drm-intel-gt-next, since fixes are cherry-picked from that one. Patch
> > will then appear in both the -fixes and the -next branch.
>
> Well exactly that's the stuff Daniel told me to avoid :)
>
> But maybe your i915 workflow is somehow better handling that than the
> AMD workflow.
If it's a bugfix to a patch that merged through drm-misc-next, I'd
always be inclined to merge the fixup using the same process (which
would be drm-next-fixes).
In i915 we do always merge the patches to -next first, and never do a
backmerge of -fixes (as it's a cherry-picked branch) so the workflows
differ there.
Here the time between the fixup and the previous patch is so long that
either way is fine with. So feel free to apply to drm-intel-gt-next.
Regards, Joonas
> Christian.
>
> >
> > Thanks,
> > /Thomas
> >
> >
> >> Regards,
> >> Christian.
> >>
> >>> /Thomas
> >>>
> >>>
> > ---
> > drivers/dma-buf/dma-fence-array.c | 6 +-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/dma-buf/dma-fence-array.c b/drivers/dma-
> > buf/dma-fence-array.c
> > index d3fbd950be94..3e07f961e2f3 100644
> > --- a/drivers/dma-buf/dma-fence-array.c
> > +++ b/drivers/dma-buf/dma-fence-array.c
> > @@ -104,7 +104,11 @@ static bool
> > dma_fence_array_signaled(struct
> > dma_fence *fence)
> > {
> > struct dma_fence_array *array =
> > to_dma_fence_array(fence);
> >
> > - return atomic_read(>num_pending) <= 0;
> > + if (atomic_read(>num_pending) > 0)
> > + return false;
> > +
> > + dma_fence_array_clear_pending_error(array);
> > + return true;
> > }
> >
> > static void dma_fence_array_release(struct dma_fence *fence)
> >
>
Re: [Intel-gfx] [PATCH] dma_fence_array: Fix PENDING_ERROR leak in dma_fence_array_signaled()
Am 29.11.21 um 13:46 schrieb Thomas Hellström:
On Mon, 2021-11-29 at 13:33 +0100, Christian König wrote:
Am 29.11.21 um 13:23 schrieb Thomas Hellström:
Hi, Christian,
On Mon, 2021-11-29 at 09:21 +0100, Christian König wrote:
Am 29.11.21 um 08:35 schrieb Thomas Hellström:
If a dma_fence_array is reported signaled by a call to
dma_fence_is_signaled(), it may leak the PENDING_ERROR status.
Fix this by clearing the PENDING_ERROR status if we return true
in
dma_fence_array_signaled().
Fixes: 1f70b8b812f3 ("dma-fence: Propagate errors to dma-fence-
array container")
Cc: linaro-mm-...@lists.linaro.org
Cc: Christian König
Cc: Chris Wilson
Signed-off-by: Thomas Hellström
Reviewed-by: Christian König
How are the dma-buf / dma-fence patches typically merged? If i915
is
the only fence->error user, could we take this through drm-intel to
avoid a backmerge for upcoming i915 work?
Well that one here looks like a bugfix to me, so either through
drm-misc-fixes ore some i915 -fixes branch sounds fine to me.
If you have any new development based on that a backmerge of the -
fixes
into your -next branch is unavoidable anyway.
Ok, I'll check with Joonas if I can take it through
drm-intel-gt-next, since fixes are cherry-picked from that one. Patch
will then appear in both the -fixes and the -next branch.
Well exactly that's the stuff Daniel told me to avoid :)
But maybe your i915 workflow is somehow better handling that than the
AMD workflow.
Christian.
Thanks,
/Thomas
Regards,
Christian.
/Thomas
---
drivers/dma-buf/dma-fence-array.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-fence-array.c b/drivers/dma-
buf/dma-fence-array.c
index d3fbd950be94..3e07f961e2f3 100644
--- a/drivers/dma-buf/dma-fence-array.c
+++ b/drivers/dma-buf/dma-fence-array.c
@@ -104,7 +104,11 @@ static bool
dma_fence_array_signaled(struct
dma_fence *fence)
{
struct dma_fence_array *array =
to_dma_fence_array(fence);
- return atomic_read(>num_pending) <= 0;
+ if (atomic_read(>num_pending) > 0)
+ return false;
+
+ dma_fence_array_clear_pending_error(array);
+ return true;
}
static void dma_fence_array_release(struct dma_fence *fence)
Re: [Intel-gfx] [PATCH] dma_fence_array: Fix PENDING_ERROR leak in dma_fence_array_signaled()
Am 29.11.21 um 08:35 schrieb Thomas Hellström:
If a dma_fence_array is reported signaled by a call to
dma_fence_is_signaled(), it may leak the PENDING_ERROR status.
Fix this by clearing the PENDING_ERROR status if we return true in
dma_fence_array_signaled().
Fixes: 1f70b8b812f3 ("dma-fence: Propagate errors to dma-fence-array container")
Cc: linaro-mm-...@lists.linaro.org
Cc: Christian König
Cc: Chris Wilson
Signed-off-by: Thomas Hellström
Reviewed-by: Christian König
---
drivers/dma-buf/dma-fence-array.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-fence-array.c
b/drivers/dma-buf/dma-fence-array.c
index d3fbd950be94..3e07f961e2f3 100644
--- a/drivers/dma-buf/dma-fence-array.c
+++ b/drivers/dma-buf/dma-fence-array.c
@@ -104,7 +104,11 @@ static bool dma_fence_array_signaled(struct dma_fence
*fence)
{
struct dma_fence_array *array = to_dma_fence_array(fence);
- return atomic_read(>num_pending) <= 0;
+ if (atomic_read(>num_pending) > 0)
+ return false;
+
+ dma_fence_array_clear_pending_error(array);
+ return true;
}
static void dma_fence_array_release(struct dma_fence *fence)
Re: [Intel-gfx] [PATCH] dma_fence_array: Fix PENDING_ERROR leak in dma_fence_array_signaled()
Am 29.11.21 um 13:23 schrieb Thomas Hellström:
Hi, Christian,
On Mon, 2021-11-29 at 09:21 +0100, Christian König wrote:
Am 29.11.21 um 08:35 schrieb Thomas Hellström:
If a dma_fence_array is reported signaled by a call to
dma_fence_is_signaled(), it may leak the PENDING_ERROR status.
Fix this by clearing the PENDING_ERROR status if we return true in
dma_fence_array_signaled().
Fixes: 1f70b8b812f3 ("dma-fence: Propagate errors to dma-fence-
array container")
Cc: linaro-mm-...@lists.linaro.org
Cc: Christian König
Cc: Chris Wilson
Signed-off-by: Thomas Hellström
Reviewed-by: Christian König
How are the dma-buf / dma-fence patches typically merged? If i915 is
the only fence->error user, could we take this through drm-intel to
avoid a backmerge for upcoming i915 work?
Well that one here looks like a bugfix to me, so either through
drm-misc-fixes ore some i915 -fixes branch sounds fine to me.
If you have any new development based on that a backmerge of the -fixes
into your -next branch is unavoidable anyway.
Regards,
Christian.
/Thomas
---
drivers/dma-buf/dma-fence-array.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-fence-array.c b/drivers/dma-
buf/dma-fence-array.c
index d3fbd950be94..3e07f961e2f3 100644
--- a/drivers/dma-buf/dma-fence-array.c
+++ b/drivers/dma-buf/dma-fence-array.c
@@ -104,7 +104,11 @@ static bool dma_fence_array_signaled(struct
dma_fence *fence)
{
struct dma_fence_array *array = to_dma_fence_array(fence);
- return atomic_read(>num_pending) <= 0;
+ if (atomic_read(>num_pending) > 0)
+ return false;
+
+ dma_fence_array_clear_pending_error(array);
+ return true;
}
static void dma_fence_array_release(struct dma_fence *fence)
Re: [Intel-gfx] [PATCH] dma_fence_array: Fix PENDING_ERROR leak in dma_fence_array_signaled()
On Mon, 2021-11-29 at 13:33 +0100, Christian König wrote:
> Am 29.11.21 um 13:23 schrieb Thomas Hellström:
> > Hi, Christian,
> >
> > On Mon, 2021-11-29 at 09:21 +0100, Christian König wrote:
> > > Am 29.11.21 um 08:35 schrieb Thomas Hellström:
> > > > If a dma_fence_array is reported signaled by a call to
> > > > dma_fence_is_signaled(), it may leak the PENDING_ERROR status.
> > > >
> > > > Fix this by clearing the PENDING_ERROR status if we return true
> > > > in
> > > > dma_fence_array_signaled().
> > > >
> > > > Fixes: 1f70b8b812f3 ("dma-fence: Propagate errors to dma-fence-
> > > > array container")
> > > > Cc: linaro-mm-...@lists.linaro.org
> > > > Cc: Christian König
> > > > Cc: Chris Wilson
> > > > Signed-off-by: Thomas Hellström
> > > >
> > > Reviewed-by: Christian König
> > How are the dma-buf / dma-fence patches typically merged? If i915
> > is
> > the only fence->error user, could we take this through drm-intel to
> > avoid a backmerge for upcoming i915 work?
>
> Well that one here looks like a bugfix to me, so either through
> drm-misc-fixes ore some i915 -fixes branch sounds fine to me.
>
> If you have any new development based on that a backmerge of the -
> fixes
> into your -next branch is unavoidable anyway.
Ok, I'll check with Joonas if I can take it through
drm-intel-gt-next, since fixes are cherry-picked from that one. Patch
will then appear in both the -fixes and the -next branch.
Thanks,
/Thomas
>
> Regards,
> Christian.
>
> >
> > /Thomas
> >
> >
> > > > ---
> > > > drivers/dma-buf/dma-fence-array.c | 6 +-
> > > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/dma-buf/dma-fence-array.c b/drivers/dma-
> > > > buf/dma-fence-array.c
> > > > index d3fbd950be94..3e07f961e2f3 100644
> > > > --- a/drivers/dma-buf/dma-fence-array.c
> > > > +++ b/drivers/dma-buf/dma-fence-array.c
> > > > @@ -104,7 +104,11 @@ static bool
> > > > dma_fence_array_signaled(struct
> > > > dma_fence *fence)
> > > > {
> > > > struct dma_fence_array *array =
> > > > to_dma_fence_array(fence);
> > > >
> > > > - return atomic_read(>num_pending) <= 0;
> > > > + if (atomic_read(>num_pending) > 0)
> > > > + return false;
> > > > +
> > > > + dma_fence_array_clear_pending_error(array);
> > > > + return true;
> > > > }
> > > >
> > > > static void dma_fence_array_release(struct dma_fence *fence)
> >
>
Re: [Intel-gfx] [PATCH] dma_fence_array: Fix PENDING_ERROR leak in dma_fence_array_signaled()
Hi, Christian,
On Mon, 2021-11-29 at 09:21 +0100, Christian König wrote:
> Am 29.11.21 um 08:35 schrieb Thomas Hellström:
> > If a dma_fence_array is reported signaled by a call to
> > dma_fence_is_signaled(), it may leak the PENDING_ERROR status.
> >
> > Fix this by clearing the PENDING_ERROR status if we return true in
> > dma_fence_array_signaled().
> >
> > Fixes: 1f70b8b812f3 ("dma-fence: Propagate errors to dma-fence-
> > array container")
> > Cc: linaro-mm-...@lists.linaro.org
> > Cc: Christian König
> > Cc: Chris Wilson
> > Signed-off-by: Thomas Hellström
>
> Reviewed-by: Christian König
How are the dma-buf / dma-fence patches typically merged? If i915 is
the only fence->error user, could we take this through drm-intel to
avoid a backmerge for upcoming i915 work?
/Thomas
>
> > ---
> > drivers/dma-buf/dma-fence-array.c | 6 +-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/dma-buf/dma-fence-array.c b/drivers/dma-
> > buf/dma-fence-array.c
> > index d3fbd950be94..3e07f961e2f3 100644
> > --- a/drivers/dma-buf/dma-fence-array.c
> > +++ b/drivers/dma-buf/dma-fence-array.c
> > @@ -104,7 +104,11 @@ static bool dma_fence_array_signaled(struct
> > dma_fence *fence)
> > {
> > struct dma_fence_array *array = to_dma_fence_array(fence);
> >
> > - return atomic_read(>num_pending) <= 0;
> > + if (atomic_read(>num_pending) > 0)
> > + return false;
> > +
> > + dma_fence_array_clear_pending_error(array);
> > + return true;
> > }
> >
> > static void dma_fence_array_release(struct dma_fence *fence)
>
[Intel-gfx] [PATCH] dma_fence_array: Fix PENDING_ERROR leak in dma_fence_array_signaled()
If a dma_fence_array is reported signaled by a call to
dma_fence_is_signaled(), it may leak the PENDING_ERROR status.
Fix this by clearing the PENDING_ERROR status if we return true in
dma_fence_array_signaled().
Fixes: 1f70b8b812f3 ("dma-fence: Propagate errors to dma-fence-array container")
Cc: linaro-mm-...@lists.linaro.org
Cc: Christian König
Cc: Chris Wilson
Signed-off-by: Thomas Hellström
---
drivers/dma-buf/dma-fence-array.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-fence-array.c
b/drivers/dma-buf/dma-fence-array.c
index d3fbd950be94..3e07f961e2f3 100644
--- a/drivers/dma-buf/dma-fence-array.c
+++ b/drivers/dma-buf/dma-fence-array.c
@@ -104,7 +104,11 @@ static bool dma_fence_array_signaled(struct dma_fence
*fence)
{
struct dma_fence_array *array = to_dma_fence_array(fence);
- return atomic_read(>num_pending) <= 0;
+ if (atomic_read(>num_pending) > 0)
+ return false;
+
+ dma_fence_array_clear_pending_error(array);
+ return true;
}
static void dma_fence_array_release(struct dma_fence *fence)
--
2.31.1
