From mboxrd@z Thu Jan 1 00:00:00 1970 From: Maarten Lankhorst Subject: Re: [PATCH v2 06/11] staging/android: turn fence_info into a __u64 pointer Date: Wed, 3 Feb 2016 09:32:20 +0100 Message-ID: <56B1BB14.2020307@linux.intel.com> References: <1454419402-10769-1-git-send-email-gustavo@padovan.org> <1454419402-10769-7-git-send-email-gustavo@padovan.org> <56B0BA29.1040003@linux.intel.com> <20160202202807.GA2808@joana> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTP id BA0206E684 for ; Wed, 3 Feb 2016 00:32:24 -0800 (PST) In-Reply-To: <20160202202807.GA2808@joana> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Gustavo Padovan , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org, dri-devel@lists.freedesktop.org, Daniel Stone , =?UTF-8?Q?Arve_Hj=c3=b8nnev=c3=a5g?= , Riley Andrews , Daniel Vetter , Rob Clark , Greg Hackmann , John Harrison , Gustavo Padovan List-Id: dri-devel@lists.freedesktop.org T3AgMDItMDItMTYgb20gMjE6Mjggc2NocmVlZiBHdXN0YXZvIFBhZG92YW46Cj4gSGkgTWFhcnRl biwKPgo+IDIwMTYtMDItMDIgTWFhcnRlbiBMYW5raG9yc3QgPG1hYXJ0ZW4ubGFua2hvcnN0QGxp bnV4LmludGVsLmNvbT46Cj4KPj4gT3AgMDItMDItMTYgb20gMTQ6MjMgc2NocmVlZiBHdXN0YXZv IFBhZG92YW46Cj4+PiBGcm9tOiBHdXN0YXZvIFBhZG92YW4gPGd1c3Rhdm8ucGFkb3ZhbkBjb2xs YWJvcmEuY28udWs+Cj4+Pgo+Pj4gTWFraW5nIGZlbmNlX2luZm8gYSBwb2ludGVyIGVuYWJsZXMg dXMgdG8gZXh0ZW5kIHRoZSBzdHJ1Y3QgaW4gdGhlIGZ1dHVyZQo+Pj4gd2l0aG91dCBicmVha2lu ZyB0aGUgQUJJLgo+Pj4KPj4+IHYyOiB1c2UgdHlwZSBfX3U2NCBmb3IgZmVuY2VfaW5mbwo+Pj4g U2lnbmVkLW9mZi1ieTogR3VzdGF2byBQYWRvdmFuIDxndXN0YXZvLnBhZG92YW5AY29sbGFib3Jh LmNvLnVrPgo+Pj4gLS0tCj4+PiAgZHJpdmVycy9zdGFnaW5nL2FuZHJvaWQvc3luYy5jICAgICAg fCAyICstCj4+PiAgZHJpdmVycy9zdGFnaW5nL2FuZHJvaWQvdWFwaS9zeW5jLmggfCAyICstCj4+ PiAgMiBmaWxlcyBjaGFuZ2VkLCAyIGluc2VydGlvbnMoKyksIDIgZGVsZXRpb25zKC0pCj4+Pgo+ Pj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvc3RhZ2luZy9hbmRyb2lkL3N5bmMuYyBiL2RyaXZlcnMv c3RhZ2luZy9hbmRyb2lkL3N5bmMuYwo+Pj4gaW5kZXggZjc1MzBmMC4uMDNiMTIxNCAxMDA2NDQK Pj4+IC0tLSBhL2RyaXZlcnMvc3RhZ2luZy9hbmRyb2lkL3N5bmMuYwo+Pj4gKysrIGIvZHJpdmVy cy9zdGFnaW5nL2FuZHJvaWQvc3luYy5jCj4+PiBAQCAtNTI1LDcgKzUyNSw3IEBAIHN0YXRpYyBs b25nIHN5bmNfZmlsZV9pb2N0bF9mZW5jZV9pbmZvKHN0cnVjdCBzeW5jX2ZpbGUgKnN5bmNfZmls ZSwKPj4+ICAJaWYgKGluZm8tPnN0YXR1cyA+PSAwKQo+Pj4gIAkJaW5mby0+c3RhdHVzID0gIWlu Zm8tPnN0YXR1czsKPj4+ICAKPj4+IC0JbGVuID0gc2l6ZW9mKHN0cnVjdCBzeW5jX2ZpbGVfaW5m byk7Cj4+PiArCWxlbiA9IHNpemVvZihzdHJ1Y3Qgc3luY19maWxlX2luZm8pIC0gc2l6ZW9mKF9f dTY0KTsKPj4+ICAKPj4+ICAJZm9yIChpID0gMDsgaSA8IHN5bmNfZmlsZS0+bnVtX2ZlbmNlczsg KytpKSB7Cj4+PiAgCQlzdHJ1Y3QgZmVuY2UgKmZlbmNlID0gc3luY19maWxlLT5jYnNbaV0uZmVu Y2U7Cj4+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9zdGFnaW5nL2FuZHJvaWQvdWFwaS9zeW5jLmgg Yi9kcml2ZXJzL3N0YWdpbmcvYW5kcm9pZC91YXBpL3N5bmMuaAo+Pj4gaW5kZXggZWQyODFmYy4u OGUyZWQzMiAxMDA2NDQKPj4+IC0tLSBhL2RyaXZlcnMvc3RhZ2luZy9hbmRyb2lkL3VhcGkvc3lu Yy5oCj4+PiArKysgYi9kcml2ZXJzL3N0YWdpbmcvYW5kcm9pZC91YXBpL3N5bmMuaAo+Pj4gQEAg LTU0LDcgKzU0LDcgQEAgc3RydWN0IHN5bmNfZmlsZV9pbmZvIHsKPj4+ICAJY2hhcgluYW1lWzMy XTsKPj4+ICAJX19zMzIJc3RhdHVzOwo+Pj4gIAo+Pj4gLQlfX3U4CWZlbmNlX2luZm9bMF07Cj4+ PiArCV9fdTY0CWZlbmNlX2luZm87Cj4+Pgo+PiBXaXRoIHRoZSBpbmZvcm1hdGlvbiBmcm9tIHRo ZSBjb21taXQgSSB3b3VsZCBleHBlY3QgZmVuY2VfaW5mbyB3b3VsZCBiZSBhIHRydWUgcG9pbnRl ci4KPiBJIG1pc3NlZCB0aGF0LiBJdCBpcyBmaXhlZCBmb3IgdjMgbm93Lgo+Cj4+IHNvIGlmIChz eW5jX2ZpbGVfaW5mby0+bnVtX2ZlbmNlcyA9PSAwKSBub3QgY29weWluZyBmZW5jZV9pbmZvLCBi dXQgc2V0IGFsbCBtZW1iZXJzIGluY2x1ZGluZyB0aGlzIG9uZS4KPj4gZWxzZSBmb3JfYWxsX2Zl bmNlcygpIGNvcHlfdG9fdXNlcihmZW5jZV9pbmZvIHN0cnVjdCkKPj4KPj4gT3RoZXJ3aXNlIHlv dSBzdGlsbCBjYW4ndCBleHRlbmQgdGhlIHN0cnVjdCB3aXRob3V0IGJyZWFraW5nIHRoZSBhYmku Cj4gYWxsIHN5bmNfZmlsZXMgaGF2ZSBhdCBsZWFzdCAxIGZlbmNlIHNvIHdlIGFyZSBsZWZ0IHRv IHRoZSBlbHNlIGNhc2UsCj4gY291cHkgYWxsIGZlbmNlcyB0byB0aGUgdXNlci4gU28gd2UgYWx3 YXlzIGhhdmUgdmFyaWFibGUgbGVuZ3RoIGF0IHRoZQo+IGVuZCBvZiB0aGUgc3RydWN0LiBCdXQg SSBzdGlsbCBkb24ndCBzZWUgaG93IHRoaXMgd2lsbCBwcmV2ZW50IHVzIGZyb20KPiBicmVha2lu ZyB0aGUgQVBJLgpPayB0aGlzIHdhcyB3aXRoIGZlbmNlX2luZm8gbm90IGJlaW5nIGEgcG9pbnRl ci4gOikKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJp LWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6 Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756701AbcBCIc0 (ORCPT ); Wed, 3 Feb 2016 03:32:26 -0500 Received: from mga03.intel.com ([134.134.136.65]:58471 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756435AbcBCIcZ (ORCPT ); Wed, 3 Feb 2016 03:32:25 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,389,1449561600"; d="scan'208";a="739671713" Subject: Re: [PATCH v2 06/11] staging/android: turn fence_info into a __u64 pointer To: Gustavo Padovan , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org, dri-devel@lists.freedesktop.org, Daniel Stone , =?UTF-8?Q?Arve_Hj=c3=b8nnev=c3=a5g?= , 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> <20160202202807.GA2808@joana> From: Maarten Lankhorst Message-ID: <56B1BB14.2020307@linux.intel.com> Date: Wed, 3 Feb 2016 09:32:20 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <20160202202807.GA2808@joana> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Op 02-02-16 om 21:28 schreef Gustavo Padovan: > 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. Ok this was with fence_info not being a pointer. :)