From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?Christian_K=c3=b6nig?= Subject: Re: [PATCH 1/3] locking/ww_mutex: cleanup lock->ctx usage in amdgpu Date: Mon, 19 Feb 2018 16:41:55 +0100 Message-ID: References: <20180215141944.4332-1-christian.koenig@amd.com> <20180219152421.GQ22199@phenom.ffwll.local> Reply-To: christian.koenig-5C7GfCeVMHo@public.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20180219152421.GQ22199-dv86pmgwkMBes7Z6vYuT8azUEOm+Xw19@public.gmane.org> Content-Language: en-US List-Id: Discussion list for AMD gfx List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org Sender: "amd-gfx" To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org QW0gMTkuMDIuMjAxOCB1bSAxNjoyNCBzY2hyaWViIERhbmllbCBWZXR0ZXI6Cj4gT24gVGh1LCBG ZWIgMTUsIDIwMTggYXQgMDM6MTk6NDJQTSArMDEwMCwgQ2hyaXN0aWFuIEvDtm5pZyB3cm90ZToK Pj4gYW1kZ3B1IG5lZWRzIHRvIHZlcmlmeSBpZiB1c2Vyc3BhY2Ugc2VuZHMgdXMgdmFsaWQgYWRk cmVzc2VzIGFuZCB0aGUgc2ltcGxlc3QKPj4gd2F5IG9mIGRvaW5nIHRoaXMgaXMgdG8gY2hlY2sg aWYgdGhlIGJ1ZmZlciBvYmplY3QgaXMgbG9ja2VkIHdpdGggdGhlIHRpY2tldAo+PiBvZiB0aGUg Y3VycmVudCBzdWJtaXNzaW9uLgo+Pgo+PiBDbGVhbiB1cCB0aGUgYWNjZXNzIHRvIHRoZSB3d19t dXRleCBpbnRlcm5hbHMgYnkgcHJvdmlkaW5nIGEgZnVuY3Rpb24KPj4gZm9yIHRoaXMgYW5kIGV4 dGVuZCB0aGUgY2hlY2sgdG8gdGhlIHRocmVhZCBvd25pbmcgdGhlIHVuZGVybHlpbmcgbXV0ZXgu Cj4+Cj4+IFNpZ25lZC1vZmYtYnk6IENocmlzdGlhbiBLw7ZuaWcgPGNocmlzdGlhbi5rb2VuaWdA YW1kLmNvbT4KPj4gLS0tCj4+ICAgZHJpdmVycy9ncHUvZHJtL2FtZC9hbWRncHUvYW1kZ3B1X2Nz LmMgfCAgMyArKy0KPj4gICBpbmNsdWRlL2xpbnV4L3d3X211dGV4LmggICAgICAgICAgICAgICB8 IDE3ICsrKysrKysrKysrKysrKysrCj4+ICAgMiBmaWxlcyBjaGFuZ2VkLCAxOSBpbnNlcnRpb25z KCspLCAxIGRlbGV0aW9uKC0pCj4+Cj4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vYW1k L2FtZGdwdS9hbWRncHVfY3MuYyBiL2RyaXZlcnMvZ3B1L2RybS9hbWQvYW1kZ3B1L2FtZGdwdV9j cy5jCj4+IGluZGV4IGVhYTNjYjBjM2FkMS4uNGMwNGI1NjBlMzU4IDEwMDY0NAo+PiAtLS0gYS9k cml2ZXJzL2dwdS9kcm0vYW1kL2FtZGdwdS9hbWRncHVfY3MuYwo+PiArKysgYi9kcml2ZXJzL2dw dS9kcm0vYW1kL2FtZGdwdS9hbWRncHVfY3MuYwo+PiBAQCAtMTU5NCw3ICsxNTk0LDggQEAgaW50 IGFtZGdwdV9jc19maW5kX21hcHBpbmcoc3RydWN0IGFtZGdwdV9jc19wYXJzZXIgKnBhcnNlciwK Pj4gICAJKm1hcCA9IG1hcHBpbmc7Cj4+ICAgCj4+ICAgCS8qIERvdWJsZSBjaGVjayB0aGF0IHRo ZSBCTyBpcyByZXNlcnZlZCBieSB0aGlzIENTICovCj4+IC0JaWYgKFJFQURfT05DRSgoKmJvKS0+ dGJvLnJlc3YtPmxvY2suY3R4KSAhPSAmcGFyc2VyLT50aWNrZXQpCj4+ICsJaWYgKCF3d19tdXRl eF9pc19vd25lZF9ieSgmKCpibyktPnRiby5yZXN2LT5sb2NrLCBjdXJyZW50LAo+PiArCQkJCSAg JnBhcnNlci0+dGlja2V0KSkKPj4gICAJCXJldHVybiAtRUlOVkFMOwo+PiAgIAo+PiAgIAlpZiAo ISgoKmJvKS0+ZmxhZ3MgJiBBTURHUFVfR0VNX0NSRUFURV9WUkFNX0NPTlRJR1VPVVMpKSB7Cj4+ IGRpZmYgLS1naXQgYS9pbmNsdWRlL2xpbnV4L3d3X211dGV4LmggYi9pbmNsdWRlL2xpbnV4L3d3 X211dGV4LmgKPj4gaW5kZXggMzlmZGExOTViZjc4Li5kZDU4MGRiMjg5ZTggMTAwNjQ0Cj4+IC0t LSBhL2luY2x1ZGUvbGludXgvd3dfbXV0ZXguaAo+PiArKysgYi9pbmNsdWRlL2xpbnV4L3d3X211 dGV4LmgKPj4gQEAgLTM1OCw0ICszNTgsMjEgQEAgc3RhdGljIGlubGluZSBib29sIHd3X211dGV4 X2lzX2xvY2tlZChzdHJ1Y3Qgd3dfbXV0ZXggKmxvY2spCj4+ICAgCXJldHVybiBtdXRleF9pc19s b2NrZWQoJmxvY2stPmJhc2UpOwo+PiAgIH0KPj4gICAKPj4gKy8qKgo+PiArICogd3dfbXV0ZXhf aXNfb3duZWRfYnkgLSBpcyB0aGUgdy93IG11dGV4IGxvY2tlZCBieSB0aGlzIHRhc2sgaW4gdGhh dCBjb250ZXh0Cj4+ICsgKiBAbG9jazogdGhlIG11dGV4IHRvIGJlIHF1ZXJpZWQKPj4gKyAqIEB0 YXNrOiB0aGUgdGFzayBzdHJ1Y3R1cmUgdG8gY2hlY2sKPj4gKyAqIEBjdHg6IHRoZSB3L3cgYWNx dWlyZSBjb250ZXh0IHRvIHRlc3QKPj4gKyAqCj4+ICsgKiBSZXR1cm5zIHRydWUgaWYgdGhlIG11 dGV4IGlzIGxvY2tlZCBpbiB0aGUgY29udGV4dCBieSB0aGUgZ2l2ZW4gdGFzaywgZmFsc2UKPj4g KyAqIG90aGVyd2lzZS4KPj4gKyAqLwo+PiArc3RhdGljIGlubGluZSBib29sIHd3X211dGV4X2lz X293bmVkX2J5KHN0cnVjdCB3d19tdXRleCAqbG9jaywKPj4gKwkJCQkJc3RydWN0IHRhc2tfc3Ry dWN0ICp0YXNrLAo+PiArCQkJCQlzdHJ1Y3Qgd3dfYWNxdWlyZV9jdHggKmN0eCkKPj4gK3sKPj4g KwlyZXR1cm4gbGlrZWx5KF9fbXV0ZXhfb3duZXIoJmxvY2stPmJhc2UpID09IHRhc2spICYmCj4+ ICsJCVJFQURfT05DRShsb2NrLT5jdHgpID09IGN0eDsKPiBKdXN0IGNvbXBhcmluZyB0aGUgY29u dGV4dCBzaG91bGQgYmUgZ29vZCBlbm91Z2guIElmIHlvdSBldmVyIHBhc3MgYQo+IHd3X2FjcXVp cmVfY3R4IHdoaWNoIGRvZXMgbm90IGJlbG9uZyB0byB5b3VyIG93biB0aHJlYWQgeW91ciBzZXJp b3VzbHkKPiB3cmVha2luZyB0aGluZ3MgbXVjaCB3b3JzZSBhbHJlYWR5IChhbmQgaWYgd2UgZG8g Y2F0Y2ggdGhhdCwgc2hvdWxkCj4gcHJvYmFibHkgbG9jayB0aGUgY3R4IHRvIGEgZ2l2ZW4gdGFz ayB3aGVuIHd3LW11dGV4IGRlYnVnZ2luZyBpcyBlbmFibGVkKS4KPgo+IFRoYXQgYWxzbyBzaW1w bGlmaWVzIHRoZSBmdW5jdGlvbiBzaWduYXR1cmUuCj4KPiBPZiBjb3Vyc2UgdGhhdCBtZWFucyBp ZiB5b3UgZG9uJ3QgaGF2ZSBhIGN0eCwgeW91IGNhbid0IHRlc3Qgb3duZXJzaGlwIG9mCj4gYSB3 d19tdXRlLCBidXQgSSB0aGluayB0aGF0J3Mgbm90IGEgcmVhbGx5IHZhbGlkIHVzZS1jYXNlLgoK V2VsbCBleGFjdGx5IHRoYXQgaXMgdGhlIHVzZSBjYXNlIGluIFRUTSwgc2VlIHBhdGNoICMzIGlu IHRoaXMgc2VyaWVzLgoKSW4gVFRNIHRoZSBldmljdGVkIEJPcyBhcmUgdHJ5bG9ja2VkIGFuZCBz byB3ZSBuZWVkIGEgd2F5IG9mIHRlc3RpbmcgZm9yIApvd25lcnNoaXAgd2l0aG91dCBhIGNvbnRl eHQuCgpDaHJpc3RpYW4uCgo+ICAgQW5kIG5vdCBuZWVkZWQKPiBmb3IgY21kIHN1Ym1pc3Npb24s IHdoZXJlIHlvdSBuZWVkIHRoZSBjdHggYW55d2F5Lgo+Cj4gQmVzaWRlcyB0aGlzIGludGVyZmFj ZSBuaXQgbG9va3MgYWxsIGdvb2QuIFdpdGggdGhlIHRhc2sgY2hlY2smcGFyYW1ldGVyCj4gcmVt b3ZlZDoKPgo+IFJldmlld2VkLWJ5OiBEYW5pZWwgVmV0dGVyIDxkYW5pZWwudmV0dGVyQGZmd2xs LmNoPgo+Cj4gLURhbmllbAo+Cj4+ICt9Cj4+ICsKPj4gICAjZW5kaWYKPj4gLS0gCj4+IDIuMTQu MQo+Pgo+PiBfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwo+ PiBkcmktZGV2ZWwgbWFpbGluZyBsaXN0Cj4+IGRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5v cmcKPj4gaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmkt ZGV2ZWwKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmFt ZC1nZnggbWFpbGluZyBsaXN0CmFtZC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8v bGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vYW1kLWdmeAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752697AbeBSPl7 (ORCPT ); Mon, 19 Feb 2018 10:41:59 -0500 Received: from mail-wr0-f193.google.com ([209.85.128.193]:40029 "EHLO mail-wr0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752048AbeBSPl6 (ORCPT ); Mon, 19 Feb 2018 10:41:58 -0500 X-Google-Smtp-Source: AH8x226zPYbfujukx77hTHQCdIITE+Su+MM1BoYCbC7xzLmFF4bv94feLf33jPYIbKmqnvoJicNg3g== Reply-To: christian.koenig@amd.com Subject: Re: [PATCH 1/3] locking/ww_mutex: cleanup lock->ctx usage in amdgpu To: dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org References: <20180215141944.4332-1-christian.koenig@amd.com> <20180219152421.GQ22199@phenom.ffwll.local> From: =?UTF-8?Q?Christian_K=c3=b6nig?= Message-ID: Date: Mon, 19 Feb 2018 16:41:55 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <20180219152421.GQ22199@phenom.ffwll.local> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am 19.02.2018 um 16:24 schrieb Daniel Vetter: > On Thu, Feb 15, 2018 at 03:19:42PM +0100, Christian König wrote: >> amdgpu needs to verify if userspace sends us valid addresses and the simplest >> way of doing this is to check if the buffer object is locked with the ticket >> of the current submission. >> >> Clean up the access to the ww_mutex internals by providing a function >> for this and extend the check to the thread owning the underlying mutex. >> >> Signed-off-by: Christian König >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 3 ++- >> include/linux/ww_mutex.h | 17 +++++++++++++++++ >> 2 files changed, 19 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> index eaa3cb0c3ad1..4c04b560e358 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> @@ -1594,7 +1594,8 @@ int amdgpu_cs_find_mapping(struct amdgpu_cs_parser *parser, >> *map = mapping; >> >> /* Double check that the BO is reserved by this CS */ >> - if (READ_ONCE((*bo)->tbo.resv->lock.ctx) != &parser->ticket) >> + if (!ww_mutex_is_owned_by(&(*bo)->tbo.resv->lock, current, >> + &parser->ticket)) >> return -EINVAL; >> >> if (!((*bo)->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS)) { >> diff --git a/include/linux/ww_mutex.h b/include/linux/ww_mutex.h >> index 39fda195bf78..dd580db289e8 100644 >> --- a/include/linux/ww_mutex.h >> +++ b/include/linux/ww_mutex.h >> @@ -358,4 +358,21 @@ static inline bool ww_mutex_is_locked(struct ww_mutex *lock) >> return mutex_is_locked(&lock->base); >> } >> >> +/** >> + * ww_mutex_is_owned_by - is the w/w mutex locked by this task in that context >> + * @lock: the mutex to be queried >> + * @task: the task structure to check >> + * @ctx: the w/w acquire context to test >> + * >> + * Returns true if the mutex is locked in the context by the given task, false >> + * otherwise. >> + */ >> +static inline bool ww_mutex_is_owned_by(struct ww_mutex *lock, >> + struct task_struct *task, >> + struct ww_acquire_ctx *ctx) >> +{ >> + return likely(__mutex_owner(&lock->base) == task) && >> + READ_ONCE(lock->ctx) == ctx; > Just comparing the context should be good enough. If you ever pass a > ww_acquire_ctx which does not belong to your own thread your seriously > wreaking things much worse already (and if we do catch that, should > probably lock the ctx to a given task when ww-mutex debugging is enabled). > > That also simplifies the function signature. > > Of course that means if you don't have a ctx, you can't test ownership of > a ww_mute, but I think that's not a really valid use-case. Well exactly that is the use case in TTM, see patch #3 in this series. In TTM the evicted BOs are trylocked and so we need a way of testing for ownership without a context. Christian. > And not needed > for cmd submission, where you need the ctx anyway. > > Besides this interface nit looks all good. With the task check¶meter > removed: > > Reviewed-by: Daniel Vetter > > -Daniel > >> +} >> + >> #endif >> -- >> 2.14.1 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel