From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gustavo Padovan Subject: Re: [PATCH v3 06/11] staging/android: turn fence_info into a __u64 pointer Date: Wed, 3 Feb 2016 18:09:57 -0200 Message-ID: <20160203200957.GG2808@joana> References: <1454505940-18094-1-git-send-email-gustavo@padovan.org> <1454505940-18094-7-git-send-email-gustavo@padovan.org> <56B2111A.8080103@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-yw0-f193.google.com (mail-yw0-f193.google.com [209.85.161.193]) by gabe.freedesktop.org (Postfix) with ESMTPS id D7E0E6E0C9 for ; Wed, 3 Feb 2016 12:10:02 -0800 (PST) Received: by mail-yw0-f193.google.com with SMTP id f6so565080ywa.1 for ; Wed, 03 Feb 2016 12:10:02 -0800 (PST) Content-Disposition: inline In-Reply-To: <56B2111A.8080103@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 SGkgTWFhcnRlbiwKCjIwMTYtMDItMDMgTWFhcnRlbiBMYW5raG9yc3QgPG1hYXJ0ZW4ubGFua2hv cnN0QGxpbnV4LmludGVsLmNvbT46Cgo+IE9wIDAzLTAyLTE2IG9tIDE0OjI1IHNjaHJlZWYgR3Vz dGF2byBQYWRvdmFuOgo+ID4gRnJvbTogR3VzdGF2byBQYWRvdmFuIDxndXN0YXZvLnBhZG92YW5A Y29sbGFib3JhLmNvLnVrPgo+ID4KPiA+IFR1cm4gc3luY19mZW5jZV9pbmZvIGludG8gX191NjQg dHlwZSBlbmFibGUgdXMgdG8gZXh0ZW5kIHRoZSBzdHJ1Y3QgaW4gdGhlCj4gPiBmdXR1cmUgd2l0 aG91dCBicmVha2luZyB0aGUgQUJJLgo+ID4KPiA+IHYyOiB1c2UgdHlwZSBfX3U2NCBmb3IgZmVu Y2VfaW5mbwo+ID4KPiA+IHYzOiBmaXggY29tbWl0IG1lc3NhZ2UgdG8gcmVmbGVjdCB0aGUgdjIg Y2hhbmdlCj4gPgo+ID4gU2lnbmVkLW9mZi1ieTogR3VzdGF2byBQYWRvdmFuIDxndXN0YXZvLnBh ZG92YW5AY29sbGFib3JhLmNvLnVrPgo+ID4gLS0tCj4gPiAgZHJpdmVycy9zdGFnaW5nL2FuZHJv aWQvc3luYy5jICAgICAgfCAyICstCj4gPiAgZHJpdmVycy9zdGFnaW5nL2FuZHJvaWQvdWFwaS9z eW5jLmggfCAyICstCj4gPiAgMiBmaWxlcyBjaGFuZ2VkLCAyIGluc2VydGlvbnMoKyksIDIgZGVs ZXRpb25zKC0pCj4gPgo+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvc3RhZ2luZy9hbmRyb2lkL3N5 bmMuYyBiL2RyaXZlcnMvc3RhZ2luZy9hbmRyb2lkL3N5bmMuYwo+ID4gaW5kZXggMmFiMGMyMC4u ODQyNTQ1NyAxMDA2NDQKPiA+IC0tLSBhL2RyaXZlcnMvc3RhZ2luZy9hbmRyb2lkL3N5bmMuYwo+ ID4gKysrIGIvZHJpdmVycy9zdGFnaW5nL2FuZHJvaWQvc3luYy5jCj4gPiBAQCAtNTI1LDcgKzUy NSw3IEBAIHN0YXRpYyBsb25nIHN5bmNfZmlsZV9pb2N0bF9mZW5jZV9pbmZvKHN0cnVjdCBzeW5j X2ZpbGUgKnN5bmNfZmlsZSwKPiA+ICAJaWYgKGluZm8tPnN0YXR1cyA+PSAwKQo+ID4gIAkJaW5m by0+c3RhdHVzID0gIWluZm8tPnN0YXR1czsKPiA+ICAKPiA+IC0JbGVuID0gc2l6ZW9mKHN0cnVj dCBzeW5jX2ZpbGVfaW5mbyk7Cj4gPiArCWxlbiA9IHNpemVvZihzdHJ1Y3Qgc3luY19maWxlX2lu Zm8pIC0gc2l6ZW9mKF9fdTY0KTsKPiA+ICAKPiA+ICAJZm9yIChpID0gMDsgaSA8IHN5bmNfZmls ZS0+bnVtX2ZlbmNlczsgKytpKSB7Cj4gPiAgCQlzdHJ1Y3QgZmVuY2UgKmZlbmNlID0gc3luY19m aWxlLT5jYnNbaV0uZmVuY2U7Cj4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9zdGFnaW5nL2FuZHJv aWQvdWFwaS9zeW5jLmggYi9kcml2ZXJzL3N0YWdpbmcvYW5kcm9pZC91YXBpL3N5bmMuaAo+ID4g aW5kZXggYTBjZjM1Ny4uZTY0OTk1MyAxMDA2NDQKPiA+IC0tLSBhL2RyaXZlcnMvc3RhZ2luZy9h bmRyb2lkL3VhcGkvc3luYy5oCj4gPiArKysgYi9kcml2ZXJzL3N0YWdpbmcvYW5kcm9pZC91YXBp L3N5bmMuaAo+ID4gQEAgLTU0LDcgKzU0LDcgQEAgc3RydWN0IHN5bmNfZmlsZV9pbmZvIHsKPiA+ ICAJY2hhcgluYW1lWzMyXTsKPiA+ICAJX19zMzIJc3RhdHVzOwo+ID4gIAo+ID4gLQlfX3U4CXN5 bmNfZmVuY2VfaW5mb1swXTsKPiA+ICsJX191NjQJc3luY19mZW5jZV9pbmZvOwo+ID4gIH07Cj4g PiAgCj4gPiAgI2RlZmluZSBTWU5DX0lPQ19NQUdJQwkJJz4nCj4gVGhpcyBzdGlsbCBkb2Vzbid0 IGRvIHdoYXQgeW91IGV4cGVjdCBpdCB0by4KPiAKPiBJIHRoaW5rIHRoaXMgaXMgd2hhdCB5b3Ug d2FudCBpcyBmb3IgdXNlcnNwYWNlIHRvIGRvOgo+IAo+IHN0cnVjdCBzeW5jX2ZpbGVfaW5mbyBp bmZvOwo+IAo+IGluZm8uZmxhZ3MgPSBpbmZvLm51bV9mZW5jZXMgPSAwOwo+IGlvY3RsKGZkLCBT WU5DX0lPQ19GRU5DRV9JTkZPLCAmaW5mbyk7Cj4gaWYgKGluZm8ubnVtX2ZlbmNlcykgewo+IGlu Zm8uc3luY19mZW5jZV9pbmZvID0gKHVpbnRwdHIpa2NhbGxvYyhpbmZvLm51bV9mZW5jZXMsIHNp emVvZihzdHJ1Y3Qgc3luY19mZW5jZV9pbmZvKSk7Cj4gaW9jdGwoZmQsIFNZTkNfSU9DX0ZFTkNF X0lORk8sICZpbmZvKTsKPiB9Cj4gCj4gTWF5YmUgdXNlcnNwYWNlIGNvdWxkIHByZWFsbG9jYXRl IHRoZSBtYXggaW4gYWR2YW5jZSBhbmQgc2V0IG51bV9mZW5jZXMgaGlnaGVyLAo+IAo+IGtlcm5l bCB3b3VsZCBkbyBzb21ldGhpbmcgbGlrZToKPiAKPiBudW1fZmVuY2VzID0gbWluKGluZm8ubnVt X2ZlbmNlcywgc3luYy0+bnVtX2ZlbmNlcyk7Cj4gc3RydWN0IHN5bmNfZmVuY2VfaW5mbyBhcnJh eVtudW1fZmVuY2VzXTsKPiAKPiBpbmZvLm51bV9mZW5jZXMgPSBzeW5jLT5udW1fZmVuY2VzOwo+ IGlmIChudW1fZmVuY2VzICYmCj4gICAgIGNvcHlfdG9fdXNlcigodm9pZCAqIF9fdXNlcikodW5z aWduZWQgbG9uZylpbmZvLnN5bmNfZmVuY2VfaW5mbywgYXJyYXksIG51bV9mZW5jZXMgICogc2l6 ZW9mKGFycmF5KSkpCj4gIHJldHVybiAtRUZBVUxUOwoKSWYgd2UgYXJlIGdvaW5nIHRvIGNhbGwg SU9DVEwgdHdpY2UgSSB3b3VsZCBhY3R1YWxseSBoYXZlIGEgbmV3IElPQ1RMIG9ubHkKdG8gZmV0 Y2ggc3luY19mZW5jZV9pbmZvLgoKRmlyc3Qgd2Ugd291bGQgY2FsbAoKaW9jdGwoZmQsIFNZTkNf SU9DX0ZJTEVfSU5GTywgJmluZm8pOwoKd2hlcmUgaW5mbyBpczoKCnN0cnVjdCBzeW5jX2ZpbGVf aW5mbyB7ICAgIAogICAgICAgIGNoYXIgICAgbmFtZVszMl07ICAKICAgICAgICBfX3MzMiAgIHN0 YXR1czsgICAgCiAgICAgICAgX191MzIgICBmbGFnczsgICAgIAogICAgICAgIF9fdTMyICAgbnVt X2ZlbmNlczsKfTsKCnRoZW4gd2Ugd291bGQgYWxsb2NhdGUgYSBidWZmZXIgd2l0aAoKc2l6ZSA9 IGluZm8ubnVtX2ZlbmNlcyAqIHNpemVvZihzdHJ1Y3Qgc3luY19mZW5jZV9pbmZvKQoKYW5kIGNh bGwgdGhlIG5ldyBpb2N0bAoKaW9jdGwoZmQsIFNZTkNfSU9DX1NZTkNfRkVOQ0VfSU5GTywgc3lu Y19mZW5jZV9pbmZvKTsKClRoaXMgbG9va3MgbGlrZSBhIGNsZWFuZXIgc29sdXRpb24gYW5kIGRv ZXNuJ3QgYnJlYWsgQUJJLiBXaGF0IGRvIHlvdQp0aGluaz8KCglHdXN0YXZvCl9fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxp c3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwOi8vbGlzdHMuZnJlZWRlc2t0 b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932224AbcBCUKG (ORCPT ); Wed, 3 Feb 2016 15:10:06 -0500 Received: from mail-yw0-f193.google.com ([209.85.161.193]:32832 "EHLO mail-yw0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753859AbcBCUKD (ORCPT ); Wed, 3 Feb 2016 15:10:03 -0500 X-Greylist: delayed 110756 seconds by postgrey-1.27 at vger.kernel.org; Wed, 03 Feb 2016 15:10:02 EST Date: Wed, 3 Feb 2016 18:09:57 -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 v3 06/11] staging/android: turn fence_info into a __u64 pointer Message-ID: <20160203200957.GG2808@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: <1454505940-18094-1-git-send-email-gustavo@padovan.org> <1454505940-18094-7-git-send-email-gustavo@padovan.org> <56B2111A.8080103@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56B2111A.8080103@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-03 Maarten Lankhorst : > Op 03-02-16 om 14:25 schreef Gustavo Padovan: > > From: Gustavo Padovan > > > > Turn sync_fence_info into __u64 type enable us to extend the struct in the > > future without breaking the ABI. > > > > v2: use type __u64 for fence_info > > > > v3: fix commit message to reflect the v2 change > > > > 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 2ab0c20..8425457 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 a0cf357..e649953 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 sync_fence_info[0]; > > + __u64 sync_fence_info; > > }; > > > > #define SYNC_IOC_MAGIC '>' > This still doesn't do what you expect it to. > > I think this is what you want is for userspace to do: > > struct sync_file_info info; > > info.flags = info.num_fences = 0; > ioctl(fd, SYNC_IOC_FENCE_INFO, &info); > if (info.num_fences) { > info.sync_fence_info = (uintptr)kcalloc(info.num_fences, sizeof(struct sync_fence_info)); > ioctl(fd, SYNC_IOC_FENCE_INFO, &info); > } > > Maybe userspace could preallocate the max in advance and set num_fences higher, > > kernel would do something like: > > num_fences = min(info.num_fences, sync->num_fences); > struct sync_fence_info array[num_fences]; > > info.num_fences = sync->num_fences; > if (num_fences && > copy_to_user((void * __user)(unsigned long)info.sync_fence_info, array, num_fences * sizeof(array))) > return -EFAULT; If we are going to call IOCTL twice I would actually have a new IOCTL only to fetch sync_fence_info. First we would call ioctl(fd, SYNC_IOC_FILE_INFO, &info); where info is: struct sync_file_info { char name[32]; __s32 status; __u32 flags; __u32 num_fences; }; then we would allocate a buffer with size = info.num_fences * sizeof(struct sync_fence_info) and call the new ioctl ioctl(fd, SYNC_IOC_SYNC_FENCE_INFO, sync_fence_info); This looks like a cleaner solution and doesn't break ABI. What do you think? Gustavo