From mboxrd@z Thu Jan 1 00:00:00 1970 From: Maarten Lankhorst Subject: Re: [PATCH] staging/android: refactor SYNC_IOC_FILE_INFO Date: Tue, 1 Mar 2016 09:35:58 +0100 Message-ID: <56D5546E.4010701@linux.intel.com> References: <1456511507-2534-4-git-send-email-gustavo@padovan.org> <1456520410-28195-1-git-send-email-gustavo@padovan.org> <56D400CD.6070804@linux.intel.com> <20160229220839.GD2479@joana> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 578316E3C1 for ; Tue, 1 Mar 2016 08:36:02 +0000 (UTC) In-Reply-To: <20160229220839.GD2479@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 T3AgMjktMDItMTYgb20gMjM6MDggc2NocmVlZiBHdXN0YXZvIFBhZG92YW46Cj4gSGkgTWFhcnRl biwKPgo+IDIwMTYtMDItMjkgTWFhcnRlbiBMYW5raG9yc3QgPG1hYXJ0ZW4ubGFua2hvcnN0QGxp bnV4LmludGVsLmNvbT46Cj4KPj4gT3AgMjYtMDItMTYgb20gMjI6MDAgc2NocmVlZiBHdXN0YXZv IFBhZG92YW46Cj4+PiBGcm9tOiBHdXN0YXZvIFBhZG92YW4gPGd1c3Rhdm8ucGFkb3ZhbkBjb2xs YWJvcmEuY28udWs+Cj4+Pgo+Pj4gQ2hhbmdlIFNZTkNfSU9DX0ZJTEVfSU5GTyBiZWhhdmlvdXIg dG8gYXZvaWQgZnV0dXJlIEFQSSBicmVha3MgYW5kCj4+PiBvcHRpbWl6ZSBidWZmZXIgYWxsb2Nh dGlvbi4gSW4gdGhlIG5ldyBhcHByb2FjaCB0aGUgaW9jdGwgbmVlZHMgdG8gYmUgY2FsbGVkCj4+ PiB0d2ljZSB0byByZXRyaWV2ZSB0aGUgYXJyYXkgb2YgZmVuY2VfaW5mb3MgcG9pbnRlZCBieSBp bmZvLT5zeW5jX2ZlbmNlX2luZm8uCj4+Pgo+Pj4gVGhlIGZpcnN0IGNhbGwgc2hvdWxkIHBhc3Mg bnVtX2ZlbmNlcyA9IDAsIHRoZSBrZXJuZWwgd2lsbCB0aGVuIGZpbGwKPj4+IGluZm8tPm51bV9m ZW5jZXMuIFVzZXJzcGFjZSByZWNlaXZlcyBiYWNrIHRoZSBudW1iZXIgb2YgZmVuY2VzIGFuZAo+ Pj4gYWxsb2NhdGVzIGEgYnVmZmVyIHNpemUgbnVtX2ZlbmNlcyAqIHNpemVvZihzdHJ1Y3Qgc3lu Y19mZW5jZV9pbmZvKSBvbgo+Pj4gaW5mby0+c3luY19mZW5jZV9pbmZvLgo+Pj4KPj4+IEl0IHRo ZW4gY2FsbCB0aGUgaW9jdGwgYWdhaW4gcGFzc2luZyBudW1fZmVuY2VzIHJlY2VpdmVkIGluIGlu Zm8tPm51bV9mZW5jZXMuCj4+PiBUaGUga2VybmVsIGNoZWNrcyBpZiBpbmZvLT5udW1fZmVuY2Vz ID4gMCBhbmQgaWYgeWVzIGl0IGZpbGwKPj4+IGluZm8tPnN5bmNfZmVuY2VfaW5mbyB3aXRoIGFu IGFycmF5IGNvbnRhaW5pbmcgYWxsIGZlbmNlX2luZm9zLgo+Pj4KPj4+IGluZm8tPmxlbiBub3cg cmVwcmVzZW50cyB0aGUgbGVuZ3RoIG9mIHRoZSBidWZmZXIgc3luY19mZW5jZV9pbmZvIHBvaW50 cwo+Pj4gdG8uIEFsc28sIGluZm8tPnN5bmNfZmVuY2VfaW5mbyB3YXMgY29udmVydGVkIHRvIF9f dTY0IHBvaW50ZXIuCj4+Pgo+Pj4gQW4gZXhhbXBsZSB1c2Vyc3BhY2UgY29kZSB3b3VsZCBiZToK Pj4+Cj4+PiAJc3RydWN0IHN5bmNfZmlsZV9pbmZvICppbmZvOwo+Pj4gCWludCBlcnIsIHNpemUs IG51bV9mZW5jZXM7Cj4+Pgo+Pj4gCWluZm8gPSBtYWxsb2Moc2l6ZW9mKCppbmZvKSk7Cj4+Pgo+ Pj4gCW1lbXNldChpbmZvLCAwLCBzaXplb2YoKmluZm8pKTsKPj4+Cj4+PiAJZXJyID0gaW9jdGwo ZmQsIFNZTkNfSU9DX0ZJTEVfSU5GTywgaW5mbyk7Cj4+PiAJbnVtX2ZlbmNlcyA9IGluZm8tPm51 bV9mZW5jZXM7Cj4+Pgo+Pj4gCWlmIChudW1fZmVuY2VzKSB7Cj4+PiAJCW1lbXNldChpbmZvLCAw LCBzaXplb2YoKmluZm8pKTsKPj4gV291bGQgdGhpcyBtZW1zZXQgc3RpbGwgYmUgbmVlZGVkIGlm IHdlIGRpZG4ndCBjaGVjayBmb3IgbnVsbHMgaW4gaW5mby0+c3RhdHVzIGFuZCBpbmZvLT5uYW1l ID8KPj4KPj4gU2VlbXMgdG8gbWUgdGhhdCBpdCBjb3VsZCBiZSBza2lwcGVkIGluIHRoYXQgY2Fz ZS4KPiBZZXMsIEkgYWdyZWUuCj4KPj4+IAkJc2l6ZSA9IHNpemVvZihzdHJ1Y3Qgc3luY19mZW5j ZV9pbmZvKSAqIG51bV9mZW5jZXM7Cj4+PiAJCWluZm8tPmxlbiA9IHNpemU7Cj4+PiAJCWluZm8t Pm51bV9mZW5jZXMgPSBudW1fZmVuY2VzOwo+Pj4gCQlpbmZvLT5zeW5jX2ZlbmNlX2luZm8gPSAo dWludDY0X3QpIGNhbGxvYyhudW1fZmVuY2VzLAo+Pj4gCQkJCQkJCSAgc2l6ZW9mKHN0cnVjdCBz eW5jX2ZlbmNlX2luZm8pKTsKPj4+Cj4+PiAJCWVyciA9IGlvY3RsKGZkLCBTWU5DX0lPQ19GSUxF X0lORk8sIGluZm8pOwo+Pj4gCX0KPj4+Cj4+PiB2MjogZml4IGZlbmNlX2luZm8gbWVtb3J5IGxl YWsKPj4+Cj4+PiBTaWduZWQtb2ZmLWJ5OiBHdXN0YXZvIFBhZG92YW4gPGd1c3Rhdm8ucGFkb3Zh bkBjb2xsYWJvcmEuY28udWs+Cj4+PiAtLS0KPj4+ICBkcml2ZXJzL3N0YWdpbmcvYW5kcm9pZC9z eW5jLmMgICAgICB8IDUyICsrKysrKysrKysrKysrKysrKysrKysrKysrKysrLS0tLS0tLS0KPj4+ ICBkcml2ZXJzL3N0YWdpbmcvYW5kcm9pZC91YXBpL3N5bmMuaCB8ICA5ICsrKy0tLS0KPj4+ICAy IGZpbGVzIGNoYW5nZWQsIDQ1IGluc2VydGlvbnMoKyksIDE2IGRlbGV0aW9ucygtKQo+Pj4KPj4+ IGRpZmYgLS1naXQgYS9kcml2ZXJzL3N0YWdpbmcvYW5kcm9pZC9zeW5jLmMgYi9kcml2ZXJzL3N0 YWdpbmcvYW5kcm9pZC9zeW5jLmMKPj4+IGluZGV4IGRjNWYzODIuLjIzNzlmMjMgMTAwNjQ0Cj4+ PiAtLS0gYS9kcml2ZXJzL3N0YWdpbmcvYW5kcm9pZC9zeW5jLmMKPj4+ICsrKyBiL2RyaXZlcnMv c3RhZ2luZy9hbmRyb2lkL3N5bmMuYwo+Pj4gQEAgLTUwMiwyMSArNTAyLDIyIEBAIHN0YXRpYyBp bnQgc3luY19maWxsX2ZlbmNlX2luZm8oc3RydWN0IGZlbmNlICpmZW5jZSwgdm9pZCAqZGF0YSwg aW50IHNpemUpCj4+PiAgc3RhdGljIGxvbmcgc3luY19maWxlX2lvY3RsX2ZlbmNlX2luZm8oc3Ry dWN0IHN5bmNfZmlsZSAqc3luY19maWxlLAo+Pj4gIAkJCQkJdW5zaWduZWQgbG9uZyBhcmcpCj4+ PiAgewo+Pj4gLQlzdHJ1Y3Qgc3luY19maWxlX2luZm8gKmluZm87Cj4+PiArCXN0cnVjdCBzeW5j X2ZpbGVfaW5mbyBpbiwgKmluZm87Cj4+PiArCXN0cnVjdCBzeW5jX2ZlbmNlX2luZm8gKmZlbmNl X2luZm8gPSBOVUxMOwo+Pj4gIAlfX3UzMiBzaXplOwo+Pj4gIAlfX3UzMiBsZW4gPSAwOwo+PiA9 IDAgdW5uZWVkZWQuCj4+PiAgCWludCByZXQsIGk7Cj4+PiAgCj4+PiAtCWlmIChjb3B5X2Zyb21f dXNlcigmc2l6ZSwgKHZvaWQgX191c2VyICopYXJnLCBzaXplb2Yoc2l6ZSkpKQo+Pj4gKwlpZiAo Y29weV9mcm9tX3VzZXIoJmluLCAodm9pZCBfX3VzZXIgKilhcmcsIHNpemVvZigqaW5mbykpKQo+ Pj4gIAkJcmV0dXJuIC1FRkFVTFQ7Cj4+PiAgCj4+PiAtCWlmIChzaXplIDwgc2l6ZW9mKHN0cnVj dCBzeW5jX2ZpbGVfaW5mbykpCj4+PiAtCQlyZXR1cm4gLUVJTlZBTDsKPj4+ICsJaWYgKGluLnN0 YXR1cyB8fCBzdHJjbXAoaW4ubmFtZSwgIlwwIikpCj4+PiArCQlyZXR1cm4gLUVGQVVMVDsKPj4g VGhlc2UgbWVtYmVycyBhbHdheXMgZ2V0IHdyaXR0ZW4gYnkgdGhlIGZlbmNlIGlvY3RsLCBJJ20g bm90IHN1cmUgaXQgYWRkcyB2YWx1ZSB0byBoYXZlIHRoZW0gZXhwbGljaXRseSB6ZXJvJ2Qgb3V0 IGJ5IHVzZXJzcGFjZS4KPj4+IC0JaWYgKHNpemUgPiA0MDk2KQo+Pj4gLQkJc2l6ZSA9IDQwOTY7 Cj4+PiArCWlmIChpbi5udW1fZmVuY2VzICYmICFpbi5zeW5jX2ZlbmNlX2luZm8pCj4+PiArCQly ZXR1cm4gLUVGQVVMVDsKPj4gVGhpcyBjaGVjayBpcyB1bm5lZWRlZCwgaXQgd2lsbCBoYXBwZW4g aW4gdGhlIGNvcHlfdG9fdXNlciBjYWxsIGFueXdheS4KPj4+IC0JaW5mbyA9IGt6YWxsb2Moc2l6 ZSwgR0ZQX0tFUk5FTCk7Cj4+PiArCWluZm8gPSBremFsbG9jKHNpemVvZigqaW5mbyksIEdGUF9L RVJORUwpOwo+Pj4gIAlpZiAoIWluZm8pCj4+PiAgCQlyZXR1cm4gLUVOT01FTTsKPj4+ICAKPj4+ IEBAIC01MjUsMTQgKzUyNiwzMyBAQCBzdGF0aWMgbG9uZyBzeW5jX2ZpbGVfaW9jdGxfZmVuY2Vf aW5mbyhzdHJ1Y3Qgc3luY19maWxlICpzeW5jX2ZpbGUsCj4+PiAgCWlmIChpbmZvLT5zdGF0dXMg Pj0gMCkKPj4+ICAJCWluZm8tPnN0YXR1cyA9ICFpbmZvLT5zdGF0dXM7Cj4+PiAgCj4+PiAtCWlu Zm8tPm51bV9mZW5jZXMgPSBzeW5jX2ZpbGUtPm51bV9mZW5jZXM7Cj4+PiArCS8qCj4+PiArCSAq IFBhc3NpbmcgbnVtX2ZlbmNlcyA9IDAgbWVhbnMgdGhhdCB1c2Vyc3BhY2Ugd2FudCB0byBrbm93 IGhvdwo+Pj4gKwkgKiBtYW55IGZlbmNlcyBhcmUgaW4gdGhlIHN5bmNfZmlsZSB0byBiZSBhYmxl IHRvIGFsbG9jYXRlIGEgYnVmZmVyIHRvCj4+PiArCSAqIGZpdCBhbGwgc3luY19mZW5jZV9pbmZv cyBhbmQgY2FsbCB0aGUgaW9jdGwgYWdhaW4gd2l0aCB0aGUgYnVmZmVyCj4+PiArCSAqIGFzc2ln bmVkIHRvIGluZm8tPnN5bmNfZmVuY2VfaW5mby4gVGhlIHNlY29uZCBjYWxsIHBhc3MgdGhlCj4+ PiArCSAqIG51bV9mZW5jZXMgdmFsdWUgcmVjZWl2ZWQgaW4gdGhlIGZpcnN0IGNhbGwuCj4+PiAr CSAqLwo+Pj4gKwlpZiAoIWluLm51bV9mZW5jZXMpCj4+PiArCQlnb3RvIG5vX2ZlbmNlczsKPj4+ ICsKPj4+ICsJc2l6ZSA9IHN5bmNfZmlsZS0+bnVtX2ZlbmNlcyAqIHNpemVvZigqZmVuY2VfaW5m byk7Cj4+PiArCWlmIChpbi5sZW4gIT0gc2l6ZSkgewo+Pj4gKwkJcmV0ID0gLUVGQVVMVDsKPj4+ ICsJCWdvdG8gb3V0Owo+Pj4gKwl9Cj4+IE1heWJlIGNoZWNrIGZvciBpbi5sZW4gPCBzaXplLCBh bmQgc2V0IHNldCB0byBzaXplPwo+Pgo+Pgo+Pj4gLQlsZW4gPSBzaXplb2Yoc3RydWN0IHN5bmNf ZmlsZV9pbmZvKTsKPj4+ICsJZmVuY2VfaW5mbyA9IGt6YWxsb2Moc2l6ZSwgR0ZQX0tFUk5FTCk7 Cj4+PiArCWlmICghZmVuY2VfaW5mbykgewo+Pj4gKwkJcmV0ID0gLUVOT01FTTsKPj4+ICsJCWdv dG8gb3V0Owo+Pj4gKwl9Cj4+PiAgCj4+PiAgCWZvciAoaSA9IDA7IGkgPCBzeW5jX2ZpbGUtPm51 bV9mZW5jZXM7ICsraSkgewo+Pj4gIAkJc3RydWN0IGZlbmNlICpmZW5jZSA9IHN5bmNfZmlsZS0+ Y2JzW2ldLmZlbmNlOwo+Pj4gIAo+Pj4gLQkJcmV0ID0gc3luY19maWxsX2ZlbmNlX2luZm8oZmVu Y2UsICh1OCAqKWluZm8gKyBsZW4sIHNpemUgLSBsZW4pOwo+Pj4gKwkJcmV0ID0gc3luY19maWxs X2ZlbmNlX2luZm8oZmVuY2UsICh1OCAqKWZlbmNlX2luZm8gKyBsZW4sCj4+PiArCQkJCQkgICBz aXplIC0gbGVuKTsKPj4+ICAKPj4+ICAJCWlmIChyZXQgPCAwKQo+Pj4gIAkJCWdvdG8gb3V0Owo+ Pj4gQEAgLTU0MCwxNCArNTYwLDI0IEBAIHN0YXRpYyBsb25nIHN5bmNfZmlsZV9pb2N0bF9mZW5j ZV9pbmZvKHN0cnVjdCBzeW5jX2ZpbGUgKnN5bmNfZmlsZSwKPj4+ICAJCWxlbiArPSByZXQ7Cj4+ PiAgCX0KPj4+ICAKPj4+ICsJaWYgKGNvcHlfdG9fdXNlcigodm9pZCBfX3VzZXIgKilpbi5zeW5j X2ZlbmNlX2luZm8sIGZlbmNlX2luZm8sIHNpemUpKSB7Cj4+PiArCQlyZXQgPSAtRUZBVUxUOwo+ Pj4gKwkJZ290byBvdXQ7Cj4+PiArCX0KPj4+ICsKPj4+ICAJaW5mby0+bGVuID0gbGVuOwo+Pj4g KwlpbmZvLT5zeW5jX2ZlbmNlX2luZm8gPSAoX191NjQpIGluLnN5bmNfZmVuY2VfaW5mbzsKPj4+ ICsKPj4+ICtub19mZW5jZXM6Cj4+PiArCWluZm8tPm51bV9mZW5jZXMgPSBzeW5jX2ZpbGUtPm51 bV9mZW5jZXM7Cj4+PiAgCj4+PiAtCWlmIChjb3B5X3RvX3VzZXIoKHZvaWQgX191c2VyICopYXJn LCBpbmZvLCBsZW4pKQo+Pj4gKwlpZiAoY29weV90b191c2VyKCh2b2lkIF9fdXNlciAqKWFyZywg aW5mbywgc2l6ZW9mKCppbmZvKSkpCj4+PiAgCQlyZXQgPSAtRUZBVUxUOwo+Pj4gIAllbHNlCj4+ PiAgCQlyZXQgPSAwOwo+Pj4gIAo+Pj4gIG91dDoKPj4+ICsJa2ZyZWUoZmVuY2VfaW5mbyk7Cj4+ PiAgCWtmcmVlKGluZm8pOwo+Pj4gIAo+Pj4gIAlyZXR1cm4gcmV0Owo+Pj4gZGlmZiAtLWdpdCBh L2RyaXZlcnMvc3RhZ2luZy9hbmRyb2lkL3VhcGkvc3luYy5oIGIvZHJpdmVycy9zdGFnaW5nL2Fu ZHJvaWQvdWFwaS9zeW5jLmgKPj4+IGluZGV4IGYwYjQxY2UuLjlhYWQ2MjMgMTAwNjQ0Cj4+PiAt LS0gYS9kcml2ZXJzL3N0YWdpbmcvYW5kcm9pZC91YXBpL3N5bmMuaAo+Pj4gKysrIGIvZHJpdmVy cy9zdGFnaW5nL2FuZHJvaWQvdWFwaS9zeW5jLmgKPj4+IEBAIC00MiwyMSArNDIsMjAgQEAgc3Ry dWN0IHN5bmNfZmVuY2VfaW5mbyB7Cj4+PiAgCj4+PiAgLyoqCj4+PiAgICogc3RydWN0IHN5bmNf ZmlsZV9pbmZvIC0gZGF0YSByZXR1cm5lZCBmcm9tIGZlbmNlIGluZm8gaW9jdGwKPj4+IC0gKiBA bGVuOglpb2N0bCBjYWxsZXIgd3JpdGVzIHRoZSBzaXplIG9mIHRoZSBidWZmZXIgaXRzIHBhc3Np bmcgaW4uCj4+PiAtICoJCWlvY3RsIHJldHVybnMgbGVuZ3RoIG9mIHN5bmNfZmlsZV9pbmZvIHJl dHVybmVkIHRvCj4+PiAtICoJCXVzZXJzcGFjZSBpbmNsdWRpbmcgcHRfaW5mby4KPj4+ICAgKiBA bmFtZToJbmFtZSBvZiBmZW5jZQo+Pj4gICAqIEBzdGF0dXM6CXN0YXR1cyBvZiBmZW5jZS4gMTog c2lnbmFsZWQgMDphY3RpdmUgPDA6ZXJyb3IKPj4+ICAgKiBAbnVtX2ZlbmNlcwludW1iZXIgb2Yg ZmVuY2VzIGluIHRoZSBzeW5jX2ZpbGUKPj4+ICsgKiBAbGVuOglpb2N0bCBjYWxsZXIgd3JpdGVz IHRoZSBzaXplIG9mIHRoZSBidWZmZXIgaXRzIHBhc3NpbmcgaW4uCj4+PiArICoJCWlvY3RsIHJl dHVybnMgbGVuZ3RoIG9mIGFsbCBmZW5jZV9pbmZvcyBzdW1tZWQuCj4+PiAgICogQHN5bmNfZmVu Y2VfaW5mbzogYXJyYXkgb2Ygc3luY19mZW5jZV9pbmZvIGZvciBldmVyeSBmZW5jZSBpbiB0aGUg c3luY19maWxlCj4+PiAgICovCj4+PiAgc3RydWN0IHN5bmNfZmlsZV9pbmZvIHsKPj4+IC0JX191 MzIJbGVuOwo+Pj4gIAljaGFyCW5hbWVbMzJdOwo+Pj4gIAlfX3MzMglzdGF0dXM7Cj4+PiAgCV9f dTMyCW51bV9mZW5jZXM7Cj4+PiArCV9fdTMyCWxlbjsKPj4+ICAKPj4+IC0JX191OAlzeW5jX2Zl bmNlX2luZm9bMF07Cj4+PiArCV9fdTY0CXN5bmNfZmVuY2VfaW5mbzsKPj4+ICB9Owo+Pj4gIAo+ Pj4gICNkZWZpbmUgU1lOQ19JT0NfTUFHSUMJCSc+Jwo+PiBOb3Qgc3VyZSBpZiBsZW4gYWRkcyBh bnl0aGluZyBoZXJlLCB1c2Vyc3BhY2Uga25vd3MgdG8gYWxsb2NhdGUgbnVtX2ZlbmNlcyAqIHNp emVvZihzdHJ1Y3Qgc3luY19mZW5jZV9pbmZvKTsKPiBJIHRoaW5rIG9mIGxlbiBiZWluZyB1c2Vm dWwgaWYgd2UgZGVjaWRlIHRvIGV4dGVuZCBzdHJ1Y3Qgc3luY19mZW5jZV9pbmZvIGluCj4gdGhl IGZ1dHVyZSwgc28gd2UgbWF5IHVzZSBsZW4gdG8gaGVscCBkaXNjb3ZlciB0aGUgc2l6ZSBvZiBl YWNoCj4gc3luY19mZW5jZV9pbmZvLiBXaGF0IGRvIHlvdSB0aGluaz8KPgpJIGRvbid0IHRoaW5r IHlvdSBjb3VsZCBleHRlbmQgaXQgYXJiaXRyYXJpbHksIHlvdSBjb3VsZCBtYWtlIHVzZXJzcGFj ZSBwYXNzIGEgZmxhZyB0byBpbmRpY2F0ZSB0aGUgc3RydWN0IGlzIGV4dGVuZGVkLCBzbyBrZXJu ZWwgc3BhY2UgY2FuIGNob29zZQp3aGV0aGVyIHRvIHVzZSB0aGUgYmlnZ2VyIHNpemUgc3RydWN0 IG9yIG5vdC4KCnNvbWV0aGluZyBsaWtlIHN5bmNfZmlsZV9pbmZvLmZsYWdzID0gRkVOQ0VfSU5G T19WMjsgLS0ga2VybmVsIGNhbiByZWplY3Qgd2l0aCAtRUlOVkFMIGlmIHVuc3VwcG9ydGVkLCBv ciBmaWxsIGluIGEgdjIgc3RydWN0LgoKfk1hYXJ0ZW4KX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxA bGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxt YW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752600AbcCAIgG (ORCPT ); Tue, 1 Mar 2016 03:36:06 -0500 Received: from mga02.intel.com ([134.134.136.20]:34735 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751577AbcCAIgD (ORCPT ); Tue, 1 Mar 2016 03:36:03 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,522,1449561600"; d="scan'208";a="914227798" Subject: Re: [PATCH] staging/android: refactor SYNC_IOC_FILE_INFO 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: <1456511507-2534-4-git-send-email-gustavo@padovan.org> <1456520410-28195-1-git-send-email-gustavo@padovan.org> <56D400CD.6070804@linux.intel.com> <20160229220839.GD2479@joana> From: Maarten Lankhorst Message-ID: <56D5546E.4010701@linux.intel.com> Date: Tue, 1 Mar 2016 09:35:58 +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: <20160229220839.GD2479@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 29-02-16 om 23:08 schreef Gustavo Padovan: > Hi Maarten, > > 2016-02-29 Maarten Lankhorst : > >> Op 26-02-16 om 22:00 schreef Gustavo Padovan: >>> From: Gustavo Padovan >>> >>> Change SYNC_IOC_FILE_INFO behaviour to avoid future API breaks and >>> optimize buffer allocation. In the new approach the ioctl needs to be called >>> twice to retrieve the array of fence_infos pointed by info->sync_fence_info. >>> >>> The first call should pass num_fences = 0, the kernel will then fill >>> info->num_fences. Userspace receives back the number of fences and >>> allocates a buffer size num_fences * sizeof(struct sync_fence_info) on >>> info->sync_fence_info. >>> >>> It then call the ioctl again passing num_fences received in info->num_fences. >>> The kernel checks if info->num_fences > 0 and if yes it fill >>> info->sync_fence_info with an array containing all fence_infos. >>> >>> info->len now represents the length of the buffer sync_fence_info points >>> to. Also, info->sync_fence_info was converted to __u64 pointer. >>> >>> An example userspace code would be: >>> >>> struct sync_file_info *info; >>> int err, size, num_fences; >>> >>> info = malloc(sizeof(*info)); >>> >>> memset(info, 0, sizeof(*info)); >>> >>> err = ioctl(fd, SYNC_IOC_FILE_INFO, info); >>> num_fences = info->num_fences; >>> >>> if (num_fences) { >>> memset(info, 0, sizeof(*info)); >> Would this memset still be needed if we didn't check for nulls in info->status and info->name ? >> >> Seems to me that it could be skipped in that case. > Yes, I agree. > >>> size = sizeof(struct sync_fence_info) * num_fences; >>> info->len = size; >>> info->num_fences = num_fences; >>> info->sync_fence_info = (uint64_t) calloc(num_fences, >>> sizeof(struct sync_fence_info)); >>> >>> err = ioctl(fd, SYNC_IOC_FILE_INFO, info); >>> } >>> >>> v2: fix fence_info memory leak >>> >>> Signed-off-by: Gustavo Padovan >>> --- >>> drivers/staging/android/sync.c | 52 +++++++++++++++++++++++++++++-------- >>> drivers/staging/android/uapi/sync.h | 9 +++---- >>> 2 files changed, 45 insertions(+), 16 deletions(-) >>> >>> diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c >>> index dc5f382..2379f23 100644 >>> --- a/drivers/staging/android/sync.c >>> +++ b/drivers/staging/android/sync.c >>> @@ -502,21 +502,22 @@ static int sync_fill_fence_info(struct fence *fence, void *data, int size) >>> static long sync_file_ioctl_fence_info(struct sync_file *sync_file, >>> unsigned long arg) >>> { >>> - struct sync_file_info *info; >>> + struct sync_file_info in, *info; >>> + struct sync_fence_info *fence_info = NULL; >>> __u32 size; >>> __u32 len = 0; >> = 0 unneeded. >>> int ret, i; >>> >>> - if (copy_from_user(&size, (void __user *)arg, sizeof(size))) >>> + if (copy_from_user(&in, (void __user *)arg, sizeof(*info))) >>> return -EFAULT; >>> >>> - if (size < sizeof(struct sync_file_info)) >>> - return -EINVAL; >>> + if (in.status || strcmp(in.name, "\0")) >>> + return -EFAULT; >> These members always get written by the fence ioctl, I'm not sure it adds value to have them explicitly zero'd out by userspace. >>> - if (size > 4096) >>> - size = 4096; >>> + if (in.num_fences && !in.sync_fence_info) >>> + return -EFAULT; >> This check is unneeded, it will happen in the copy_to_user call anyway. >>> - info = kzalloc(size, GFP_KERNEL); >>> + info = kzalloc(sizeof(*info), GFP_KERNEL); >>> if (!info) >>> return -ENOMEM; >>> >>> @@ -525,14 +526,33 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, >>> if (info->status >= 0) >>> info->status = !info->status; >>> >>> - info->num_fences = sync_file->num_fences; >>> + /* >>> + * Passing num_fences = 0 means that userspace want to know how >>> + * many fences are in the sync_file to be able to allocate a buffer to >>> + * fit all sync_fence_infos and call the ioctl again with the buffer >>> + * assigned to info->sync_fence_info. The second call pass the >>> + * num_fences value received in the first call. >>> + */ >>> + if (!in.num_fences) >>> + goto no_fences; >>> + >>> + size = sync_file->num_fences * sizeof(*fence_info); >>> + if (in.len != size) { >>> + ret = -EFAULT; >>> + goto out; >>> + } >> Maybe check for in.len < size, and set set to size? >> >> >>> - len = sizeof(struct sync_file_info); >>> + fence_info = kzalloc(size, GFP_KERNEL); >>> + if (!fence_info) { >>> + ret = -ENOMEM; >>> + goto out; >>> + } >>> >>> for (i = 0; i < sync_file->num_fences; ++i) { >>> struct fence *fence = sync_file->cbs[i].fence; >>> >>> - ret = sync_fill_fence_info(fence, (u8 *)info + len, size - len); >>> + ret = sync_fill_fence_info(fence, (u8 *)fence_info + len, >>> + size - len); >>> >>> if (ret < 0) >>> goto out; >>> @@ -540,14 +560,24 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, >>> len += ret; >>> } >>> >>> + if (copy_to_user((void __user *)in.sync_fence_info, fence_info, size)) { >>> + ret = -EFAULT; >>> + goto out; >>> + } >>> + >>> info->len = len; >>> + info->sync_fence_info = (__u64) in.sync_fence_info; >>> + >>> +no_fences: >>> + info->num_fences = sync_file->num_fences; >>> >>> - if (copy_to_user((void __user *)arg, info, len)) >>> + if (copy_to_user((void __user *)arg, info, sizeof(*info))) >>> ret = -EFAULT; >>> else >>> ret = 0; >>> >>> out: >>> + kfree(fence_info); >>> kfree(info); >>> >>> return ret; >>> diff --git a/drivers/staging/android/uapi/sync.h b/drivers/staging/android/uapi/sync.h >>> index f0b41ce..9aad623 100644 >>> --- a/drivers/staging/android/uapi/sync.h >>> +++ b/drivers/staging/android/uapi/sync.h >>> @@ -42,21 +42,20 @@ struct sync_fence_info { >>> >>> /** >>> * struct sync_file_info - data returned from fence info ioctl >>> - * @len: ioctl caller writes the size of the buffer its passing in. >>> - * ioctl returns length of sync_file_info returned to >>> - * userspace including pt_info. >>> * @name: name of fence >>> * @status: status of fence. 1: signaled 0:active <0:error >>> * @num_fences number of fences in the sync_file >>> + * @len: ioctl caller writes the size of the buffer its passing in. >>> + * ioctl returns length of all fence_infos summed. >>> * @sync_fence_info: array of sync_fence_info for every fence in the sync_file >>> */ >>> struct sync_file_info { >>> - __u32 len; >>> char name[32]; >>> __s32 status; >>> __u32 num_fences; >>> + __u32 len; >>> >>> - __u8 sync_fence_info[0]; >>> + __u64 sync_fence_info; >>> }; >>> >>> #define SYNC_IOC_MAGIC '>' >> Not sure if len adds anything here, userspace knows to allocate num_fences * sizeof(struct sync_fence_info); > I think of len being useful if we decide to extend struct sync_fence_info in > the future, so we may use len to help discover the size of each > sync_fence_info. What do you think? > I don't think you could extend it arbitrarily, you could make userspace pass a flag to indicate the struct is extended, so kernel space can choose whether to use the bigger size struct or not. something like sync_file_info.flags = FENCE_INFO_V2; -- kernel can reject with -EINVAL if unsupported, or fill in a v2 struct. ~Maarten