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 5F490C77B60 for ; Wed, 26 Apr 2023 21:17:45 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A024C6B012F; Wed, 26 Apr 2023 17:17:44 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9B1F56B0130; Wed, 26 Apr 2023 17:17:44 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 879886B0131; Wed, 26 Apr 2023 17:17:44 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 744E76B012F for ; Wed, 26 Apr 2023 17:17:44 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 43F131C6A6A for ; Wed, 26 Apr 2023 21:17:44 +0000 (UTC) X-FDA: 80724804048.05.3B29CCD Received: from mail-pj1-f50.google.com (mail-pj1-f50.google.com [209.85.216.50]) by imf02.hostedemail.com (Postfix) with ESMTP id 7162080014 for ; Wed, 26 Apr 2023 21:17:42 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=zrqOQt3v; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf02.hostedemail.com: domain of cmllamas@google.com designates 209.85.216.50 as permitted sender) smtp.mailfrom=cmllamas@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1682543862; 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=4Mn6UfNCg0frCwO9LQEVgo3zTrnFxCqpigYKkVd6EnQ=; b=1+kyrEfqJlzf7mpX1j5qZVNgA8YSs7EziegzY9cnuscAZXtEloa2Suh2bOOXIIvv+4XtVz 4TqqWRbaHak8q7hPVCoX7UUgsDZNZtzbM9Y3sYMN99+ac4oKuqC5PhWS1RGjj8ydyFycEq D3EluJenbqVklzAvsMsMNN+P0B+FPJY= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=zrqOQt3v; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf02.hostedemail.com: domain of cmllamas@google.com designates 209.85.216.50 as permitted sender) smtp.mailfrom=cmllamas@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1682543862; a=rsa-sha256; cv=none; b=gpivM4ifa5m8dMTOZFj22BH7dbyD9C6f6Rb9TP/WAbGIc5zG97fMGZC5nUWJ8KwKJVKY0a KB0lbKYJEVihCgA/vjrE5U0o+jCUFF8v5+5s4noxtrF730IYOQgQxaMvMuX3QkUjXAfE7Q 1u9Xc1ujf8Y0n1d6HFlucbOV41bMdsw= Received: by mail-pj1-f50.google.com with SMTP id 98e67ed59e1d1-2474e09fdcfso6428999a91.0 for ; Wed, 26 Apr 2023 14:17:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1682543861; x=1685135861; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=4Mn6UfNCg0frCwO9LQEVgo3zTrnFxCqpigYKkVd6EnQ=; b=zrqOQt3vdLfXDAeTjlXqmMr8Ec9NjcVaOcvqpHYfKX+LD3A+CR5V10B9dDUmPeOn6l Mw9rdth014TD14+uVerzeCMXVi21GmnSZqIktpQmSPdRBtHcIN2xP+IMdXxwMkMPehUU ASyUAs9lrWygPrjbil61i3XS8CqREgMyFTC4IYgs8VIU+jDlUXvNNF8NFz+n+s4UoLO3 T6eTk+Badq+U/FUAmZDYU0uGFukuQ7abN2sIsljGeI2igl70BG43DJHzCs+BcZaqLne9 YtnkCl2L3azyKiA24BE1njB56iQVpZeytVegm0htnvVJPkESsaml0LV2fPjIfP3rzbIa RZwA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682543861; x=1685135861; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=4Mn6UfNCg0frCwO9LQEVgo3zTrnFxCqpigYKkVd6EnQ=; b=N9NAjyKZmDPva2u4QX527u0EPyBbhQ32IrCNSfKMsBRgiDgFM/CD8GUVeYlyxZjdQc FAOWg3muEF25hTkYukvrBW9pcf4dj9MEdhiovPJRuw+3sgF/v6arrcErLBzcYOjqStQM lsMvjUxNsG8MEx2oeGonRpIS0n9fbz7bHJYurEd0OTkeUHYafa5gALALhwHj5QuzzBIc wdlF7tgYXW0R1iobZYmEtpYKzBvw3mksKG2tmSJHcTnvvIYJsFldOl4KiNZB47UhJEjs iEkUifBHBtA/ju72nPylaoY3fzuHHtPHuGYQ3CYKnJZv/YWlGdv0MF2n7Rv/FfxP3K2P 0ciA== X-Gm-Message-State: AAQBX9c60Wm0hg8R41OUPMnPF+1FA+NgqydeBhvYMb/7Bgt0K9b/FPCq a4o7r4PwZ7FfqLCS69B/ZFDRcw== X-Google-Smtp-Source: AKy350Y5ZisNDXgxTIhq93sjeqfrFBwlRJvH5WaxjoX5i7xg40wIQ26a6adZ2jxKYgd09EMsxx96ZA== X-Received: by 2002:a17:90a:660d:b0:246:f9ff:8e70 with SMTP id l13-20020a17090a660d00b00246f9ff8e70mr21040459pjj.26.1682543860989; Wed, 26 Apr 2023 14:17:40 -0700 (PDT) Received: from google.com (157.214.185.35.bc.googleusercontent.com. [35.185.214.157]) by smtp.gmail.com with ESMTPSA id n10-20020a63ee4a000000b0051b0e564963sm10314070pgk.49.2023.04.26.14.17.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Apr 2023 14:17:40 -0700 (PDT) Date: Wed, 26 Apr 2023 21:17:37 +0000 From: Carlos Llamas To: "Liam R. Howlett" , Greg Kroah-Hartman , Arve =?iso-8859-1?B?SGr4bm5lduVn?= , Todd Kjos , Martijn Coenen , Joel Fernandes , Christian Brauner , Suren Baghdasaryan , linux-kernel@vger.kernel.org, kernel-team@android.com, linux-mm@kvack.org Subject: Re: [RFC PATCH 2/3] Revert "android: binder: stop saving a pointer to the VMA" Message-ID: References: <20230424205548.1935192-1-cmllamas@google.com> <20230424205548.1935192-2-cmllamas@google.com> <20230424223419.6n2z72mocgmcj3aw@revolver> <20230425014328.d6vvimziv6je5xdg@revolver> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230425014328.d6vvimziv6je5xdg@revolver> X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 7162080014 X-Stat-Signature: kodjz1h3omqt68kspfkmdfgfmbgi6fnn X-HE-Tag: 1682543862-368082 X-HE-Meta: U2FsdGVkX18QmUIgsT0R9V6/BL43EcU5+/EzYyiyhIefmW8CbUtyBM/QZdUtM1Iu+6HRYHdt2xwHlgtQ3VqLg181IVwH5e+cmdpQB6VwaMgFDHOQIQv7ICBt2afYg70YBm5TUepSbWjoKvx3jdESoCeutkWhJ7t/YTvx0O671UGQn5u/Ofz+sl2OQiYxctriOG/zqKEgYQTR4iOaADT1FLIOI6PHqmV7qa0CLi0dMEjt5m3GNVnV10dsO0DvHPKcxIPELrYl2T7KvN1nPkILjYmFNX3mrzBK1YxZYTXeiNyBVa2DgN71f/cGy5u3MpYd2c5IVsbfqNCU3o+qmlUwUNoVCjMsVlYZfDv+dTHT7F/ZaOg8MtGLGdSzWG43jSuzbM6V3ANmNfw+U5ZW8/8mXrpnbP6D/KxP0TGVkGPMqOXjtmvszjpEmfXX3tdcNTDM4rONZxKUJBqxTzpJIl2zAHuJ4pzCyqDd4ZnA1MMHEQI6z+Jxt4iKHbm0qJ8AGEfT3UG91w03yMlxMuu9htTy4Pnu1b5uWxcS+yb2Zz8jHIvsKRHn95p05u3ms6T6XcmPitHVO0u766anPvufDZZmZyMFsZlUtoG19l41L+Y6wyavrnQWBURQt6Fvicmqwdzkw/I7D2ScrFyU5TXU/NQOkglqJSopfthkyTJiM7kMBPjtwia7OiXud+B0f49UxIK+OvcR49nwl0PawxaIMsblUL/4EMWUb+pk5UgMWn3rWQGwvJGHImJcByOZ7zh2kS/DWYjmkGmKQPmsNHHfgJx82oKEmRFEQr9a2uGMP+j5AQsTqAoqZYC5UV9WKVTkhD3tt0djxXjRfYAGF3vfU9tbdFOycRCLlBufWxkN3z7onRzM1nQl1mrruI8iMJD7VgUhA2x/dhTKyvPAoIcw+b9bn1OZd7ONSqNS1Dh/mSupeDrxTvZYuTR/mIt3xuuR0tXheXQ4enqk9KUzTm7xDCS VIfMwr1+ lI/DSr+n3vGllhDnY8znW2af8SDN80TXp5IjiP+eKgVbvSzoC5JsZt4SEOK/vun/oDaIE23F3q1hJZEKIsHPSN0FqlV43gnz3w59zvSyBjvoSVTtLJeNtm0Xtp85OGE71Rqh73uXoIEUCrjlakgjTc2UeyYSNmr0bwCeqk53bQcz+7fkQx4Ff+KjD2WIOEMvfiazKCOa8t5dIjPHoXe5j0ENxWidyhsl1VW7wOqt/ggSmYI13VboKQTU/1euPrzUxvf10HuUQaaXEfsl03Jo70pztwXRpndNLohwLTNzowSSBrHoT2wi5EvXZEivjI2lskzLun5AOHrl3hMDqGeQIdU2S9PZcoEnm836GjWb/6odXD7UT1oGrLEGxt3hLIRjWfxj7S/YsCZXLW58cMbH7YeIc2IoG9fZxGEeIREaPjBAXxYkJTwwKjWtBjfoux1D62baz X-Bogosity: Ham, tests=bogofilter, spamicity=0.000034, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Apr 24, 2023 at 09:43:28PM -0400, Liam R. Howlett wrote: > * Carlos Llamas [230424 19:11]: > > > > The specifics are in the third patch of this patchset but the gist of it > > is that during ->mmap() handler, binder will complete the initialization > > of the binder_alloc structure. With the last step of this process being > > the caching of the vma pointer. Since the ordering is protected with a > > barrier we can then check alloc->vma to determine if the initialization > > has been completed. > > > > Since this check is part of the critical path for every single binder > > transaction, the performance plummeted when we started contending for > > the mmap_lock. In this particular case, binder doesn't actually use the > > vma. > > So why does binder_update_page_range() take the mmap_read_lock then use > the cached vma in the reverted patch? > > If you want to use it as a flag to see if the driver is initialized, why > not use the cached address != 0? > > Or better yet, > > >It only needs to know if the internal structure has been fully > > initialized and it is safe to use it. > > This seems like a good reason to use your own rwsem. This is, > essentially, rolling your own lock with > smp_store_release()/smp_load_acquire() and a pointer which should not be > cached. We can't use an rwsem to protect the initialization. We already have an alloc->mutex which would be an option. However, using it under ->mmap() would only lead to dead-locks with the mmap_lock. I agree with you that we could use some other flag instead of the vma pointer to signal the initialization. I've actually tried several times to come up with a scenario in which caching the vma pointer becomes an issue to stop doing this altogether. However, I can't find anything concrete. I don't think the current solution in which we do all these unnecessary vma lookups is correct. Instead, I'm currently working on a redesign of this section in which binder stops to allocate/insert pages manually. We should be making use of the page-fault handler and let the infra handle all the work. The overall idea is here: https://lore.kernel.org/all/ZEGh4mliGHvyWIvo@google.com/ It's hard to make the case for just dropping the vma pointer after ~15 years and take the performance hit without having an actual issue to support this idea. So I'll revert this for now and keep working on the page-fault solution. Thanks Liam, I'll keep you in the loop.