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: Tue, 20 Feb 2018 13:31:14 +0100 Message-ID: <9c0175bd-cd5e-e643-de9b-5b2f6b2449a1@gmail.com> References: <20180215141944.4332-1-christian.koenig@amd.com> <20180219152421.GQ22199@phenom.ffwll.local> <20180219161544.GY22199@phenom.ffwll.local> <2e696981-6b4a-e292-c16a-0f3477dbf8ce@gmail.com> <20180219164354.GB22199@phenom.ffwll.local> <20180220113318.GN22199@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: <20180220113318.GN22199-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: christian.koenig-5C7GfCeVMHo@public.gmane.org, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org QW0gMjAuMDIuMjAxOCB1bSAxMjozMyBzY2hyaWViIERhbmllbCBWZXR0ZXI6Cj4gW1NOSVBdCj4g QWgsIHNvIHRoZSB0dG1fY3R4IEkndmUgc3BvdHRlZCB3YXMgc29tZXRoaW5nIGVudGlyZWx5IGRp ZmZlcmVudCBhbmQKPiBkb2Vzbid0IGNvbnRhaW4gdGhlIHd3X2FjcXVpcmVfY3R4IChJIGRpZG4n dCBjaGVjayk/IEknZCBhc3N1bWUgeW91IGhhdmUKPiB0aGUgc2FtZSBjdHggcGFzc2VkIGFyb3Vu ZCB0byBldmVyeXRoaW5nIGluIHR0bSwgYnV0IGlmIHRoYXQgZG9lc24ndCBleGlzdAo+IHRoZW4g d2UgY2FuIGluZGVlZCBub3QgYW5ub3RhdGUgd3dfbXV0ZXhfdHJ5bG9ja19jdHggd2l0aCB0aGUg cmlnaHQgY3R4LgoKWWVzLCBleGFjdGx5LgoKSSBhY3R1YWxseSB0cmllZCB0aGlzIGFwcHJvYWNo LCBlLmcuIHB1dCB0aGUgd3dfYWNxdWlyZV9jb250ZXh0IGludG8gdGhlIAp0dG1fb3BlcmF0aW9u X2NvbnRleHQgYW5kIHRoZW4gdXNlIHRoYXQgd2l0aCB3d19tdXRleF90cnlsb2NrX2N0eC4KCkJ1 dCBhKSB0aGF0IHR1cm5lZCBvdXQgdG8gYmUgdG8gbXVjaCBoYXNzbGUsIGUuZy4gYXQgbGVhc3Qg YW1kZ3B1IApkb2Vzbid0IHVzZSBhIHd3X2FjcXVpcmUgY29udGV4dCBpbiBtb3N0IGNhc2VzLgoK QW5kIGIpIGl0IGFjdHVhbGx5IHdhc24ndCB3aGF0IEkgd2FzIGxvb2tpbmcgZm9yLCBlLmcuIEkg Y291bGRuJ3QgCmRpc3RpbmN0IGJldHdlZW4gdGhlIHRyeWxvY2tlZCBCT3MgYW4gZXZlcnl0aGlu ZyBlbHNlIGFueSBtb3JlLgoKPj4gW1NOSVBdCj4+IEJ1dCB0byBtZSBpdCBhY3R1YWxseSBsb29r cyBtb3JlIGxpa2UgdGhhdCBtYWtlcyBpdCB1bm5lY2Vzc2FyeSBjb21wbGljYXRlZC4KPj4gVGhl IHVzZSBjYXNlIGluIGFtZGdwdSB3aGljaCBjb3VsZCBvbmx5IGNoZWNrIHRoZSBjb250ZXh0IGlz bid0IHBlcmZvcm1hbmNlCj4+IGNyaXRpY2FsLgo+IE9oIEknbSBub3Qgd29ycmllZCBhYm91dCB0 aGUgcnVudGltZSBvdmVyaGVhZCBhdCBhbGwsIEknbSB3b3JyaWVkIGFib3V0Cj4gY29uY2VwdHVh bCBjbGFyaXR5IG9mIHRoaXMgc3R1ZmYuIElmIHlvdSBoYXZlIGEgY3R4IHRoZXJlJ3Mgbm8gbmVl ZCB0bwo+IGFsc28gbG9vayBhdCAtPm93bmVyLgo+Cj4gQW5vdGhlciBpZGVhOiBXZSBkcm9wIHRo ZSB0YXNrIGFyZ3VtZW50IGZyb20gZnVuY3Rpb25zIGFuZCBnbyB3aXRoIHRoZQo+IGZvbGxvd2lu ZyBsb2dpYzoKPgo+IHd3X211dGV4X2lzX293bmVyKGxvY2ssIGN0eCkKPiB7Cj4gCWlmIChjdHgp Cj4gCQlyZXR1cm4gbG9jay0+Y3R4ID09IGN0eDsKPiAJZWxzZQo+IAkJcmV0dXJuIGxvY2stPm93 bmVyID09IGN1cnJlbnQ7Cj4gfQo+Cj4gSSB0aGluayB0aGF0IHdvdWxkIHNvbHZlIHlvdXIgdXNl IGNhc2UsIGFuZCBnaXZlcyB1cyB0aGUgbmVhdCBpbnRlcmZhY2UKPiBJJ20gYWltaW5nIGZvci4g S2VybmVsZG9jIGNhbiB0aGVuIGV4cGxhaW4gd2hhdCdzIGhhcHBlbmluZyBmb3IgYSBOVUxMCj4g Y3R4LgoKR29vZCBwb2ludCwgZ29pbmcgdG8gYWRqdXN0IHRoZSBwYXRjaGVzIHRoaXMgd2F5IGFu ZCByZXNlbmQuCgpDaHJpc3RpYW4uCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fCmFtZC1nZnggbWFpbGluZyBsaXN0CmFtZC1nZnhAbGlzdHMuZnJlZWRlc2t0 b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vYW1k LWdmeAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751828AbeBTMbU (ORCPT ); Tue, 20 Feb 2018 07:31:20 -0500 Received: from mail-wr0-f182.google.com ([209.85.128.182]:36352 "EHLO mail-wr0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751630AbeBTMbR (ORCPT ); Tue, 20 Feb 2018 07:31:17 -0500 X-Google-Smtp-Source: AH8x225P59gR02Aavfdtc6MqM/mMk0dwDcaSGUpapGApPXIm4GPaaB2cqpc86x2e2rljEH91Ih90dA== Reply-To: christian.koenig@amd.com Subject: Re: [PATCH 1/3] locking/ww_mutex: cleanup lock->ctx usage in amdgpu To: christian.koenig@amd.com, 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> <20180219161544.GY22199@phenom.ffwll.local> <2e696981-6b4a-e292-c16a-0f3477dbf8ce@gmail.com> <20180219164354.GB22199@phenom.ffwll.local> <20180220113318.GN22199@phenom.ffwll.local> From: =?UTF-8?Q?Christian_K=c3=b6nig?= Message-ID: <9c0175bd-cd5e-e643-de9b-5b2f6b2449a1@gmail.com> Date: Tue, 20 Feb 2018 13:31:14 +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: <20180220113318.GN22199@phenom.ffwll.local> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am 20.02.2018 um 12:33 schrieb Daniel Vetter: > [SNIP] > Ah, so the ttm_ctx I've spotted was something entirely different and > doesn't contain the ww_acquire_ctx (I didn't check)? I'd assume you have > the same ctx passed around to everything in ttm, but if that doesn't exist > then we can indeed not annotate ww_mutex_trylock_ctx with the right ctx. Yes, exactly. I actually tried this approach, e.g. put the ww_acquire_context into the ttm_operation_context and then use that with ww_mutex_trylock_ctx. But a) that turned out to be to much hassle, e.g. at least amdgpu doesn't use a ww_acquire context in most cases. And b) it actually wasn't what I was looking for, e.g. I couldn't distinct between the trylocked BOs an everything else any more. >> [SNIP] >> But to me it actually looks more like that makes it unnecessary complicated. >> The use case in amdgpu which could only check the context isn't performance >> critical. > Oh I'm not worried about the runtime overhead at all, I'm worried about > conceptual clarity of this stuff. If you have a ctx there's no need to > also look at ->owner. > > Another idea: We drop the task argument from functions and go with the > following logic: > > ww_mutex_is_owner(lock, ctx) > { > if (ctx) > return lock->ctx == ctx; > else > return lock->owner == current; > } > > I think that would solve your use case, and gives us the neat interface > I'm aiming for. Kerneldoc can then explain what's happening for a NULL > ctx. Good point, going to adjust the patches this way and resend. Christian.