From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gustavo Padovan Subject: Re: [PATCH 06/10] staging/android: turn fence_info into a __64 pointer Date: Mon, 1 Feb 2016 16:00:08 -0200 Message-ID: <20160201180008.GB3207@joana> References: <1454102426-20637-1-git-send-email-gustavo@padovan.org> <1454102426-20637-7-git-send-email-gustavo@padovan.org> <56AF1A43.5010900@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-yk0-f193.google.com (mail-yk0-f193.google.com [209.85.160.193]) by gabe.freedesktop.org (Postfix) with ESMTPS id 77BEE6E4C4 for ; Mon, 1 Feb 2016 10:00:13 -0800 (PST) Received: by mail-yk0-f193.google.com with SMTP id z13so3050417ykd.3 for ; Mon, 01 Feb 2016 10:00:13 -0800 (PST) Content-Disposition: inline In-Reply-To: <56AF1A43.5010900@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Maarten Lankhorst Cc: devel@driverdev.osuosl.org, Daniel Stone , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Arve =?iso-8859-1?B?SGr4bm5lduVn?= , Daniel Vetter , Riley Andrews , Gustavo Padovan , John Harrison List-Id: dri-devel@lists.freedesktop.org SGkgTWFhcnRlbiwKCjIwMTYtMDItMDEgTWFhcnRlbiBMYW5raG9yc3QgPG1hYXJ0ZW4ubGFua2hv cnN0QGxpbnV4LmludGVsLmNvbT46Cgo+IE9wIDI5LTAxLTE2IG9tIDIyOjIwIHNjaHJlZWYgR3Vz dGF2byBQYWRvdmFuOgo+ID4gRnJvbTogR3VzdGF2byBQYWRvdmFuIDxndXN0YXZvLnBhZG92YW5A Y29sbGFib3JhLmNvLnVrPgo+ID4KPiA+IE1ha2luZyBmZW5jZV9pbmZvIGEgcG9pbnRlciBlbmFi bGVzIHVzIHRvIGV4dGVuZCB0aGUgc3RydWN0IGluIHRoZSBmdXR1cmUKPiA+IHdpdGhvdXQgYnJl YWtpbmcgdGhlIEFCSS4KPiA+Cj4gPiBTaWduZWQtb2ZmLWJ5OiBHdXN0YXZvIFBhZG92YW4gPGd1 c3Rhdm8ucGFkb3ZhbkBjb2xsYWJvcmEuY28udWs+Cj4gPiAtLS0KPiA+ICBkcml2ZXJzL3N0YWdp bmcvYW5kcm9pZC9zeW5jLmMgICAgICB8IDIgKy0KPiA+ICBkcml2ZXJzL3N0YWdpbmcvYW5kcm9p ZC91YXBpL3N5bmMuaCB8IDIgKy0KPiA+ICAyIGZpbGVzIGNoYW5nZWQsIDIgaW5zZXJ0aW9ucygr KSwgMiBkZWxldGlvbnMoLSkKPiA+Cj4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9zdGFnaW5nL2Fu ZHJvaWQvc3luYy5jIGIvZHJpdmVycy9zdGFnaW5nL2FuZHJvaWQvc3luYy5jCj4gPiBpbmRleCBm NzUzMGYwLi41MWQ0ZjQ3IDEwMDY0NAo+ID4gLS0tIGEvZHJpdmVycy9zdGFnaW5nL2FuZHJvaWQv c3luYy5jCj4gPiArKysgYi9kcml2ZXJzL3N0YWdpbmcvYW5kcm9pZC9zeW5jLmMKPiA+IEBAIC01 MjUsNyArNTI1LDcgQEAgc3RhdGljIGxvbmcgc3luY19maWxlX2lvY3RsX2ZlbmNlX2luZm8oc3Ry dWN0IHN5bmNfZmlsZSAqc3luY19maWxlLAo+ID4gIAlpZiAoaW5mby0+c3RhdHVzID49IDApCj4g PiAgCQlpbmZvLT5zdGF0dXMgPSAhaW5mby0+c3RhdHVzOwo+ID4gIAo+ID4gLQlsZW4gPSBzaXpl b2Yoc3RydWN0IHN5bmNfZmlsZV9pbmZvKTsKPiA+ICsJbGVuID0gc2l6ZW9mKHN0cnVjdCBzeW5j X2ZpbGVfaW5mbykgLSBzaXplb2YoX191NjQgKik7Cj4gPiAgCj4gPiAgCWZvciAoaSA9IDA7IGkg PCBzeW5jX2ZpbGUtPm51bV9mZW5jZXM7ICsraSkgewo+ID4gIAkJc3RydWN0IGZlbmNlICpmZW5j ZSA9IHN5bmNfZmlsZS0+Y2JzW2ldLmZlbmNlOwo+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvc3Rh Z2luZy9hbmRyb2lkL3VhcGkvc3luYy5oIGIvZHJpdmVycy9zdGFnaW5nL2FuZHJvaWQvdWFwaS9z eW5jLmgKPiA+IGluZGV4IGVkMjgxZmMuLjlmMDdhYTcgMTAwNjQ0Cj4gPiAtLS0gYS9kcml2ZXJz L3N0YWdpbmcvYW5kcm9pZC91YXBpL3N5bmMuaAo+ID4gKysrIGIvZHJpdmVycy9zdGFnaW5nL2Fu ZHJvaWQvdWFwaS9zeW5jLmgKPiA+IEBAIC01NCw3ICs1NCw3IEBAIHN0cnVjdCBzeW5jX2ZpbGVf aW5mbyB7Cj4gPiAgCWNoYXIJbmFtZVszMl07Cj4gPiAgCV9fczMyCXN0YXR1czsKPiA+ICAKPiA+ IC0JX191OAlmZW5jZV9pbmZvWzBdOwo+ID4gKwlfX3U2NAkqZmVuY2VfaW5mbzsKPiA+ICB9Owo+ ID4KPiBQb2ludGVycyBhcmUgYXdmdWwsIGl0IHNob3VsZCBiZSBhIF9fdTY0IHNpbmNlIGl0J3Mg YSBwb2ludGVyIHR5cGUuIFVzZXJzcGFjZSBzaG91bGQgY2FzdCBpdCB0byBhIHVpbnRwdHJfdCBp biB1c2Vyc3BhY2UuCgpPaCwgSSBtYWRlIGEgbWlzdGFrZS4gSSdsbCBmaXggdGhpcy4KCgo+IFRo aXMgc3RydWN0dXJlIGFsc28gd29uJ3Qgd29yayBvbiA2NC1iaXRzIHN5c3RlbXMsIHRoZXJlIG1h eSBiZSBhIGhvbGUgYmV0d2VlbiBmZW5jZV9pbmZvIGFuZCBzdGF0dXMgKG9yIG51bV9mZW5jZXMg aW4gbmV4dCBwYXRjaCkuCj4gCj4gSXQncyBwcm9iYWJseSBiZXN0IHRvIG1vdmUgaXQgdG8gdGhl IHRvcCBhbmQgZW5zdXJlIHRoZSBzdHJ1Y3QgaXMgNjQtYml0cyBhbGlnbmVkLgoKVGhhdCBpcyBu b3QgcG9zc2libGUgYmVjYXVzZSB3ZSBhcmUgbm90IGFsbG9jYXRpbmcgb25seSA2NGJpdHMgdGhl cmUgYnV0CmEgYXJyYXkgb2Ygc3RydWN0IGZlbmNlX2luZm8sIHNvIGl0IG5lZWRzIHRvIGJlIHRo ZSBsYXN0IG9uZS4gTWF5YmUgd2UKY2FuIGFkZCBzb21lIHNvcnQgb2YgcGFkZGluZz8KCgoJR3Vz dGF2bwpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmkt ZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cDov L2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752764AbcBASAU (ORCPT ); Mon, 1 Feb 2016 13:00:20 -0500 Received: from mail-yk0-f194.google.com ([209.85.160.194]:34184 "EHLO mail-yk0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752451AbcBASAN (ORCPT ); Mon, 1 Feb 2016 13:00:13 -0500 Date: Mon, 1 Feb 2016 16:00:08 -0200 From: Gustavo Padovan To: Maarten Lankhorst Cc: Greg Kroah-Hartman , linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org, dri-devel@lists.freedesktop.org, Daniel Stone , Arve =?iso-8859-1?B?SGr4bm5lduVn?= , Riley Andrews , Daniel Vetter , Rob Clark , Greg Hackmann , John Harrison , Gustavo Padovan Subject: Re: [PATCH 06/10] staging/android: turn fence_info into a __64 pointer Message-ID: <20160201180008.GB3207@joana> Mail-Followup-To: Gustavo Padovan , Maarten Lankhorst , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org, dri-devel@lists.freedesktop.org, Daniel Stone , Arve =?iso-8859-1?B?SGr4bm5lduVn?= , Riley Andrews , Daniel Vetter , Rob Clark , Greg Hackmann , John Harrison , Gustavo Padovan References: <1454102426-20637-1-git-send-email-gustavo@padovan.org> <1454102426-20637-7-git-send-email-gustavo@padovan.org> <56AF1A43.5010900@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56AF1A43.5010900@linux.intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Maarten, 2016-02-01 Maarten Lankhorst : > Op 29-01-16 om 22:20 schreef Gustavo Padovan: > > From: Gustavo Padovan > > > > Making fence_info a pointer enables us to extend the struct in the future > > without breaking the ABI. > > > > Signed-off-by: Gustavo Padovan > > --- > > drivers/staging/android/sync.c | 2 +- > > drivers/staging/android/uapi/sync.h | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c > > index f7530f0..51d4f47 100644 > > --- a/drivers/staging/android/sync.c > > +++ b/drivers/staging/android/sync.c > > @@ -525,7 +525,7 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, > > if (info->status >= 0) > > info->status = !info->status; > > > > - len = sizeof(struct sync_file_info); > > + len = sizeof(struct sync_file_info) - sizeof(__u64 *); > > > > for (i = 0; i < sync_file->num_fences; ++i) { > > struct fence *fence = sync_file->cbs[i].fence; > > diff --git a/drivers/staging/android/uapi/sync.h b/drivers/staging/android/uapi/sync.h > > index ed281fc..9f07aa7 100644 > > --- a/drivers/staging/android/uapi/sync.h > > +++ b/drivers/staging/android/uapi/sync.h > > @@ -54,7 +54,7 @@ struct sync_file_info { > > char name[32]; > > __s32 status; > > > > - __u8 fence_info[0]; > > + __u64 *fence_info; > > }; > > > Pointers are awful, it should be a __u64 since it's a pointer type. Userspace should cast it to a uintptr_t in userspace. Oh, I made a mistake. I'll fix this. > This structure also won't work on 64-bits systems, there may be a hole between fence_info and status (or num_fences in next patch). > > It's probably best to move it to the top and ensure the struct is 64-bits aligned. That is not possible because we are not allocating only 64bits there but a array of struct fence_info, so it needs to be the last one. Maybe we can add some sort of padding? Gustavo