[Mesa-dev] [PATCH v2] dri3: Prevent multiple freeing of buffers.

[Sergii Romantsov]

Hello, Michel

On 2018-04-10 09:44 AM, Sergii Romantsov wrote:
> Commit 3160cb86aa92 adds optimization with flag 'reallocate'.
> Processing of flag causes buffers freeing while pointer
> is still hold in caller stack and than again used to be freed.
>
> Fixes: 3160cb86aa92 "egl/x11: Re-allocate buffers if format is suboptimal"
>
> v2:
>  used flag 'busy' instead of introducing new one.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105906
> Signed-off-by: Sergii Romantsov <sergii.romantsov at globallogic.com>
> Tested-by: Andriy Khulap <andriy.khulap at globallogic.com>
> ---
>  src/loader/loader_dri3_helper.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_
helper.c
> index fe17df1..a934db1 100644
> --- a/src/loader/loader_dri3_helper.c
> +++ b/src/loader/loader_dri3_helper.c
> @@ -1688,6 +1688,7 @@ dri3_get_buffer(__DRIdrawable *driDrawable,
>             (buffer_type == loader_dri3_buffer_front &&
draw->have_fake_front))
>            && buffer) {
>
> +         buffer->busy = true;
>           /* Fill the new buffer with data from an old buffer */
>           dri3_fence_await(draw->conn, draw, buffer);
>           if (!loader_dri3_blit_image(draw,

Hello, Michel,
I've updated patch according to your remarks:
https://lists.freedesktop.org/archives/mesa-dev/2018-April/193331.html

And about question

> This seems a bit of a hack. Will this always be cleared again, in
> particular if it's the fake front buffer?
>
There are seems no needs in clearing due the next step is unconditional
freeing of that buffer by call 'dri3_free_render_buffer(draw, buffer)'.


On Thu, Apr 26, 2018 at 7:43 PM, Michel Dänzer <michel at daenzer.net> wrote:

> On 2018-04-10 09:44 AM, Sergii Romantsov wrote:
> > Commit 3160cb86aa92 adds optimization with flag 'reallocate'.
> > Processing of flag causes buffers freeing while pointer
> > is still hold in caller stack and than again used to be freed.
> >
> > Fixes: 3160cb86aa92 "egl/x11: Re-allocate buffers if format is
> suboptimal"
> >
> > v2:
> >  used flag 'busy' instead of introducing new one.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105906
> > Signed-off-by: Sergii Romantsov <sergii.romantsov at globallogic.com>
> > Tested-by: Andriy Khulap <andriy.khulap at globallogic.com>
> > ---
> >  src/loader/loader_dri3_helper.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_
> helper.c
> > index fe17df1..a934db1 100644
> > --- a/src/loader/loader_dri3_helper.c
> > +++ b/src/loader/loader_dri3_helper.c
> > @@ -1688,6 +1688,7 @@ dri3_get_buffer(__DRIdrawable *driDrawable,
> >             (buffer_type == loader_dri3_buffer_front &&
> draw->have_fake_front))
> >            && buffer) {
> >
> > +         buffer->busy = true;
> >           /* Fill the new buffer with data from an old buffer */
> >           dri3_fence_await(draw->conn, draw, buffer);
> >           if (!loader_dri3_blit_image(draw,
>
> This seems a bit of a hack. Will this always be cleared again, in
> particular if it's the fake front buffer?
>
>
> > @@ -1731,6 +1732,7 @@ dri3_get_buffer(__DRIdrawable *driDrawable,
> >        draw->buffers[buf_id] = buffer;
> >     }
> >     dri3_fence_await(draw->conn, draw, buffer);
> > +   buffer = draw->buffers[buf_id];
>
> This should be something like
>
>    if (buffer_type == loader_dri3_buffer_back) {
>       buf_id = dri3_find_back(draw);
>
>       if (buf_id < 0)
>          return NULL;
>    }
>    buffer = draw->buffers[buf_id];
>
> to get the now-current back buffer.
>
>
> > @@ -1744,7 +1746,8 @@ dri3_get_buffer(__DRIdrawable *driDrawable,
> >     if (buffer_type == loader_dri3_buffer_back &&
> >         draw->cur_blit_source != -1 &&
> >         draw->buffers[draw->cur_blit_source] &&
> > -       buffer != draw->buffers[draw->cur_blit_source]) {
> > +       buffer != draw->buffers[draw->cur_blit_source] &&
> > +       buffer != NULL) {
> >
> >        struct loader_dri3_buffer *source = draw->buffers[draw->cur_blit_
> source];
> >
> >
>
> With the above, this NULL guard isn't needed.
>
>
> --
> Earthling Michel Dänzer               |               http://www.amd.com
> Libre software enthusiast             |             Mesa and X developer
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180427/9c03d1fe/attachment-0001.html>