All of lore.kernel.org
 help / color / mirror / Atom feed
From: dinghao.liu@zju.edu.cn
To: "Dan Carpenter" <dan.carpenter@oracle.com>
Cc: "Markus Elfring" <Markus.Elfring@web.de>,
	dri-devel@lists.freedesktop.org, nouveau@lists.freedesktop.org,
	kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Ben Skeggs" <bskeggs@redhat.com>,
	"David Airlie" <airlied@linux.ie>, "Kangjie Lu" <kjlu@umn.edu>
Subject: Re: Re: [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new()
Date: Wed, 3 Jun 2020 10:21:47 +0800 (GMT+08:00)	[thread overview]
Message-ID: <5d580094.f274c.17277fc124e.Coremail.dinghao.liu@zju.edu.cn> (raw)
In-Reply-To: <20200602153900.GW22511@kadam>


> On Tue, Jun 02, 2020 at 01:10:34PM +0200, Markus Elfring wrote:
> > > The original patch was basically fine.
> > 
> > I propose to reconsider the interpretation of the software situation once more.
> > 
> > * Should the allocated clock object be kept usable even after
> >   a successful return from this function?
> 
> Heh.  You're right.  The patch is freeing "clk" on the success path so
> that doesn't work.
> 

Ben has explained this problem:
https://lore.kernel.org/patchwork/patch/1249592/
Since the caller will check "pclk" on failure, we don't need to free
"clk" in gm20b_clk_new() and I think this patch is no longer needed.

Regards,
Dinghao

WARNING: multiple messages have this Message-ID (diff)
From: dinghao.liu@zju.edu.cn
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: David Airlie <airlied@linux.ie>,
	nouveau@lists.freedesktop.org, kernel-janitors@vger.kernel.org,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	Markus Elfring <Markus.Elfring@web.de>,
	Ben Skeggs <bskeggs@redhat.com>, Kangjie Lu <kjlu@umn.edu>
Subject: Re: Re: [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new()
Date: Wed, 03 Jun 2020 02:21:47 +0000	[thread overview]
Message-ID: <5d580094.f274c.17277fc124e.Coremail.dinghao.liu@zju.edu.cn> (raw)
In-Reply-To: <20200602153900.GW22511@kadam>

Cj4gT24gVHVlLCBKdW4gMDIsIDIwMjAgYXQgMDE6MTA6MzRQTSArMDIwMCwgTWFya3VzIEVsZnJp
bmcgd3JvdGU6Cj4gPiA+IFRoZSBvcmlnaW5hbCBwYXRjaCB3YXMgYmFzaWNhbGx5IGZpbmUuCj4g
PiAKPiA+IEkgcHJvcG9zZSB0byByZWNvbnNpZGVyIHRoZSBpbnRlcnByZXRhdGlvbiBvZiB0aGUg
c29mdHdhcmUgc2l0dWF0aW9uIG9uY2UgbW9yZS4KPiA+IAo+ID4gKiBTaG91bGQgdGhlIGFsbG9j
YXRlZCBjbG9jayBvYmplY3QgYmUga2VwdCB1c2FibGUgZXZlbiBhZnRlcgo+ID4gICBhIHN1Y2Nl
c3NmdWwgcmV0dXJuIGZyb20gdGhpcyBmdW5jdGlvbj8KPiAKPiBIZWguICBZb3UncmUgcmlnaHQu
ICBUaGUgcGF0Y2ggaXMgZnJlZWluZyAiY2xrIiBvbiB0aGUgc3VjY2VzcyBwYXRoIHNvCj4gdGhh
dCBkb2Vzbid0IHdvcmsuCj4gCgpCZW4gaGFzIGV4cGxhaW5lZCB0aGlzIHByb2JsZW06Cmh0dHBz
Oi8vbG9yZS5rZXJuZWwub3JnL3BhdGNod29yay9wYXRjaC8xMjQ5NTkyLwpTaW5jZSB0aGUgY2Fs
bGVyIHdpbGwgY2hlY2sgInBjbGsiIG9uIGZhaWx1cmUsIHdlIGRvbid0IG5lZWQgdG8gZnJlZQoi
Y2xrIiBpbiBnbTIwYl9jbGtfbmV3KCkgYW5kIEkgdGhpbmsgdGhpcyBwYXRjaCBpcyBubyBsb25n
ZXIgbmVlZGVkLgoKUmVnYXJkcywKRGluZ2hhbw=

WARNING: multiple messages have this Message-ID (diff)
From: dinghao.liu@zju.edu.cn
To: "Dan Carpenter" <dan.carpenter@oracle.com>
Cc: David Airlie <airlied@linux.ie>,
	nouveau@lists.freedesktop.org, kernel-janitors@vger.kernel.org,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	Markus Elfring <Markus.Elfring@web.de>,
	Ben Skeggs <bskeggs@redhat.com>, Kangjie Lu <kjlu@umn.edu>
Subject: Re: Re: [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new()
Date: Wed, 3 Jun 2020 10:21:47 +0800 (GMT+08:00)	[thread overview]
Message-ID: <5d580094.f274c.17277fc124e.Coremail.dinghao.liu@zju.edu.cn> (raw)
In-Reply-To: <20200602153900.GW22511@kadam>


> On Tue, Jun 02, 2020 at 01:10:34PM +0200, Markus Elfring wrote:
> > > The original patch was basically fine.
> > 
> > I propose to reconsider the interpretation of the software situation once more.
> > 
> > * Should the allocated clock object be kept usable even after
> >   a successful return from this function?
> 
> Heh.  You're right.  The patch is freeing "clk" on the success path so
> that doesn't work.
> 

Ben has explained this problem:
https://lore.kernel.org/patchwork/patch/1249592/
Since the caller will check "pclk" on failure, we don't need to free
"clk" in gm20b_clk_new() and I think this patch is no longer needed.

Regards,
Dinghao
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2020-06-03  2:22 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-31  8:03 [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new() Markus Elfring
2020-05-31  8:03 ` Markus Elfring
2020-05-31  8:03 ` Markus Elfring
2020-05-31  8:23 ` dinghao.liu
2020-05-31  8:23   ` dinghao.liu
2020-05-31  8:23   ` dinghao.liu
2020-05-31  8:38   ` Markus Elfring
2020-05-31  8:38     ` Markus Elfring
2020-05-31  8:38     ` Markus Elfring
2020-05-31  8:38     ` Markus Elfring
2020-05-31  8:52     ` dinghao.liu
2020-05-31  8:52       ` dinghao.liu
2020-05-31  8:52       ` dinghao.liu
2020-05-31  9:00       ` Markus Elfring
2020-05-31  9:00         ` Markus Elfring
2020-05-31  9:00         ` Markus Elfring
2020-05-31  9:15         ` dinghao.liu
2020-05-31  9:15           ` dinghao.liu
2020-05-31  9:15           ` dinghao.liu
2020-05-31  9:51           ` Markus Elfring
2020-05-31  9:51             ` Markus Elfring
2020-05-31  9:51             ` Markus Elfring
2020-05-31  9:51             ` Markus Elfring
2020-05-31 10:42             ` dinghao.liu
2020-05-31 10:42               ` dinghao.liu
2020-05-31 10:42               ` dinghao.liu
2020-05-31 12:14               ` drm/nouveau/clk/gm20b: Fix memory leaks after failed gk20a_clk_ctor() calls Markus Elfring
2020-05-31 12:14                 ` Markus Elfring
2020-05-31 12:14                 ` Markus Elfring
2020-05-31 12:14                 ` Markus Elfring
2020-06-02 10:29   ` Re: [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new() Dan Carpenter
2020-06-02 10:29     ` Dan Carpenter
2020-06-02 10:29     ` Dan Carpenter
2020-06-02 10:29     ` Dan Carpenter
2020-06-02 11:10     ` Markus Elfring
2020-06-02 11:10       ` Markus Elfring
2020-06-02 11:10       ` Markus Elfring
2020-06-02 11:10       ` Markus Elfring
2020-06-02 15:39       ` Dan Carpenter
2020-06-02 15:39         ` Dan Carpenter
2020-06-02 15:39         ` Dan Carpenter
2020-06-03  2:21         ` dinghao.liu [this message]
2020-06-03  2:21           ` dinghao.liu
2020-06-03  2:21           ` dinghao.liu
2020-06-03  5:04           ` drm/nouveau/clk/gm20b: Understanding challenges around gm20b_clk_new() Markus Elfring
2020-06-03  5:04             ` Markus Elfring
2020-06-03  5:04             ` Markus Elfring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5d580094.f274c.17277fc124e.Coremail.dinghao.liu@zju.edu.cn \
    --to=dinghao.liu@zju.edu.cn \
    --cc=Markus.Elfring@web.de \
    --cc=airlied@linux.ie \
    --cc=bskeggs@redhat.com \
    --cc=dan.carpenter@oracle.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=kjlu@umn.edu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nouveau@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.