On Thu 27 Apr 2017, Emil Velikov wrote:
> On 27 April 2017 at 12:14, Xu, Randy <randy...@intel.com> wrote:
> > Hi, Chad
> >
> > Please review this patch, we need it to solve some instability issues
Randy and Tapani, could you provide a few dEQP test names that this
patch fixes? I'd like to mention at least one EGL and one Vulkan test in
the commit message.
> The patch is correct, although the commit message can be improved upon.
> Read through the following example and consider the alternative
> solution mentioned within.
Yes, this patch is correct. It makes brw_dri_create_fence_fd() behave
like all the other drivers' create_fence_fd funcs, which call dup().
Since this is an easy one-liner that can backport to stable, let's take
it.
However, I believe the fully correct solution is Emil's plan B:
__DRI2fenceExtensionRec::create_fence_fd should transfer fd ownership to
the driver, and therefore no dup is needed. But that's a slightly more
invasive change that's not as easily backported to stable.
Reviewed-by: Chad Versace <chadvers...@chromium.org>
Cc: mesa-sta...@lists.freedesktop.org
Emil, how about one of us appends your extended commit message to
Randy's, and then pushes?
> Then either polish and resend, or send patch that implements plan B.
> If you opt for B you want to drop the dup/close from the existing
> users - freedreno and etnaviv.
>
> "
> The semantics of __DRI2fenceExtensionRec::create_fence_fd are unclear
> if the DRI driver takes ownership of the fd or not.
> Since the i965 driver supports both "in" and "out" fd it assumes "yes,
> driver takes ownership", which results in a double close.
> First time in our destroy_fence() callback and then in the loader.
>
> Other DRI modules rely on the loader issuing close().
>
> Thus we have two solutions:
> - dup() the file descriptor
> - close() only if we have an out fence.
>
> This patch implements the former, simpler solution.
>
> Fixes: 6403e376511 ("i965/sync: Implement fences based on Linux sync_file")
> Reviewed-by: Emil Velikov <emil.veli...@collabora.com>
> "
>
> In either case you want to augment create_fence_fd and destroy_fence
> (in dri_interface.h) to explicitly define the behaviour.
> Please keep that a separate patch part of this series.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
