Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754513AbdDQR6w (ORCPT ); Mon, 17 Apr 2017 13:58:52 -0400 Received: from mail-yw0-f174.google.com ([209.85.161.174]:33915 "EHLO mail-yw0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750883AbdDQR6t (ORCPT ); Mon, 17 Apr 2017 13:58:49 -0400 Date: Mon, 17 Apr 2017 13:58:46 -0400 From: Sean Paul To: Eric Anholt Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] drm/vc4: Fix refcounting of runtime PM get if it errors out. Message-ID: <20170417175846.quedtrtjmfmthv2g@art_vandelay> References: <20170417162603.12726-1-eric@anholt.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170417162603.12726-1-eric@anholt.net> User-Agent: NeoMutt/20170306-66-6ddb52-dirty (1.8.0) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1631 Lines: 52 On Mon, Apr 17, 2017 at 09:26:03AM -0700, Eric Anholt wrote: > We were returning without decrementing if the error happened, meaning > that at the next submit we wouldn't try to bring up the power domain. > > Signed-off-by: Eric Anholt This change looks good to me. It seems a little odd to wrap pm_runtime which is already refcounted in another refcount, but I'm really not familiar with the driver, and I'm sure there's a good reason for it. Reviewed-by: Sean Paul > --- > > I stumbled across this error when testing my CMA patch with a very low > (64MB) CMA area -- we would oops on the submit after the one that > failed. > > drivers/gpu/drm/vc4/vc4_gem.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c > index 777a8d9afd60..735412e3725a 100644 > --- a/drivers/gpu/drm/vc4/vc4_gem.c > +++ b/drivers/gpu/drm/vc4/vc4_gem.c > @@ -1016,13 +1016,16 @@ vc4_submit_cl_ioctl(struct drm_device *dev, void *data, > } > > mutex_lock(&vc4->power_lock); > - if (vc4->power_refcount++ == 0) > + if (vc4->power_refcount++ == 0) { > ret = pm_runtime_get_sync(&vc4->v3d->pdev->dev); > - mutex_unlock(&vc4->power_lock); > - if (ret < 0) { > - kfree(exec); > - return ret; > + if (ret < 0) { > + mutex_unlock(&vc4->power_lock); > + vc4->power_refcount--; > + kfree(exec); > + return ret; > + } > } > + mutex_unlock(&vc4->power_lock); > > exec->args = args; > INIT_LIST_HEAD(&exec->unref_list); > -- > 2.11.0 -- Sean Paul, Software Engineer, Google / Chromium OS