From mboxrd@z Thu Jan 1 00:00:00 1970 From: Frediano Ziglio Subject: Re: [PATCH 09/11] Move main reference counter to GEM object instead of TTM ones Date: Fri, 29 May 2015 07:11:52 -0400 (EDT) Message-ID: <1775637487.6685866.1432897912303.JavaMail.zimbra@redhat.com> References: <1432721046-4418-1-git-send-email-fziglio@redhat.com> <1432721046-4418-10-git-send-email-fziglio@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: spice-devel-bounces@lists.freedesktop.org Sender: "Spice-devel" To: Dave Airlie Cc: Dave Airlie , spice-devel , LKML , dri-devel , Dave Airlie List-Id: dri-devel@lists.freedesktop.org Cj4gT24gMjcgTWF5IDIwMTUgYXQgMjA6MDQsIEZyZWRpYW5vIFppZ2xpbyA8ZnppZ2xpb0ByZWRo YXQuY29tPiB3cm90ZToKPiA+IHF4bF9ibyBzdHJ1Y3R1cmUgaGFzIHR3byByZWZlcmVuY2UgY291 bnRlcnMsIG9uZSBpbiB0aGUgR0VNIG9iamVjdCBhbmQKPiA+IGFub3RoZXIgaW4gdGhlIFRUTSBv YmplY3QuIFRoZSBHRU0gb2JqZWN0IGtlZXAgYSBjb3VudGVyIHRvIHRoZSBUVE0gb2JqZWN0Cj4g PiBzbyB3aGVuIEdFTSBjb3VudGVyIHJlYWNoZWQgemVybyB0aGUgVFRNIGNvdW50ZXIgKHVzaW5n IHF4bF9ib191bnJlZikgd2FzCj4gPiBkZWNyZW1lbnRlZC4gVGhlIHF4bCBvYmplY3QgaXMgZnVs bHkgZnJlZWQgKGJvdGggR0VNIGFuZCBUVE0gcGFydCBhcmUKPiA+IGNsZWFuZWQpCj4gPiB3aGVu IHRoZSBUVE0gY291bnRlciByZWFjaCB6ZXJvLgo+ID4gT25lIGlzc3VlIHdhcyB0aGF0IHN1cmZh Y2UgaWRyIHN0cnVjdHVyZSBoYXMgbm8gb3duaW5nIG9uIHF4bF9ibyBvYmplY3RzCj4gPiBob3dl dmVyCj4gPiBpdCBjb250YWlucyBhIHBvaW50ZXIgdG8gcXhsX2JvIG9iamVjdC4gVGhpcyBjYXVz ZWQgc29tZSBuYXN0eSByYWNlCj4gPiBjb25kaXRpb24KPiA+IGZvciBpbnN0YW5jZSBxeGxfYm8g b2JqZWN0IHdhcyByZWFwZWQgZXZlbiBhZnRlciBjb3VudGVyIHdhcyBhbHJlYWR5IHplcm8uCj4g PiBUaGlzIHBhdGNoIGZpeCB0aGVzZSByYWNlcyBtb3ZpbmcgbWFpbiBjb3VudGVyICh0aGUgb25l IHVzZWQgYnkKPiA+IHF4bF9ib18odW4pcmVmKQo+ID4gdG8gR0VNIG9iamVjdCB3aGljaCBjbGVh bnVwIHJvdXRpbmUgKHF4bF9nZW1fb2JqZWN0X2ZyZWUpIHJlbW92ZSB0aGUgaWRyCj4gPiBwb2lu dGVyCj4gPiAodXNpbmcgcXhsX3N1cmZhY2VfZXZpY3QpIHdoZW4gdGhlIGNvdW50ZXJzIGFyZSBz dGlsbCB2YWxpZC4KPiAKPiBVZ2doLCBidXQgeWVzLCBub3Qgc3VyZSBJIGxpa2UgdGhpcyBmaXgg Zm9yIHRoZSBwcm9ibGVtLCBidXQgaWYgaXQgd29ya3MsCj4gCj4gUmV2aWV3ZWQtYnk6IERhdmUg QWlybGllIDxhaXJsaWVkQHJlZGhhdC5jb20+Cj4gCgpXZWxsLCB0aGUgcGF0Y2ggZG9lcyBub3Qg c3VyZWx5IGxvb2tzIHZlcnkgY2xlYXIgYnV0IEkgY2FuIGFzc3VyZSB0aGUgcHJvYmxlbXMgaXQg Zml4ZXMgYXJlIG11Y2ggbGVzcyBjbGVhciB0byB1bmRlcnN0YW5kLgpIYXZpbmcgYSBwb2ludGVy IHRvIGEgb2JqZWN0IHRoZSBpcyBnb2luZyB0byBiZSBkZWxldGVkIHdoZW5ldmVyIGFub3RoZXIg dGhyZWFkIGRlY2lkZSB0byBjYXVzZXMgZGlmZmljdWx0IHJhY2VzLiBJIHRyaWVkIHRvIGF2b2lk IHRoaXMga2luZCBvZiBjaGFuZ2UgYW5kIGZpeCB0aGUgcmFjZXMgaW5zdGVhZCBidXQgd2FzIGEg bmlnaHRtYXJlLgpNeSBmaXJzdCBleHBlcmltZW50YWwgcGF0Y2ggYWRkZWQgYW4gYWRkaXRpb25h bCBjb3VudGVyIG9uIHRvcCBvZiBHRU0gYW5kIFRUTSBvbmUgYXMgdGhlIG1haW4gY291bnRlciBi dXQgYXQgdGhlIGVuZCB3YXMgbXVjaCBtb3JlIGNvbXBsaWNhdGVkIGFuZCByZXN1bHQgd2FzIHNp bWlsYXIgdG8gbW92ZSB0aGUgbWFpbiBjb3VudGVyIHRvIEdFTS4KTWFpbmx5IGV4dGVybmFsIHJl ZmVyZW5jZXMgKGZyb20gdXNlcnNwYWNlIGFuZCBrZXJuZWwpIGFyZSBwb2ludGVycyB0byBHRU0u IFBvaW50ZXJzIHRvIFRUTSBhcmUgZnJvbSBtZW1vcnkgbWFwcGVkIGZpbGVzLiBEZWxldGluZyB0 aGUgc3BpY2UgaWQgYWZ0ZXIgR0VNIG9iamVjdCBoYXMgbm8gcmVmZXJlbmNlcyBhc3N1cmUgdGhl IG5vdCBvd25pbmcgcmVmZXJlbmNlIGZyb20gc3BpY2UgaWQgc3RpbGwgcmVmZXIgdG8gYSB2YWxp ZCBvYmplY3QuIEFzIHVzZXIgY2FuJ3QgcmV0cmlldmUgYSBwb2ludGVyIGZyb20gYSBtYXBwaW5n IChhdCBtb3N0IGNhbiByZW1hcCBpdCkgc28gdGhlcmUgYXJlIG5vIHJpc2tzIGNvdW50ZXIgdG8g R0VNIGlzIGluY3JlbWVudGVkIGFnYWluLgoKRnJlZGlhbm8KX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX18KU3BpY2UtZGV2ZWwgbWFpbGluZyBsaXN0ClNwaWNl LWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3Jn L21haWxtYW4vbGlzdGluZm8vc3BpY2UtZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755751AbbE2LMF (ORCPT ); Fri, 29 May 2015 07:12:05 -0400 Received: from mx5-phx2.redhat.com ([209.132.183.37]:35059 "EHLO mx5-phx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755144AbbE2LLz (ORCPT ); Fri, 29 May 2015 07:11:55 -0400 Date: Fri, 29 May 2015 07:11:52 -0400 (EDT) From: Frediano Ziglio To: Dave Airlie Cc: spice-devel , Dave Airlie , dri-devel , Dave Airlie , LKML Message-ID: <1775637487.6685866.1432897912303.JavaMail.zimbra@redhat.com> In-Reply-To: References: <1432721046-4418-1-git-send-email-fziglio@redhat.com> <1432721046-4418-10-git-send-email-fziglio@redhat.com> Subject: Re: [Spice-devel] [PATCH 09/11] Move main reference counter to GEM object instead of TTM ones MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [10.36.6.245] X-Mailer: Zimbra 8.0.6_GA_5922 (ZimbraWebClient - FF38 (Linux)/8.0.6_GA_5922) Thread-Topic: Move main reference counter to GEM object instead of TTM ones Thread-Index: mjEOQ6ebqTWpAwPNLXFkvY8F2QNbvw== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > On 27 May 2015 at 20:04, Frediano Ziglio wrote: > > qxl_bo structure has two reference counters, one in the GEM object and > > another in the TTM object. The GEM object keep a counter to the TTM object > > so when GEM counter reached zero the TTM counter (using qxl_bo_unref) was > > decremented. The qxl object is fully freed (both GEM and TTM part are > > cleaned) > > when the TTM counter reach zero. > > One issue was that surface idr structure has no owning on qxl_bo objects > > however > > it contains a pointer to qxl_bo object. This caused some nasty race > > condition > > for instance qxl_bo object was reaped even after counter was already zero. > > This patch fix these races moving main counter (the one used by > > qxl_bo_(un)ref) > > to GEM object which cleanup routine (qxl_gem_object_free) remove the idr > > pointer > > (using qxl_surface_evict) when the counters are still valid. > > Uggh, but yes, not sure I like this fix for the problem, but if it works, > > Reviewed-by: Dave Airlie > Well, the patch does not surely looks very clear but I can assure the problems it fixes are much less clear to understand. Having a pointer to a object the is going to be deleted whenever another thread decide to causes difficult races. I tried to avoid this kind of change and fix the races instead but was a nightmare. My first experimental patch added an additional counter on top of GEM and TTM one as the main counter but at the end was much more complicated and result was similar to move the main counter to GEM. Mainly external references (from userspace and kernel) are pointers to GEM. Pointers to TTM are from memory mapped files. Deleting the spice id after GEM object has no references assure the not owning reference from spice id still refer to a valid object. As user can't retrieve a pointer from a mapping (at most can remap it) so there are no risks counter to GEM is incremented again. Frediano