From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756063AbeDQQQB (ORCPT ); Tue, 17 Apr 2018 12:16:01 -0400 Received: from bombadil.infradead.org ([198.137.202.133]:47168 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755445AbeDQQQA (ORCPT ); Tue, 17 Apr 2018 12:16:00 -0400 Date: Tue, 17 Apr 2018 09:15:57 -0700 From: Matthew Wilcox To: Souptick Joarder Cc: Jani Nikula , joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com, airlied@linux.ie, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] gpu: drm: i915: Change return type to vm_fault_t Message-ID: <20180417161557.GA3603@bombadil.infradead.org> References: <20180417151127.GA31655@jordon-HP-15-Notebook-PC> <87h8o9g8be.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.2 (2017-12-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Apr 17, 2018 at 09:14:32PM +0530, Souptick Joarder wrote: > Not exactly. The plan for these patches is to introduce new vm_fault_t type > in vm_operations_struct fault handlers. It's now available in 4.17-rc1. We will > push all the required drivers/filesystem changes through different maintainers > to linus tree. Once everything is converted into vm_fault_t type then Changing > it from a signed to an unsigned int causes GCC to warn about an assignment > from an incompatible type -- int foo(void) is incompatible with > unsigned int foo(void). > > Please refer 1c8f422059ae ("mm: change return type to vm_fault_t") in 4.17-rc1. I think this patch would be clearer if you did - int ret; + int err; + vm_fault_t ret; Then it would be clearer to the maintainer that you're splitting apart the VM_FAULT and errno codes. Sorry for not catching this during initial review. > On Tue, Apr 17, 2018 at 8:59 PM, Jani Nikula > wrote: > > On Tue, 17 Apr 2018, Souptick Joarder wrote: > >> Use new return type vm_fault_t for fault handler. For > >> now, this is just documenting that the function returns > >> a VM_FAULT value rather than an errno. Once all instances > >> are converted, vm_fault_t will become a distinct type. > >> > >> Reference id -> 1c8f422059ae ("mm: change return type to > >> vm_fault_t") > >> > >> Signed-off-by: Souptick Joarder > >> --- > >> drivers/gpu/drm/i915/i915_drv.h | 3 ++- > >> drivers/gpu/drm/i915/i915_gem.c | 15 ++++++++------- > >> 2 files changed, 10 insertions(+), 8 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > >> index a42deeb..95b0d50 100644 > >> --- a/drivers/gpu/drm/i915/i915_drv.h > >> +++ b/drivers/gpu/drm/i915/i915_drv.h > >> @@ -51,6 +51,7 @@ > >> #include > >> #include > >> #include > >> +#include > >> > >> #include "i915_params.h" > >> #include "i915_reg.h" > >> @@ -3363,7 +3364,7 @@ int i915_gem_wait_for_idle(struct drm_i915_private *dev_priv, > >> unsigned int flags); > >> int __must_check i915_gem_suspend(struct drm_i915_private *dev_priv); > >> void i915_gem_resume(struct drm_i915_private *dev_priv); > >> -int i915_gem_fault(struct vm_fault *vmf); > >> +vm_fault_t i915_gem_fault(struct vm_fault *vmf); > >> int i915_gem_object_wait(struct drm_i915_gem_object *obj, > >> unsigned int flags, > >> long timeout, > >> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > >> index dd89abd..bdac690 100644 > >> --- a/drivers/gpu/drm/i915/i915_gem.c > >> +++ b/drivers/gpu/drm/i915/i915_gem.c > >> @@ -1882,7 +1882,7 @@ int i915_gem_mmap_gtt_version(void) > >> * The current feature set supported by i915_gem_fault() and thus GTT mmaps > >> * is exposed via I915_PARAM_MMAP_GTT_VERSION (see i915_gem_mmap_gtt_version). > >> */ > >> -int i915_gem_fault(struct vm_fault *vmf) > >> +vm_fault_t i915_gem_fault(struct vm_fault *vmf) > >> { > >> #define MIN_CHUNK_PAGES ((1 << 20) >> PAGE_SHIFT) /* 1 MiB */ > >> struct vm_area_struct *area = vmf->vma; > >> @@ -1895,6 +1895,7 @@ int i915_gem_fault(struct vm_fault *vmf) > >> pgoff_t page_offset; > >> unsigned int flags; > >> int ret; > >> + vm_fault_t retval; > > > > What's the point of changing the name? An unnecessary change. > > > > BR, > > Jani. > > > >> > >> /* We don't use vmf->pgoff since that has the fake offset */ > >> page_offset = (vmf->address - area->vm_start) >> PAGE_SHIFT; > >> @@ -2000,7 +2001,7 @@ int i915_gem_fault(struct vm_fault *vmf) > >> * and so needs to be reported. > >> */ > >> if (!i915_terminally_wedged(&dev_priv->gpu_error)) { > >> - ret = VM_FAULT_SIGBUS; > >> + retval = VM_FAULT_SIGBUS; > >> break; > >> } > >> case -EAGAIN: > >> @@ -2017,21 +2018,21 @@ int i915_gem_fault(struct vm_fault *vmf) > >> * EBUSY is ok: this just means that another thread > >> * already did the job. > >> */ > >> - ret = VM_FAULT_NOPAGE; > >> + retval = VM_FAULT_NOPAGE; > >> break; > >> case -ENOMEM: > >> - ret = VM_FAULT_OOM; > >> + retval = VM_FAULT_OOM; > >> break; > >> case -ENOSPC: > >> case -EFAULT: > >> - ret = VM_FAULT_SIGBUS; > >> + retval = VM_FAULT_SIGBUS; > >> break; > >> default: > >> WARN_ONCE(ret, "unhandled error in i915_gem_fault: %i\n", ret); > >> - ret = VM_FAULT_SIGBUS; > >> + retval = VM_FAULT_SIGBUS; > >> break; > >> } > >> - return ret; > >> + return retval; > >> } > >> > >> static void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj) > >> -- > >> 1.9.1 > >> > > > > -- > > Jani Nikula, Intel Open Source Technology Center From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matthew Wilcox Subject: Re: [PATCH] gpu: drm: i915: Change return type to vm_fault_t Date: Tue, 17 Apr 2018 09:15:57 -0700 Message-ID: <20180417161557.GA3603@bombadil.infradead.org> References: <20180417151127.GA31655@jordon-HP-15-Notebook-PC> <87h8o9g8be.fsf@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Souptick Joarder Cc: airlied@linux.ie, intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, rodrigo.vivi@intel.com List-Id: dri-devel@lists.freedesktop.org T24gVHVlLCBBcHIgMTcsIDIwMTggYXQgMDk6MTQ6MzJQTSArMDUzMCwgU291cHRpY2sgSm9hcmRl ciB3cm90ZToKPiBOb3QgZXhhY3RseS4gVGhlIHBsYW4gZm9yIHRoZXNlIHBhdGNoZXMgaXMgdG8g aW50cm9kdWNlIG5ldyB2bV9mYXVsdF90IHR5cGUKPiBpbiB2bV9vcGVyYXRpb25zX3N0cnVjdCBm YXVsdCBoYW5kbGVycy4gSXQncyBub3cgYXZhaWxhYmxlIGluIDQuMTctcmMxLiBXZSB3aWxsCj4g cHVzaCBhbGwgdGhlIHJlcXVpcmVkIGRyaXZlcnMvZmlsZXN5c3RlbSBjaGFuZ2VzIHRocm91Z2gg ZGlmZmVyZW50IG1haW50YWluZXJzCj4gdG8gbGludXMgdHJlZS4gT25jZSBldmVyeXRoaW5nIGlz IGNvbnZlcnRlZCBpbnRvIHZtX2ZhdWx0X3QgdHlwZSB0aGVuIENoYW5naW5nCj4gaXQgZnJvbSBh IHNpZ25lZCB0byBhbiB1bnNpZ25lZCBpbnQgY2F1c2VzIEdDQyB0byB3YXJuIGFib3V0IGFuIGFz c2lnbm1lbnQKPiBmcm9tIGFuIGluY29tcGF0aWJsZSB0eXBlIC0tIGludCBmb28odm9pZCkgaXMg aW5jb21wYXRpYmxlIHdpdGgKPiB1bnNpZ25lZCBpbnQgZm9vKHZvaWQpLgo+IAo+IFBsZWFzZSBy ZWZlciAxYzhmNDIyMDU5YWUgKCJtbTogY2hhbmdlIHJldHVybiB0eXBlIHRvIHZtX2ZhdWx0X3Qi KSBpbiA0LjE3LXJjMS4KCkkgdGhpbmsgdGhpcyBwYXRjaCB3b3VsZCBiZSBjbGVhcmVyIGlmIHlv dSBkaWQKCi0JaW50IHJldDsKKwlpbnQgZXJyOworCXZtX2ZhdWx0X3QgcmV0OwoKVGhlbiBpdCB3 b3VsZCBiZSBjbGVhcmVyIHRvIHRoZSBtYWludGFpbmVyIHRoYXQgeW91J3JlIHNwbGl0dGluZyBh cGFydCB0aGUKVk1fRkFVTFQgYW5kIGVycm5vIGNvZGVzLgoKU29ycnkgZm9yIG5vdCBjYXRjaGlu ZyB0aGlzIGR1cmluZyBpbml0aWFsIHJldmlldy4KCj4gT24gVHVlLCBBcHIgMTcsIDIwMTggYXQg ODo1OSBQTSwgSmFuaSBOaWt1bGEKPiA8amFuaS5uaWt1bGFAbGludXguaW50ZWwuY29tPiB3cm90 ZToKPiA+IE9uIFR1ZSwgMTcgQXByIDIwMTgsIFNvdXB0aWNrIEpvYXJkZXIgPGpyZHIubGludXhA Z21haWwuY29tPiB3cm90ZToKPiA+PiBVc2UgbmV3IHJldHVybiB0eXBlIHZtX2ZhdWx0X3QgZm9y IGZhdWx0IGhhbmRsZXIuIEZvcgo+ID4+IG5vdywgdGhpcyBpcyBqdXN0IGRvY3VtZW50aW5nIHRo YXQgdGhlIGZ1bmN0aW9uIHJldHVybnMKPiA+PiBhIFZNX0ZBVUxUIHZhbHVlIHJhdGhlciB0aGFu IGFuIGVycm5vLiBPbmNlIGFsbCBpbnN0YW5jZXMKPiA+PiBhcmUgY29udmVydGVkLCB2bV9mYXVs dF90IHdpbGwgYmVjb21lIGEgZGlzdGluY3QgdHlwZS4KPiA+Pgo+ID4+IFJlZmVyZW5jZSBpZCAt PiAxYzhmNDIyMDU5YWUgKCJtbTogY2hhbmdlIHJldHVybiB0eXBlIHRvCj4gPj4gdm1fZmF1bHRf dCIpCj4gPj4KPiA+PiBTaWduZWQtb2ZmLWJ5OiBTb3VwdGljayBKb2FyZGVyIDxqcmRyLmxpbnV4 QGdtYWlsLmNvbT4KPiA+PiAtLS0KPiA+PiAgZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9kcnYu aCB8ICAzICsrLQo+ID4+ICBkcml2ZXJzL2dwdS9kcm0vaTkxNS9pOTE1X2dlbS5jIHwgMTUgKysr KysrKystLS0tLS0tCj4gPj4gIDIgZmlsZXMgY2hhbmdlZCwgMTAgaW5zZXJ0aW9ucygrKSwgOCBk ZWxldGlvbnMoLSkKPiA+Pgo+ID4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vaTkxNS9p OTE1X2Rydi5oIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9kcnYuaAo+ID4+IGluZGV4IGE0 MmRlZWIuLjk1YjBkNTAgMTAwNjQ0Cj4gPj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkx NV9kcnYuaAo+ID4+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfZHJ2LmgKPiA+PiBA QCAtNTEsNiArNTEsNyBAQAo+ID4+ICAjaW5jbHVkZSA8ZHJtL2RybV9nZW0uaD4KPiA+PiAgI2lu Y2x1ZGUgPGRybS9kcm1fYXV0aC5oPgo+ID4+ICAjaW5jbHVkZSA8ZHJtL2RybV9jYWNoZS5oPgo+ ID4+ICsjaW5jbHVkZSA8bGludXgvbW1fdHlwZXMuaD4KPiA+Pgo+ID4+ICAjaW5jbHVkZSAiaTkx NV9wYXJhbXMuaCIKPiA+PiAgI2luY2x1ZGUgImk5MTVfcmVnLmgiCj4gPj4gQEAgLTMzNjMsNyAr MzM2NCw3IEBAIGludCBpOTE1X2dlbV93YWl0X2Zvcl9pZGxlKHN0cnVjdCBkcm1faTkxNV9wcml2 YXRlICpkZXZfcHJpdiwKPiA+PiAgICAgICAgICAgICAgICAgICAgICAgICAgdW5zaWduZWQgaW50 IGZsYWdzKTsKPiA+PiAgaW50IF9fbXVzdF9jaGVjayBpOTE1X2dlbV9zdXNwZW5kKHN0cnVjdCBk cm1faTkxNV9wcml2YXRlICpkZXZfcHJpdik7Cj4gPj4gIHZvaWQgaTkxNV9nZW1fcmVzdW1lKHN0 cnVjdCBkcm1faTkxNV9wcml2YXRlICpkZXZfcHJpdik7Cj4gPj4gLWludCBpOTE1X2dlbV9mYXVs dChzdHJ1Y3Qgdm1fZmF1bHQgKnZtZik7Cj4gPj4gK3ZtX2ZhdWx0X3QgaTkxNV9nZW1fZmF1bHQo c3RydWN0IHZtX2ZhdWx0ICp2bWYpOwo+ID4+ICBpbnQgaTkxNV9nZW1fb2JqZWN0X3dhaXQoc3Ry dWN0IGRybV9pOTE1X2dlbV9vYmplY3QgKm9iaiwKPiA+PiAgICAgICAgICAgICAgICAgICAgICAg IHVuc2lnbmVkIGludCBmbGFncywKPiA+PiAgICAgICAgICAgICAgICAgICAgICAgIGxvbmcgdGlt ZW91dCwKPiA+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9nZW0uYyBi L2RyaXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfZ2VtLmMKPiA+PiBpbmRleCBkZDg5YWJkLi5iZGFj NjkwIDEwMDY0NAo+ID4+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfZ2VtLmMKPiA+ PiArKysgYi9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pOTE1X2dlbS5jCj4gPj4gQEAgLTE4ODIsNyAr MTg4Miw3IEBAIGludCBpOTE1X2dlbV9tbWFwX2d0dF92ZXJzaW9uKHZvaWQpCj4gPj4gICAqIFRo ZSBjdXJyZW50IGZlYXR1cmUgc2V0IHN1cHBvcnRlZCBieSBpOTE1X2dlbV9mYXVsdCgpIGFuZCB0 aHVzIEdUVCBtbWFwcwo+ID4+ICAgKiBpcyBleHBvc2VkIHZpYSBJOTE1X1BBUkFNX01NQVBfR1RU X1ZFUlNJT04gKHNlZSBpOTE1X2dlbV9tbWFwX2d0dF92ZXJzaW9uKS4KPiA+PiAgICovCj4gPj4g LWludCBpOTE1X2dlbV9mYXVsdChzdHJ1Y3Qgdm1fZmF1bHQgKnZtZikKPiA+PiArdm1fZmF1bHRf dCBpOTE1X2dlbV9mYXVsdChzdHJ1Y3Qgdm1fZmF1bHQgKnZtZikKPiA+PiAgewo+ID4+ICAjZGVm aW5lIE1JTl9DSFVOS19QQUdFUyAoKDEgPDwgMjApID4+IFBBR0VfU0hJRlQpIC8qIDEgTWlCICov Cj4gPj4gICAgICAgc3RydWN0IHZtX2FyZWFfc3RydWN0ICphcmVhID0gdm1mLT52bWE7Cj4gPj4g QEAgLTE4OTUsNiArMTg5NSw3IEBAIGludCBpOTE1X2dlbV9mYXVsdChzdHJ1Y3Qgdm1fZmF1bHQg KnZtZikKPiA+PiAgICAgICBwZ29mZl90IHBhZ2Vfb2Zmc2V0Owo+ID4+ICAgICAgIHVuc2lnbmVk IGludCBmbGFnczsKPiA+PiAgICAgICBpbnQgcmV0Owo+ID4+ICsgICAgIHZtX2ZhdWx0X3QgcmV0 dmFsOwo+ID4KPiA+IFdoYXQncyB0aGUgcG9pbnQgb2YgY2hhbmdpbmcgdGhlIG5hbWU/IEFuIHVu bmVjZXNzYXJ5IGNoYW5nZS4KPiA+Cj4gPiBCUiwKPiA+IEphbmkuCj4gPgo+ID4+Cj4gPj4gICAg ICAgLyogV2UgZG9uJ3QgdXNlIHZtZi0+cGdvZmYgc2luY2UgdGhhdCBoYXMgdGhlIGZha2Ugb2Zm c2V0ICovCj4gPj4gICAgICAgcGFnZV9vZmZzZXQgPSAodm1mLT5hZGRyZXNzIC0gYXJlYS0+dm1f c3RhcnQpID4+IFBBR0VfU0hJRlQ7Cj4gPj4gQEAgLTIwMDAsNyArMjAwMSw3IEBAIGludCBpOTE1 X2dlbV9mYXVsdChzdHJ1Y3Qgdm1fZmF1bHQgKnZtZikKPiA+PiAgICAgICAgICAgICAgICAqIGFu ZCBzbyBuZWVkcyB0byBiZSByZXBvcnRlZC4KPiA+PiAgICAgICAgICAgICAgICAqLwo+ID4+ICAg ICAgICAgICAgICAgaWYgKCFpOTE1X3Rlcm1pbmFsbHlfd2VkZ2VkKCZkZXZfcHJpdi0+Z3B1X2Vy cm9yKSkgewo+ID4+IC0gICAgICAgICAgICAgICAgICAgICByZXQgPSBWTV9GQVVMVF9TSUdCVVM7 Cj4gPj4gKyAgICAgICAgICAgICAgICAgICAgIHJldHZhbCA9IFZNX0ZBVUxUX1NJR0JVUzsKPiA+ PiAgICAgICAgICAgICAgICAgICAgICAgYnJlYWs7Cj4gPj4gICAgICAgICAgICAgICB9Cj4gPj4g ICAgICAgY2FzZSAtRUFHQUlOOgo+ID4+IEBAIC0yMDE3LDIxICsyMDE4LDIxIEBAIGludCBpOTE1 X2dlbV9mYXVsdChzdHJ1Y3Qgdm1fZmF1bHQgKnZtZikKPiA+PiAgICAgICAgICAgICAgICAqIEVC VVNZIGlzIG9rOiB0aGlzIGp1c3QgbWVhbnMgdGhhdCBhbm90aGVyIHRocmVhZAo+ID4+ICAgICAg ICAgICAgICAgICogYWxyZWFkeSBkaWQgdGhlIGpvYi4KPiA+PiAgICAgICAgICAgICAgICAqLwo+ ID4+IC0gICAgICAgICAgICAgcmV0ID0gVk1fRkFVTFRfTk9QQUdFOwo+ID4+ICsgICAgICAgICAg ICAgcmV0dmFsID0gVk1fRkFVTFRfTk9QQUdFOwo+ID4+ICAgICAgICAgICAgICAgYnJlYWs7Cj4g Pj4gICAgICAgY2FzZSAtRU5PTUVNOgo+ID4+IC0gICAgICAgICAgICAgcmV0ID0gVk1fRkFVTFRf T09NOwo+ID4+ICsgICAgICAgICAgICAgcmV0dmFsID0gVk1fRkFVTFRfT09NOwo+ID4+ICAgICAg ICAgICAgICAgYnJlYWs7Cj4gPj4gICAgICAgY2FzZSAtRU5PU1BDOgo+ID4+ICAgICAgIGNhc2Ug LUVGQVVMVDoKPiA+PiAtICAgICAgICAgICAgIHJldCA9IFZNX0ZBVUxUX1NJR0JVUzsKPiA+PiAr ICAgICAgICAgICAgIHJldHZhbCA9IFZNX0ZBVUxUX1NJR0JVUzsKPiA+PiAgICAgICAgICAgICAg IGJyZWFrOwo+ID4+ICAgICAgIGRlZmF1bHQ6Cj4gPj4gICAgICAgICAgICAgICBXQVJOX09OQ0Uo cmV0LCAidW5oYW5kbGVkIGVycm9yIGluIGk5MTVfZ2VtX2ZhdWx0OiAlaVxuIiwgcmV0KTsKPiA+ PiAtICAgICAgICAgICAgIHJldCA9IFZNX0ZBVUxUX1NJR0JVUzsKPiA+PiArICAgICAgICAgICAg IHJldHZhbCA9IFZNX0ZBVUxUX1NJR0JVUzsKPiA+PiAgICAgICAgICAgICAgIGJyZWFrOwo+ID4+ ICAgICAgIH0KPiA+PiAtICAgICByZXR1cm4gcmV0Owo+ID4+ICsgICAgIHJldHVybiByZXR2YWw7 Cj4gPj4gIH0KPiA+Pgo+ID4+ICBzdGF0aWMgdm9pZCBfX2k5MTVfZ2VtX29iamVjdF9yZWxlYXNl X21tYXAoc3RydWN0IGRybV9pOTE1X2dlbV9vYmplY3QgKm9iaikKPiA+PiAtLQo+ID4+IDEuOS4x Cj4gPj4KPiA+Cj4gPiAtLQo+ID4gSmFuaSBOaWt1bGEsIEludGVsIE9wZW4gU291cmNlIFRlY2hu b2xvZ3kgQ2VudGVyCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fCkludGVsLWdmeCBtYWlsaW5nIGxpc3QKSW50ZWwtZ2Z4QGxpc3RzLmZyZWVkZXNrdG9wLm9y ZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2ludGVsLWdm eAo=