From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755432AbdJPVbd (ORCPT ); Mon, 16 Oct 2017 17:31:33 -0400 Received: from home.keithp.com ([63.227.221.253]:46424 "EHLO elaine.keithp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751859AbdJPVbc (ORCPT ); Mon, 16 Oct 2017 17:31:32 -0400 From: "Keith Packard" To: Sean Paul Cc: linux-kernel@vger.kernel.org, Dave Airlie , Daniel Vetter , dri-devel@lists.freedesktop.org Subject: Re: [PATCH 5/5] drm: Add four ioctls for managing drm mode object leases [v6] In-Reply-To: <20171016210315.stpau32hblgxfsgm@art_vandelay> References: <20171013015631.6926-1-keithp@keithp.com> <20171013015631.6926-6-keithp@keithp.com> <20171016210315.stpau32hblgxfsgm@art_vandelay> Date: Mon, 16 Oct 2017 14:31:30 -0700 Message-ID: <878tgawzil.fsf@keithp.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Sean Paul writes: > nit: space before , Thanks. >> + /* Clone the lessor file to create a new file for us */ >> + DRM_DEBUG_LEASE("Allocating lease file\n"); >> + path_get(&lessor_file->f_path); > > Please forgive the stupid question, but where is this reference given > up? That's not a stupid question, it's a very subtle one which took me quite a while to sort out. Here's path_get: void path_get(const struct path *path) { mntget(path->mnt); dget(path->dentry); } So, getting a reference on a 'path' actually gets a reference on two of the things it points to. alloc_file is passed the path and doesn't take an additional reference on either of these fields, presumably because the normal path has the caller taking a reference while looking up the object and handing that reference off to alloc_file. In our case, we're creating a new file that refers to the same path as an existing one, so we need another reference. When the file is finally freed in __fput, the two references are dropped at the end of the function: static void __fput(struct file *file) { struct dentry *dentry =3D file->f_path.dentry; struct vfsmount *mnt =3D file->f_path.mnt; ... dput(dentry); mntput(mnt); } This was probably the twistiest part of creating a lease. All of the DRM stuff was trivial; getting the core kernel object reference counts right was a pain. >> + if (lessee->lessor =3D=3D NULL) >> + /* owner can use all objects */ >> + object_idr =3D &lessee->dev->mode_config.crtc_idr; > > What about other types of objects? If I understand your question correctly, the answer is that 'crtc_idr' is misnamed -- it holds all of the mode setting objects. Thanks for your review, let me know if you have more questions! =2D-=20 =2Dkeith --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEw4O3eCVWE9/bQJ2R2yIaaQAAABEFAlnlJTIACgkQ2yIaaQAA ABEPFBAAnLQtArTAeLriRDlb43trQ78yMABAWhe9n3MUJWyZIV5Z1x/ERT2bdklT /mCNc4Qmi/dglxzB2CoJ62TLxZmTajNe4kn4PudBjFBh7wI+4q31aUxlL5gLoirf 6DFIDukeAG3HDRdXHqpkRByWu2ci27FdIT8huVIlrp2xbrY20+1kuaADU1GMeypT DjyRN7qfQroEqzqZXmJD96g5YivRj0GCw8kJdEO/ae8jibuW22oOPCH5BtcSLPbW ePO1Ytt9O1YUWY4g9E4fMhaWwRg5NCUZe9AohQtQHAZOu07KpvbKjv/AJg2VqwsK 4WAzzBkwstDwWb6sf3OKSxbbwVxdofPxhopcbA7l7alYPQl/BTFdum/jI2+1sOUm e3tHgTMuq2s05C1c3qM9SxOJXbPO7yDPi5q4RshYdRu1R8hv/P3+hjO+kUrJQv0R jrOxGholfKWIrQCeF4Oo59rjdsTapdbXn+e/VBRiHqMGShGOsoSFKRdYpSWQpx/K fnM+oPuvyCPoP9j1SqjSixiVP64/lWtEekOFjKzdpfair0lJfIdlqxHOJbxZMRPB i59O9iVlrzOMLMfMopIB2G4eRMad9ArOGKQ16o8rab7EaeXTMzATrS7gDabPh8pH ZAhDHOmDy2DaC+Gvpso6iEaf9oeuc37WXXEichWZTgoJPfibKNs= =hlZk -----END PGP SIGNATURE----- --=-=-=--