Actually the reason why I need this CL: In multi plane patch I'd like to only mmap once for different planes of same buffer. So actually I need some way to reuse same mmap for different planes. Then it's natural to have this CL. The fix to leak is a side effect of this CL. dt_unmap still works with this CL, if user call dt_map for single plane buffer multiple times, they will get same pointer, and if they call dt_unmap , with this CL, it still get unmapped.
On Wed, Mar 7, 2018 at 7:14 AM, Emil Velikov <emil.l.veli...@gmail.com> wrote: > On 6 March 2018 at 22:43, Lepton Wu <lep...@chromium.org> wrote: >> If user calls map twice for kms_sw_displaytarget, the first mapped >> buffer could get leaked. Instead of calling mmap every time, just >> reuse previous mapping. Since user could map same displaytarget with >> different flags, we have to keep two different pointers, one for rw >> mapping and one for ro mapping. >> >> Change-Id: I65308f0ff2640bd57b2577c6a3469540c9722859 >> Signed-off-by: Lepton Wu <lep...@chromium.org> >> --- >> .../winsys/sw/kms-dri/kms_dri_sw_winsys.c | 26 ++++++++++++++----- >> 1 file changed, 19 insertions(+), 7 deletions(-) >> >> diff --git a/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c >> b/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c >> index 22e1c936ac5..30343222470 100644 >> --- a/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c >> +++ b/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c >> @@ -70,6 +70,7 @@ struct kms_sw_displaytarget >> >> uint32_t handle; >> void *mapped; >> + void *ro_mapped; >> >> int ref_count; >> struct list_head link; >> @@ -170,6 +171,11 @@ kms_sw_displaytarget_destroy(struct sw_winsys *ws, >> if (kms_sw_dt->ref_count > 0) >> return; >> >> + if (kms_sw_dt->ro_mapped) >> + munmap(kms_sw_dt->ro_mapped, kms_sw_dt->size); >> + if (kms_sw_dt->mapped) >> + munmap(kms_sw_dt->mapped, kms_sw_dt->size); >> + > I'm not 100% sure about this. There's a reason why dt_unmap exists. > > Skimming through the existing code [1] - there's a handful of cases > that indicate the leaks you're hitting. > I'd look into addressing those properly because as-is this looks alike > a duck-taping the problem. > > With the hunk gone the patch is > Reviewed-by: Emil Velikov <emil.veli...@collabora.com> > > -Emil > > [1] $ git grep -20 -w displaytarget_[a-z]*map _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
