From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Engestrom Subject: Re: [PATCH v3] drm: move allocation out of drm_get_format_name() Date: Wed, 9 Nov 2016 11:42:17 +0000 Message-ID: <20161109114217.GO25290@imgtec.com> References: <20161108101558.ihvrprbbdqjwu5wg@phenom.ffwll.local> <20161109001805.10628-1-eric@engestrom.ch> <20161109010916.rpsqcswnlyxggovd@phenom.ffwll.local> <20161109011325.hvvfsvpq734nduxd@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20161109011325.hvvfsvpq734nduxd@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Eric Engestrom , linux-kernel@vger.kernel.org, David Airlie , dri-devel@lists.freedesktop.org, Wei Yongjun , Daniel Vetter , Flora Cui , Gustavo Padovan , Tom St Denis , Chunming Zhou , Thomas Hellstrom , Laurent Pinchart , Sinclair Yeh , Xinliang Liu , Xinwei Kong , VMware Graphics , Vitaly Prosyak , Alexandre Demers , Jani Nikula , intel-gfx@lists.freedesktop.org, Emily Deng , Colin Ian King List-Id: dri-devel@lists.freedesktop.org T24gV2VkbmVzZGF5LCAyMDE2LTExLTA5IDAyOjEzOjI1ICswMTAwLCBEYW5pZWwgVmV0dGVyIHdy b3RlOgo+IE9uIFdlZCwgTm92IDA5LCAyMDE2IGF0IDAyOjA5OjE2QU0gKzAxMDAsIERhbmllbCBW ZXR0ZXIgd3JvdGU6Cj4gPiBPbiBXZWQsIE5vdiAwOSwgMjAxNiBhdCAxMjoxNzo1MkFNICswMDAw LCBFcmljIEVuZ2VzdHJvbSB3cm90ZToKPiA+ID4gVGhlIGZ1bmN0aW9uJ3MgYmVoYXZpb3VyIHdh cyBjaGFuZ2VkIGluIDkwODQ0ZjAwMDQ5ZSwgd2l0aG91dCBjaGFuZ2luZwo+ID4gPiBpdHMgc2ln bmF0dXJlLCBjYXVzaW5nIHBlb3BsZSB0byBrZWVwIHVzaW5nIGl0IHRoZSBvbGQgd2F5IHdpdGhv dXQKPiA+ID4gcmVhbGlzaW5nIHRoZXkgd2VyZSBub3cgbGVha2luZyBtZW1vcnkuCj4gPiA+IFJv YiBDbGFyayBhbHNvIG5vdGljZWQgaXQgd2FzIGFsc28gYWxsb2NhdGluZyBHRlBfS0VSTkVMIG1l bW9yeSBpbgo+ID4gPiBhdG9taWMgY29udGV4dHMsIGJyZWFraW5nIHRoZW0uCj4gPiA+IAo+ID4g PiBJbnN0ZWFkIG9mIGhhdmluZyB0byBhbGxvY2F0ZSBHRlBfQVRPTUlDIG1lbW9yeSBhbmQgZml4 aW5nIHRoZSBjYWxsZXJzCj4gPiA+IHRvIG1ha2UgdGhlbSBjbGVhbnVwIHRoZSBtZW1vcnkgYWZ0 ZXJ3YXJkcywgbGV0J3MgY2hhbmdlIHRoZSBmdW5jdGlvbidzCj4gPiA+IHNpZ25hdHVyZSBieSBo YXZpbmcgdGhlIGNhbGxlciB0YWtlIGNhcmUgb2YgdGhlIG1lbW9yeSBhbmQgcGFzc2luZyBpdCB0 bwo+ID4gPiB0aGUgZnVuY3Rpb24uCj4gPiA+IFRoZSBuZXcgcGFyYW1ldGVyIGlzIGEgc2luZ2xl LWZpZWxkIHN0cnVjdCBpbiBvcmRlciB0byBlbmZvcmNlIHRoZSBzaXplCj4gPiA+IG9mIGl0cyBi dWZmZXIgYW5kIGhlbHAgY2FsbGVycyB0byBjb3JyZWN0bHkgbWFuYWdlIHRoZWlyIG1lbW9yeS4K PiA+ID4gCj4gPiA+IEZpeGVzOiA5MDg0NGYwMDA0OWUgKCJkcm06IG1ha2UgZHJtX2dldF9mb3Jt YXRfbmFtZSB0aHJlYWQtc2FmZSIpCj4gPiA+IENjOiBSb2IgQ2xhcmsgPHJvYmRjbGFya0BnbWFp bC5jb20+Cj4gPiA+IENjOiBDaHJpc3RpYW4gS8O2bmlnIDxjaHJpc3RpYW4ua29lbmlnQGFtZC5j b20+Cj4gPiA+IEFja2VkLWJ5OiBDaHJpc3RpYW4gS8O2bmlnIDxjaHJpc3RpYW4ua29lbmlnQGFt ZC5jb20+Cj4gPiA+IEFja2VkLWJ5OiBSb2IgQ2xhcmsgPHJvYmRjbGFya0BnbWFpbC5jb20+Cj4g PiA+IEFja2VkLWJ5OiBTaW5jbGFpciBZZWggPHN5ZWhAdm13YXJlLmNvbT4gKHZtd2dmeCkKPiA+ ID4gUmV2aWV3ZWQtYnk6IEphbmkgTmlrdWxhIDxqYW5pLm5pa3VsYUBpbnRlbC5jb20+Cj4gPiA+ IFN1Z2dlc3RlZC1ieTogVmlsbGUgU3lyasOkbMOkIDx2aWxsZS5zeXJqYWxhQGxpbnV4LmludGVs LmNvbT4KPiA+ID4gU2lnbmVkLW9mZi1ieTogRXJpYyBFbmdlc3Ryb20gPGVyaWNAZW5nZXN0cm9t LmNoPgo+ID4gPiAtLS0KPiA+ID4gdjMgLSBmaXggIkZpeGVzIiB0YWcsIHJlcGxhY2UgaXQgd2l0 aCBhbiBhY3R1YWwgY29tbWl0IG1lc3NhZ2UKPiA+ID4gICAgLSBjb2xsZWN0IGFjayAmIHItYgo+ ID4gPiAKPiA+ID4gdjIgLSB1c2Ugc2luZ2xlLWZpZWxkIHN0cnVjdCBpbnN0ZWFkIG9mIHR5cGVk ZWYgdG8gbGV0IHRoZSBjb21waWxlcgo+ID4gPiAgICAgIGVuZm9yY2UgdGhlIHR5cGUgKENocmlz dGlhbiBLw7ZuaWcpCj4gPiAKPiA+IEFwcGxpZWQgdG8gZHJtLW1pc2MsIHRoYW5rcy4KPiAKPiBX ZWxsLCBoYWQgdG8gZHJvcCBpdCBhZ2FpbiBzaW5jZSBpdCBkaWRuJ3QgY29tcGlsZToKPiAKPiAK PiAgIENDIFtNXSAgZHJpdmVycy9ncHUvZHJtL2RybV9ibGVuZC5vCj4gZHJpdmVycy9ncHUvZHJt L2RybV9hdG9taWMuYzogSW4gZnVuY3Rpb24g4oCYZHJtX2F0b21pY19wbGFuZV9wcmludF9zdGF0 ZeKAmToKPiBkcml2ZXJzL2dwdS9kcm0vZHJtX2F0b21pYy5jOjkyMDo1OiBlcnJvcjogdG9vIGZl dyBhcmd1bWVudHMgdG8gZnVuY3Rpb24g4oCYZHJtX2dldF9mb3JtYXRfbmFtZeKAmQo+ICAgICAg ZHJtX2dldF9mb3JtYXRfbmFtZShmYi0+cGl4ZWxfZm9ybWF0KSk7Cj4gICAgICBefn5+fn5+fn5+ fn5+fn5+fn5+Cj4gSW4gZmlsZSBpbmNsdWRlZCBmcm9tIC4vaW5jbHVkZS9kcm0vZHJtUC5oOjcx OjAsCj4gICAgICAgICAgICAgICAgICBmcm9tIGRyaXZlcnMvZ3B1L2RybS9kcm1fYXRvbWljLmM6 Mjk6Cj4gLi9pbmNsdWRlL2RybS9kcm1fZm91cmNjLmg6NjU6Nzogbm90ZTogZGVjbGFyZWQgaGVy ZQo+ICBjaGFyICpkcm1fZ2V0X2Zvcm1hdF9uYW1lKHVpbnQzMl90IGZvcm1hdCwgc3RydWN0IGRy bV9mb3JtYXRfbmFtZV9idWYgKmJ1Zik7Cj4gICAgICAgIF5+fn5+fn5+fn5+fn5+fn5+fn4KPiAK PiBDYW4geW91IHBscyByZWJhc2Ugb250byBkcm0tbWlzYyBvciBsaW51eC1uZXh0IG9yIHNvbWV0 aGluZz8KClRoYXQgd2FzIGJhc2VkIG9uIGFpcmxpZWQvZHJtLW5leHQgKGxhc3QgZmV0Y2hlZCBv biBTdW5kYXkgSSB0aGluayksCkkgY2FuIHJlYmFzZSBpdCBvbiBkcm0tbWlzYyBpZiBpdCBoZWxw cywgYnV0IGl0IHNlZW1zIG9sZGVyIHRoYW4KZHJtLW5leHQuClNob3VsZCBJIGp1c3QgcmViYXNl IG9uIHRvcCBvZiBjdXJyZW50IGhlYWQgb2YgZHJtLW5leHQ/Cl9fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fCkludGVsLWdmeCBtYWlsaW5nIGxpc3QKSW50ZWwt Z2Z4QGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9t YWlsbWFuL2xpc3RpbmZvL2ludGVsLWdmeAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753620AbcKILn1 (ORCPT ); Wed, 9 Nov 2016 06:43:27 -0500 Received: from mailapp01.imgtec.com ([195.59.15.196]:49847 "EHLO mailapp01.imgtec.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932393AbcKILnU (ORCPT ); Wed, 9 Nov 2016 06:43:20 -0500 Date: Wed, 9 Nov 2016 11:42:17 +0000 From: Eric Engestrom To: Eric Engestrom , , David Airlie , , Wei Yongjun , Daniel Vetter , Flora Cui , Gustavo Padovan , Tom St Denis , Chunming Zhou , Thomas Hellstrom , Laurent Pinchart , Sinclair Yeh , Xinliang Liu , Xinwei Kong , VMware Graphics , Vitaly Prosyak , Alexandre Demers , Jani Nikula , , Emily Deng , Colin Ian King , Junwei Zhang , Michel =?utf-8?Q?D=C3=A4nzer?= , Alex Deucher , Christian =?utf-8?B?S8O2bmln?= Subject: Re: [Intel-gfx] [PATCH v3] drm: move allocation out of drm_get_format_name() Message-ID: <20161109114217.GO25290@imgtec.com> References: <20161108101558.ihvrprbbdqjwu5wg@phenom.ffwll.local> <20161109001805.10628-1-eric@engestrom.ch> <20161109010916.rpsqcswnlyxggovd@phenom.ffwll.local> <20161109011325.hvvfsvpq734nduxd@phenom.ffwll.local> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20161109011325.hvvfsvpq734nduxd@phenom.ffwll.local> User-Agent: Mutt/1.7.1 (2016-10-04) X-Originating-IP: [10.60.4.28] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wednesday, 2016-11-09 02:13:25 +0100, Daniel Vetter wrote: > On Wed, Nov 09, 2016 at 02:09:16AM +0100, Daniel Vetter wrote: > > On Wed, Nov 09, 2016 at 12:17:52AM +0000, Eric Engestrom wrote: > > > The function's behaviour was changed in 90844f00049e, without changing > > > its signature, causing people to keep using it the old way without > > > realising they were now leaking memory. > > > Rob Clark also noticed it was also allocating GFP_KERNEL memory in > > > atomic contexts, breaking them. > > > > > > Instead of having to allocate GFP_ATOMIC memory and fixing the callers > > > to make them cleanup the memory afterwards, let's change the function's > > > signature by having the caller take care of the memory and passing it to > > > the function. > > > The new parameter is a single-field struct in order to enforce the size > > > of its buffer and help callers to correctly manage their memory. > > > > > > Fixes: 90844f00049e ("drm: make drm_get_format_name thread-safe") > > > Cc: Rob Clark > > > Cc: Christian König > > > Acked-by: Christian König > > > Acked-by: Rob Clark > > > Acked-by: Sinclair Yeh (vmwgfx) > > > Reviewed-by: Jani Nikula > > > Suggested-by: Ville Syrjälä > > > Signed-off-by: Eric Engestrom > > > --- > > > v3 - fix "Fixes" tag, replace it with an actual commit message > > > - collect ack & r-b > > > > > > v2 - use single-field struct instead of typedef to let the compiler > > > enforce the type (Christian König) > > > > Applied to drm-misc, thanks. > > Well, had to drop it again since it didn't compile: > > > CC [M] drivers/gpu/drm/drm_blend.o > drivers/gpu/drm/drm_atomic.c: In function ‘drm_atomic_plane_print_state’: > drivers/gpu/drm/drm_atomic.c:920:5: error: too few arguments to function ‘drm_get_format_name’ > drm_get_format_name(fb->pixel_format)); > ^~~~~~~~~~~~~~~~~~~ > In file included from ./include/drm/drmP.h:71:0, > from drivers/gpu/drm/drm_atomic.c:29: > ./include/drm/drm_fourcc.h:65:7: note: declared here > char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf); > ^~~~~~~~~~~~~~~~~~~ > > Can you pls rebase onto drm-misc or linux-next or something? That was based on airlied/drm-next (last fetched on Sunday I think), I can rebase it on drm-misc if it helps, but it seems older than drm-next. Should I just rebase on top of current head of drm-next?