| Message ID | 1444308468-8910-1-git-send-email-sudipm.mukherjee@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Oct 08, 2015 at 06:17:48PM +0530, Sudip Mukherjee wrote: > We are allocating backing using psbfb_alloc() and so > backing->stolen is always true. So we were freeing backing two times. > Moreover if we follow the execution path then we should be freeing > backing after we have released the helper. So remove the one which frees > backing before the helper is released. > While at it the error labels are also renamed to give a meaningful > name. > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org> > Reviewed-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> > --- This patch was never picked up. It will not apply now. Daniel, please let me know if you want me to resend after making necessary changes. regards sudip > drivers/gpu/drm/gma500/framebuffer.c | 13 ++++--------- > 1 file changed, 4 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c > index 2eaf1b3..52e2bf3 100644 > --- a/drivers/gpu/drm/gma500/framebuffer.c > +++ b/drivers/gpu/drm/gma500/framebuffer.c > @@ -411,7 +411,7 @@ static int psbfb_create(struct psb_fbdev *fbdev, > info = drm_fb_helper_alloc_fbi(&fbdev->psb_fb_helper); > if (IS_ERR(info)) { > ret = PTR_ERR(info); > - goto out_err1; > + goto err_unlock; > } > info->par = fbdev; > > @@ -419,7 +419,7 @@ static int psbfb_create(struct psb_fbdev *fbdev, > > ret = psb_framebuffer_init(dev, psbfb, &mode_cmd, backing); > if (ret) > - goto out_unref; > + goto err_release; > > fb = &psbfb->base; > psbfb->fbdev = info; > @@ -465,14 +465,9 @@ static int psbfb_create(struct psb_fbdev *fbdev, > > mutex_unlock(&dev->struct_mutex); > return 0; > -out_unref: > - if (backing->stolen) > - psb_gtt_free_range(dev, backing); > - else > - drm_gem_object_unreference(&backing->gem); > - > +err_release: > drm_fb_helper_release_fbi(&fbdev->psb_fb_helper); > -out_err1: > +err_unlock: > mutex_unlock(&dev->struct_mutex); > psb_gtt_free_range(dev, backing); > return ret; > -- > 1.9.1 >
On Wed, Dec 9, 2015 at 12:53 PM, Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote: > On Thu, Oct 08, 2015 at 06:17:48PM +0530, Sudip Mukherjee wrote: >> We are allocating backing using psbfb_alloc() and so >> backing->stolen is always true. So we were freeing backing two times. >> Moreover if we follow the execution path then we should be freeing >> backing after we have released the helper. So remove the one which frees >> backing before the helper is released. >> While at it the error labels are also renamed to give a meaningful >> name. >> >> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org> >> Reviewed-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> >> --- > > This patch was never picked up. It will not apply now. > > Daniel, please let me know if you want me to resend after making > necessary changes. I will pick this up and pass it along to Dave. Sorry for the delay. -Patrik > > regards > sudip > >> drivers/gpu/drm/gma500/framebuffer.c | 13 ++++--------- >> 1 file changed, 4 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c >> index 2eaf1b3..52e2bf3 100644 >> --- a/drivers/gpu/drm/gma500/framebuffer.c >> +++ b/drivers/gpu/drm/gma500/framebuffer.c >> @@ -411,7 +411,7 @@ static int psbfb_create(struct psb_fbdev *fbdev, >> info = drm_fb_helper_alloc_fbi(&fbdev->psb_fb_helper); >> if (IS_ERR(info)) { >> ret = PTR_ERR(info); >> - goto out_err1; >> + goto err_unlock; >> } >> info->par = fbdev; >> >> @@ -419,7 +419,7 @@ static int psbfb_create(struct psb_fbdev *fbdev, >> >> ret = psb_framebuffer_init(dev, psbfb, &mode_cmd, backing); >> if (ret) >> - goto out_unref; >> + goto err_release; >> >> fb = &psbfb->base; >> psbfb->fbdev = info; >> @@ -465,14 +465,9 @@ static int psbfb_create(struct psb_fbdev *fbdev, >> >> mutex_unlock(&dev->struct_mutex); >> return 0; >> -out_unref: >> - if (backing->stolen) >> - psb_gtt_free_range(dev, backing); >> - else >> - drm_gem_object_unreference(&backing->gem); >> - >> +err_release: >> drm_fb_helper_release_fbi(&fbdev->psb_fb_helper); >> -out_err1: >> +err_unlock: >> mutex_unlock(&dev->struct_mutex); >> psb_gtt_free_range(dev, backing); >> return ret; >> -- >> 1.9.1 >>
On Wednesday 09 December 2015 05:50 PM, Patrik Jakobsson wrote: > On Wed, Dec 9, 2015 at 12:53 PM, Sudip Mukherjee > <sudipm.mukherjee@gmail.com> wrote: >> On Thu, Oct 08, 2015 at 06:17:48PM +0530, Sudip Mukherjee wrote: >>> We are allocating backing using psbfb_alloc() and so >>> backing->stolen is always true. So we were freeing backing two times. >>> Moreover if we follow the execution path then we should be freeing >>> backing after we have released the helper. So remove the one which frees >>> backing before the helper is released. >>> While at it the error labels are also renamed to give a meaningful >>> name. >>> >>> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org> >>> Reviewed-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> >>> --- >> >> This patch was never picked up. It will not apply now. >> >> Daniel, please let me know if you want me to resend after making >> necessary changes. > > I will pick this up and pass it along to Dave. Sorry for the delay. This was not picked up. But I guess it is still true. Do you want me to rebase and send it again.. regards sudip > > -Patrik > >> >> regards >> sudip >> >>> drivers/gpu/drm/gma500/framebuffer.c | 13 ++++--------- >>> 1 file changed, 4 insertions(+), 9 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c >>> index 2eaf1b3..52e2bf3 100644 >>> --- a/drivers/gpu/drm/gma500/framebuffer.c >>> +++ b/drivers/gpu/drm/gma500/framebuffer.c >>> @@ -411,7 +411,7 @@ static int psbfb_create(struct psb_fbdev *fbdev, >>> info = drm_fb_helper_alloc_fbi(&fbdev->psb_fb_helper); >>> if (IS_ERR(info)) { >>> ret = PTR_ERR(info); >>> - goto out_err1; >>> + goto err_unlock; >>> } >>> info->par = fbdev; >>> >>> @@ -419,7 +419,7 @@ static int psbfb_create(struct psb_fbdev *fbdev, >>> >>> ret = psb_framebuffer_init(dev, psbfb, &mode_cmd, backing); >>> if (ret) >>> - goto out_unref; >>> + goto err_release; >>> >>> fb = &psbfb->base; >>> psbfb->fbdev = info; >>> @@ -465,14 +465,9 @@ static int psbfb_create(struct psb_fbdev *fbdev, >>> >>> mutex_unlock(&dev->struct_mutex); >>> return 0; >>> -out_unref: >>> - if (backing->stolen) >>> - psb_gtt_free_range(dev, backing); >>> - else >>> - drm_gem_object_unreference(&backing->gem); >>> - >>> +err_release: >>> drm_fb_helper_release_fbi(&fbdev->psb_fb_helper); >>> -out_err1: >>> +err_unlock: >>> mutex_unlock(&dev->struct_mutex); >>> psb_gtt_free_range(dev, backing); >>> return ret; >>> -- >>> 1.9.1 >>>
On Thu, Apr 7, 2016 at 5:52 PM, Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote: > On Wednesday 09 December 2015 05:50 PM, Patrik Jakobsson wrote: >> >> On Wed, Dec 9, 2015 at 12:53 PM, Sudip Mukherjee >> <sudipm.mukherjee@gmail.com> wrote: >>> >>> On Thu, Oct 08, 2015 at 06:17:48PM +0530, Sudip Mukherjee wrote: >>>> >>>> We are allocating backing using psbfb_alloc() and so >>>> backing->stolen is always true. So we were freeing backing two times. >>>> Moreover if we follow the execution path then we should be freeing >>>> backing after we have released the helper. So remove the one which frees >>>> backing before the helper is released. >>>> While at it the error labels are also renamed to give a meaningful >>>> name. >>>> >>>> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org> >>>> Reviewed-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> >>>> --- >>> >>> >>> This patch was never picked up. It will not apply now. >>> >>> Daniel, please let me know if you want me to resend after making >>> necessary changes. >> >> >> I will pick this up and pass it along to Dave. Sorry for the delay. > > > This was not picked up. But I guess it is still true. Do you want me to > rebase and send it again.. I already have it rebased, will send it out later tonight. For real this time ;) Thanks Patrik > > regards > sudip > > >> >> -Patrik >> >>> >>> regards >>> sudip >>> >>>> drivers/gpu/drm/gma500/framebuffer.c | 13 ++++--------- >>>> 1 file changed, 4 insertions(+), 9 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/gma500/framebuffer.c >>>> b/drivers/gpu/drm/gma500/framebuffer.c >>>> index 2eaf1b3..52e2bf3 100644 >>>> --- a/drivers/gpu/drm/gma500/framebuffer.c >>>> +++ b/drivers/gpu/drm/gma500/framebuffer.c >>>> @@ -411,7 +411,7 @@ static int psbfb_create(struct psb_fbdev *fbdev, >>>> info = drm_fb_helper_alloc_fbi(&fbdev->psb_fb_helper); >>>> if (IS_ERR(info)) { >>>> ret = PTR_ERR(info); >>>> - goto out_err1; >>>> + goto err_unlock; >>>> } >>>> info->par = fbdev; >>>> >>>> @@ -419,7 +419,7 @@ static int psbfb_create(struct psb_fbdev *fbdev, >>>> >>>> ret = psb_framebuffer_init(dev, psbfb, &mode_cmd, backing); >>>> if (ret) >>>> - goto out_unref; >>>> + goto err_release; >>>> >>>> fb = &psbfb->base; >>>> psbfb->fbdev = info; >>>> @@ -465,14 +465,9 @@ static int psbfb_create(struct psb_fbdev *fbdev, >>>> >>>> mutex_unlock(&dev->struct_mutex); >>>> return 0; >>>> -out_unref: >>>> - if (backing->stolen) >>>> - psb_gtt_free_range(dev, backing); >>>> - else >>>> - drm_gem_object_unreference(&backing->gem); >>>> - >>>> +err_release: >>>> drm_fb_helper_release_fbi(&fbdev->psb_fb_helper); >>>> -out_err1: >>>> +err_unlock: >>>> mutex_unlock(&dev->struct_mutex); >>>> psb_gtt_free_range(dev, backing); >>>> return ret; >>>> -- >>>> 1.9.1 >>>> >
diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c index 2eaf1b3..52e2bf3 100644 --- a/drivers/gpu/drm/gma500/framebuffer.c +++ b/drivers/gpu/drm/gma500/framebuffer.c @@ -411,7 +411,7 @@ static int psbfb_create(struct psb_fbdev *fbdev, info = drm_fb_helper_alloc_fbi(&fbdev->psb_fb_helper); if (IS_ERR(info)) { ret = PTR_ERR(info); - goto out_err1; + goto err_unlock; } info->par = fbdev; @@ -419,7 +419,7 @@ static int psbfb_create(struct psb_fbdev *fbdev, ret = psb_framebuffer_init(dev, psbfb, &mode_cmd, backing); if (ret) - goto out_unref; + goto err_release; fb = &psbfb->base; psbfb->fbdev = info; @@ -465,14 +465,9 @@ static int psbfb_create(struct psb_fbdev *fbdev, mutex_unlock(&dev->struct_mutex); return 0; -out_unref: - if (backing->stolen) - psb_gtt_free_range(dev, backing); - else - drm_gem_object_unreference(&backing->gem); - +err_release: drm_fb_helper_release_fbi(&fbdev->psb_fb_helper); -out_err1: +err_unlock: mutex_unlock(&dev->struct_mutex); psb_gtt_free_range(dev, backing); return ret;