From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 91800C76196 for ; Sun, 26 Mar 2023 01:22:03 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DA0CB900004; Sat, 25 Mar 2023 21:22:02 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D50C6900002; Sat, 25 Mar 2023 21:22:02 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C3FB6900004; Sat, 25 Mar 2023 21:22:02 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id B25CE900002 for ; Sat, 25 Mar 2023 21:22:02 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 5CF3BAAE51 for ; Sun, 26 Mar 2023 01:22:02 +0000 (UTC) X-FDA: 80609298084.26.132B383 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf11.hostedemail.com (Postfix) with ESMTP id A6E8A40005 for ; Sun, 26 Mar 2023 01:22:00 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=AJPCqfuo; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf11.hostedemail.com: domain of broonie@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=broonie@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1679793720; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=AO1ph1qqpbQzR4r6dAMkCnKKim7qJa14BqVAA+R8bQI=; b=xLdVALZc+cqKXQIFTUG66rAiAQfK2kUXQHGSSO9WAFsVttmo9FnaXVc4JcoQB6GMnfbY82 jT7TFKpJQhIpPK0uVr2mX5l9wvXVniuN7OzrLcqSj12Y7wpT51gyqzmPg74xUfHJN71m8m APbAjHtGdyZQMVksm5l5VoQP4phhsOw= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=AJPCqfuo; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf11.hostedemail.com: domain of broonie@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=broonie@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1679793720; a=rsa-sha256; cv=none; b=nUeQ8CyVrJfeOO/tTFB8P1qiFHoj2IUqGgB1rjLUpu/fJ7Y3tuo7UjC0eQ5xVYjpyQTA0Y Lav/PWkVUUJ/PrSeCi0GtXLl5nqw0dtVtvX+Za2yXl0eDyCpqRNTCyeg/X/aX1y0PY0Y/D vP+hCeizZWjkxv/rjcWGOdvB33jhr1A= Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id AB62D60C82; Sun, 26 Mar 2023 01:21:58 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9EB7BC433D2; Sun, 26 Mar 2023 01:21:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1679793718; bh=Ncv9SH7Ho7z8GUYlNU6fYKX7cEx1O44FdSJC2LaGQeY=; h=Date:From:To:Subject:References:In-Reply-To:From; b=AJPCqfuo1npyPbJSZrYscIRMWoJETBUKic01yCgtU735v/vv41NGxYV/1+pt6J1nj R2Z2/N2KyrLAcs2quZmXT5tx4YlMRrv1VUhF6HasDm6RVe6yn8hbdVQrOHEDptSO7H RT+OyBVTSFBMUSF0clGVZqOwtS75O8PngJ6g0q2edNl+5g5xJ87eFqOhxhPO74hvZ0 3iypSlX/ND1riwDYp0eyXjkJEy5MuQLK5R9RgHOnYGi4jlubIlqyqSSfEROXa3ssn1 B6350D0OuN0JkFR/NNEzCitMl6jF3ehHhBXAbJRKrIXPwbgumALS0y08ctMjI5dCLG DC1upQjydUXWw== Date: Sun, 26 Mar 2023 02:21:54 +0100 From: Mark Brown To: "Liam R. Howlett" , linux-mm@kvack.org Subject: Re: [PATCH 2/2] regmap: Add maple tree based register cache Message-ID: References: <20230325-regcache-maple-v1-0-1c76916359fb@kernel.org> <20230325-regcache-maple-v1-2-1c76916359fb@kernel.org> <20230325234102.izwnfhnwfdryxlyk@revolver> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="lpjfk6Gqs3hUwNCC" Content-Disposition: inline In-Reply-To: <20230325234102.izwnfhnwfdryxlyk@revolver> X-Cookie: Single tasking: Just Say No. X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: A6E8A40005 X-Stat-Signature: niy5tzosxktwsyr9q5mt8cajpxb4yzdd X-HE-Tag: 1679793720-636254 X-HE-Meta: U2FsdGVkX1+xdj+QIMQJdW9kbp05BkqK6EYsf/0mKWqAfkx7qdvMpwKiHqhYDg+VQiCujmL+slNyalw0QWYrGnmHg2elXl4DVdQ4k/BZk1XXmpHXDrx+FTs9/rpsYZr+eApECp3oMAYEWFQDwmhZkulH4GfBPSb0o2OfXDIjGLyVw1VN8byZzLfLIRPwT9ftWOpBVEjg8I1AHcJeJRYJ29UA1iSbESJ+iA3nVfvaskA4238Oq3V5N7Qm3wyDQ+4g56NYqf1q9YpKqAk6VE0nfG1t5fGiXUFiWqIGCoTXlV8rsiHbxjrXDnk3wyiMbPnNYkBvw+P3CAcvySJtJ/qenxuIAKRmGhsA8wYlgsb2IRDwJFrr1fFFjp+Qbhz1xtlhJ4okAFpaH6L/uzDTDWyT4gbhbbOZzV3P3JemycgvABd0XOPNJNAbS83E70F962BCRc2ADMjNI1rGCPJr1dNa3YrI73qbbFTKIdDcBoNwvxJNqodOB0GcuKFtS6I6NcMI78bNuY0YP7n62Hxn5NkrQU1d1iPuawO2JjBa/+GXoRVGNSSgQY6GiBA6cZT29RKAd6cs2UTn3+OBvglSMQZDo/En7CaYZBxAtHhffsihSR5zf0wKVpZhmf5xdmuw97VB4hv0vJiqvfSvzH9L6LKxdhK0sUb4/4Y3R2fRBJMGuQ0rRXdOAjMn1dWnMoWMm/Myn7/jdFDBe5zwolMe8ZpRb0oVINxL4WWezK/ayS1w58IsiHAiIJ4bDQOMvNIh5VkvIpbbw4tUcwRBcv9hsD4cKfD/Qs3rIqS78e3qgdU3/pl/7eEiO+ydakwwQiWGZT0qtZ9BN4qaxCIB3SgvTEtmr11tLM2kOmjfaCWg/4d7EiULsuzcQG7Unj+SfWK5w+cDg4FBnKupHltzrZwLu009H6bT5OWhq8txt01fCDTk4if3vgkVjTBRMg0Ve8FWQaJPwrzXHEvYjrM3fNUnE9+ nPnhGB4P PNsbBk+uEYJHLcGPemdvbPIkGZR18i9RliGlCjZq5bg/s6K17S464SpBq4b8qmvsEZV/TRq0u9RfmQcEnGe3eTfAaY+DPeK4HOxhkUgxBUuDEL19nb1z6ejK6s192bZCbW92w79ZnOGlpulYFBfJknd2Xpp2Cejhs40O+fUnNbnvInZeqiUoflycVQyicD+Ja5tHvhdO5ulCwmwGQzUVEfW7w72vWwH7n8qeNAPiHO4FLZWkX2qe+lDpWzUi4Gkse+aaZPEcGr21b7zcydhTKnXotvx+elzp4xMODmuiiqqrep2cmLDDYDJ1A7BFOxIoV5wB8C6N2wOzIyukL5xVf5K2Zi0yTl06cgrL4STQ5Q2huWC4= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: --lpjfk6Gqs3hUwNCC Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Sat, Mar 25, 2023 at 07:41:02PM -0400, Liam R. Howlett wrote: > Without use of the locking, the maple tree will generate lockdep > complaints. > With the mas_* family of functions, you are responsible for locking. On > read you can take the rcu_read_lock(), on writes you need to take > mas_lock(&mas). > If you have a lock you already use, you can init the tree with the > external lock flag and set the external lock like we do in the VMA code. Oh, that's not ideal. regmap does have an external lock but it's already taken outside of the cache code, the cache code shouldn't have to worry about locking. This does mean the maple tree locks should never be contended, but there'll still be some overhead. OTOH we only have to be faster than accessing the underlying bus so it's merely annoying. I've added the locking locally. > > +static int regcache_maple_drop(struct regmap *map, unsigned int min, > > + unsigned int max) > > +{ > Wait, if this is for destroying the entire tree: > mas_lock(&mas); > mas_for_each(mas, entry, max) > kfree(entry); > __mt_destroy(mt); > mas_unlock(&mas); It's not just for that - it's also used for selectively discarding some entries in the cache, we just reuse the code when freeing the regmap. It may be worth using the above in the map free case, though it's not exactly a hot path. > If it's not for the entire tree, you can free a range then write a NULL > over the entire range. But be conscience of the maple state range after > the iterator ends, you will need to mas_set_range(&mas, min, max) prior > to the mas_store_gfp(). Will writing NULL leave tree entries in place that get iterated? Though the code will need to become more complicated anyway if we might need to split nodes that cover more than one register. > > +static int regcache_maple_exit(struct regmap *map) > > +{ > > + struct maple_tree *mt = map->cache; > > + > > + /* if we've already been called then just return */ > > + if (!mt) > > + return 0; > There's not a race here with multiple tasks in here at once, right? No, everything's locked at the caller level. It's just simplifying some of the error handling paths (IIRC the actual issue is freeing a cache that was never allocated). > > + mt_init(mt); > > + > The maple tree does support bulk loading (in the b-tree sense). You can > put the maple tree in this state and pre-allocate nodes in bulk. > kernel/fork.c currently does this through the vma iterator interface. > Note that after a bulk-load, you have to ensure you call mas_destroy() > to free any unused nodes and to potentially cause a rebalance of the > last node (this is automatic). But, again, like you said in your > comment; this is good for the first pass. Right, I saw the reallocation stuff and was going to save it for later after I'd arranged to store ranges of registers directly in the tree rather than having a node per register since there'll be some overlap there. --lpjfk6Gqs3hUwNCC Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmQfni8ACgkQJNaLcl1U h9DZ1Qf8Ct3UKXW3ZC4qLk1c8mCtFNSjD9wClr+9AgVAZ8v5qjHMfTdzY2NZH2xB 0Da+TjUprSh7C3JM44eswiqD/EoB5iinjzGaO7nb2Ztcjn2gR4t++YCRTW5nYlrx FZ3IGDzHN5A9awHGKqmEJcnC7ndzhqR3YLb8sI0oJhvKzHfa6epnN+ncsndVpA8l ow2oHAyAmvNql9w3Zd+n9X7uyaB9CaAtgymUiqX6qcBPaw1Om2Y4u/4ihBym+fZG Amh1MlO4lwo/4+fFsDIyW134ja20WNSyi6+8vnvcp53oQuvASQQKzWHfsth3owV6 VxOkhEtYj7N3/xGk1DGnWi/hqZWYrA== =7XGj -----END PGP SIGNATURE----- --lpjfk6Gqs3hUwNCC--