From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gustavo Padovan Subject: Re: [PATCH v2 06/11] staging/android: turn fence_info into a __u64 pointer Date: Tue, 2 Feb 2016 18:28:07 -0200 Message-ID: <20160202202807.GA2808@joana> References: <1454419402-10769-1-git-send-email-gustavo@padovan.org> <1454419402-10769-7-git-send-email-gustavo@padovan.org> <56B0BA29.1040003@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-qk0-f195.google.com (mail-qk0-f195.google.com [209.85.220.195]) by gabe.freedesktop.org (Postfix) with ESMTPS id DEE676E3E4 for ; Tue, 2 Feb 2016 12:28:11 -0800 (PST) Received: by mail-qk0-f195.google.com with SMTP id u128so5558864qkh.1 for ; Tue, 02 Feb 2016 12:28:11 -0800 (PST) Content-Disposition: inline In-Reply-To: <56B0BA29.1040003@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 SGkgTWFhcnRlbiwKCjIwMTYtMDItMDIgTWFhcnRlbiBMYW5raG9yc3QgPG1hYXJ0ZW4ubGFua2hv cnN0QGxpbnV4LmludGVsLmNvbT46Cgo+IE9wIDAyLTAyLTE2IG9tIDE0OjIzIHNjaHJlZWYgR3Vz dGF2byBQYWRvdmFuOgo+ID4gRnJvbTogR3VzdGF2byBQYWRvdmFuIDxndXN0YXZvLnBhZG92YW5A Y29sbGFib3JhLmNvLnVrPgo+ID4KPiA+IE1ha2luZyBmZW5jZV9pbmZvIGEgcG9pbnRlciBlbmFi bGVzIHVzIHRvIGV4dGVuZCB0aGUgc3RydWN0IGluIHRoZSBmdXR1cmUKPiA+IHdpdGhvdXQgYnJl YWtpbmcgdGhlIEFCSS4KPiA+Cj4gPiB2MjogdXNlIHR5cGUgX191NjQgZm9yIGZlbmNlX2luZm8K PiA+IFNpZ25lZC1vZmYtYnk6IEd1c3Rhdm8gUGFkb3ZhbiA8Z3VzdGF2by5wYWRvdmFuQGNvbGxh Ym9yYS5jby51az4KPiA+IC0tLQo+ID4gIGRyaXZlcnMvc3RhZ2luZy9hbmRyb2lkL3N5bmMuYyAg ICAgIHwgMiArLQo+ID4gIGRyaXZlcnMvc3RhZ2luZy9hbmRyb2lkL3VhcGkvc3luYy5oIHwgMiAr LQo+ID4gIDIgZmlsZXMgY2hhbmdlZCwgMiBpbnNlcnRpb25zKCspLCAyIGRlbGV0aW9ucygtKQo+ ID4KPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL3N0YWdpbmcvYW5kcm9pZC9zeW5jLmMgYi9kcml2 ZXJzL3N0YWdpbmcvYW5kcm9pZC9zeW5jLmMKPiA+IGluZGV4IGY3NTMwZjAuLjAzYjEyMTQgMTAw NjQ0Cj4gPiAtLS0gYS9kcml2ZXJzL3N0YWdpbmcvYW5kcm9pZC9zeW5jLmMKPiA+ICsrKyBiL2Ry aXZlcnMvc3RhZ2luZy9hbmRyb2lkL3N5bmMuYwo+ID4gQEAgLTUyNSw3ICs1MjUsNyBAQCBzdGF0 aWMgbG9uZyBzeW5jX2ZpbGVfaW9jdGxfZmVuY2VfaW5mbyhzdHJ1Y3Qgc3luY19maWxlICpzeW5j X2ZpbGUsCj4gPiAgCWlmIChpbmZvLT5zdGF0dXMgPj0gMCkKPiA+ICAJCWluZm8tPnN0YXR1cyA9 ICFpbmZvLT5zdGF0dXM7Cj4gPiAgCj4gPiAtCWxlbiA9IHNpemVvZihzdHJ1Y3Qgc3luY19maWxl X2luZm8pOwo+ID4gKwlsZW4gPSBzaXplb2Yoc3RydWN0IHN5bmNfZmlsZV9pbmZvKSAtIHNpemVv ZihfX3U2NCk7Cj4gPiAgCj4gPiAgCWZvciAoaSA9IDA7IGkgPCBzeW5jX2ZpbGUtPm51bV9mZW5j ZXM7ICsraSkgewo+ID4gIAkJc3RydWN0IGZlbmNlICpmZW5jZSA9IHN5bmNfZmlsZS0+Y2JzW2ld LmZlbmNlOwo+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvc3RhZ2luZy9hbmRyb2lkL3VhcGkvc3lu Yy5oIGIvZHJpdmVycy9zdGFnaW5nL2FuZHJvaWQvdWFwaS9zeW5jLmgKPiA+IGluZGV4IGVkMjgx ZmMuLjhlMmVkMzIgMTAwNjQ0Cj4gPiAtLS0gYS9kcml2ZXJzL3N0YWdpbmcvYW5kcm9pZC91YXBp L3N5bmMuaAo+ID4gKysrIGIvZHJpdmVycy9zdGFnaW5nL2FuZHJvaWQvdWFwaS9zeW5jLmgKPiA+ IEBAIC01NCw3ICs1NCw3IEBAIHN0cnVjdCBzeW5jX2ZpbGVfaW5mbyB7Cj4gPiAgCWNoYXIJbmFt ZVszMl07Cj4gPiAgCV9fczMyCXN0YXR1czsKPiA+ICAKPiA+IC0JX191OAlmZW5jZV9pbmZvWzBd Owo+ID4gKwlfX3U2NAlmZW5jZV9pbmZvOwo+ID4KPiBXaXRoIHRoZSBpbmZvcm1hdGlvbiBmcm9t IHRoZSBjb21taXQgSSB3b3VsZCBleHBlY3QgZmVuY2VfaW5mbyB3b3VsZCBiZSBhIHRydWUgcG9p bnRlci4KCkkgbWlzc2VkIHRoYXQuIEl0IGlzIGZpeGVkIGZvciB2MyBub3cuCgo+IAo+IHNvIGlm IChzeW5jX2ZpbGVfaW5mby0+bnVtX2ZlbmNlcyA9PSAwKSBub3QgY29weWluZyBmZW5jZV9pbmZv LCBidXQgc2V0IGFsbCBtZW1iZXJzIGluY2x1ZGluZyB0aGlzIG9uZS4KPiBlbHNlIGZvcl9hbGxf ZmVuY2VzKCkgY29weV90b191c2VyKGZlbmNlX2luZm8gc3RydWN0KQo+Cj4gT3RoZXJ3aXNlIHlv dSBzdGlsbCBjYW4ndCBleHRlbmQgdGhlIHN0cnVjdCB3aXRob3V0IGJyZWFraW5nIHRoZSBhYmku CgphbGwgc3luY19maWxlcyBoYXZlIGF0IGxlYXN0IDEgZmVuY2Ugc28gd2UgYXJlIGxlZnQgdG8g dGhlIGVsc2UgY2FzZSwKY291cHkgYWxsIGZlbmNlcyB0byB0aGUgdXNlci4gU28gd2UgYWx3YXlz IGhhdmUgdmFyaWFibGUgbGVuZ3RoIGF0IHRoZQplbmQgb2YgdGhlIHN0cnVjdC4gQnV0IEkgc3Rp bGwgZG9uJ3Qgc2VlIGhvdyB0aGlzIHdpbGwgcHJldmVudCB1cyBmcm9tCmJyZWFraW5nIHRoZSBB UEkuCgoJR3VzdGF2bwpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5v cmcKaHR0cDovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZl bAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756379AbcBBU2O (ORCPT ); Tue, 2 Feb 2016 15:28:14 -0500 Received: from mail-qk0-f193.google.com ([209.85.220.193]:34874 "EHLO mail-qk0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750793AbcBBU2M (ORCPT ); Tue, 2 Feb 2016 15:28:12 -0500 Date: Tue, 2 Feb 2016 18:28:07 -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 v2 06/11] staging/android: turn fence_info into a __u64 pointer Message-ID: <20160202202807.GA2808@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: <1454419402-10769-1-git-send-email-gustavo@padovan.org> <1454419402-10769-7-git-send-email-gustavo@padovan.org> <56B0BA29.1040003@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56B0BA29.1040003@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-02 Maarten Lankhorst : > Op 02-02-16 om 14:23 schreef Gustavo Padovan: > > From: Gustavo Padovan > > > > Making fence_info a pointer enables us to extend the struct in the future > > without breaking the ABI. > > > > v2: use type __u64 for fence_info > > 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..03b1214 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..8e2ed32 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; > > > With the information from the commit I would expect fence_info would be a true pointer. I missed that. It is fixed for v3 now. > > so if (sync_file_info->num_fences == 0) not copying fence_info, but set all members including this one. > else for_all_fences() copy_to_user(fence_info struct) > > Otherwise you still can't extend the struct without breaking the abi. all sync_files have at least 1 fence so we are left to the else case, coupy all fences to the user. So we always have variable length at the end of the struct. But I still don't see how this will prevent us from breaking the API. Gustavo