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 8614EC7EE32 for ; Wed, 31 May 2023 07:51:50 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 25DDF900003; Wed, 31 May 2023 03:51:50 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 20A98900002; Wed, 31 May 2023 03:51:50 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0FA3A900003; Wed, 31 May 2023 03:51:50 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 00F83900002 for ; Wed, 31 May 2023 03:51:49 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id CA7BC1A021D for ; Wed, 31 May 2023 07:51:49 +0000 (UTC) X-FDA: 80849781138.06.D07348E Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf23.hostedemail.com (Postfix) with ESMTP id 5E7CB14000F for ; Wed, 31 May 2023 07:51:47 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=VaWpl4Ye; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf23.hostedemail.com: domain of david@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=david@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1685519507; a=rsa-sha256; cv=none; b=WfbLpYyhjCsZ6PAMTsjy+YQ4eHSE8wYwuSGe54WdX9IZlIePs7zYgvQtYpm2R9IPGC3GRG oxXU52u3a4BkMhS8gJF0Hj6Lf+OYkX+nXTQGcNdTVs4UcZMPnebs/kAuBJ3WF2sTnulhug gWNimTK69d9rMaOLXsPtuReVABswv+A= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=VaWpl4Ye; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf23.hostedemail.com: domain of david@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=david@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1685519507; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=8gzt4NYLA85nb15i+zO3AepaT972sSN8cGl5vcJ610U=; b=kZf3dT3Dl50b7ZLq5U5Y1VsBVFtyj3vVC6LO166hTKFJ5HQcyIoxTzy0ojEQmtttX5Y6U+ dZdq1UUq/ZH5sH88Nb2YJbmqC7bWOOZIRQTlbmp7BCQVMjDjBVWG1B5K/TQ4PGSnYoUlrj 1tE96EiNeDpyApETj77j1BoYMVyCjzA= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1685519506; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=8gzt4NYLA85nb15i+zO3AepaT972sSN8cGl5vcJ610U=; b=VaWpl4YeO7M2mQSo3D0daSaZNMJejhBmN/9Ww1SSkUnxfDhXvHiL87PNPlUjBThlh2TdLy HqPJF4eUDRl4Ptg1kufODvUYWN97/V3Lx8AttUvUz8dIDcpxOFIdJ+PDkoVYX413R/qckW lpKp2JnkD5WAhHcC/d5ZAgdH+Rgz3xg= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-38-d5EmFjlfMD-VrkCqr6TBIQ-1; Wed, 31 May 2023 03:51:45 -0400 X-MC-Unique: d5EmFjlfMD-VrkCqr6TBIQ-1 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-3f608ea689cso33860675e9.3 for ; Wed, 31 May 2023 00:51:45 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685519504; x=1688111504; h=content-transfer-encoding:in-reply-to:organization:from:references :cc:to:content-language:subject:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=8gzt4NYLA85nb15i+zO3AepaT972sSN8cGl5vcJ610U=; b=fufQGzRryPyn/tIOj+eHLZZlVt/p1eK3LkJvBXPMDg9x66fF1LczMw5PdqWEpwW4xm MoSVxcxZHbWDlxGUAcYa7j9O5oYqWwyquS6Nt3HlOXdI+vS7Uq+1lZUgLD5LNHEP8F5o lyQSb/SU7LmobeN1hqDzK7uAvwPGnZPofOt72ya467XvX6NOO/15B19JazJua9iDRLZX wohJK9vCdyAFlKg+vD3FHLQWZmY5OkCURQ1ZhFgsmrBoqiyursm1l9uZ3mMQ/peUsdCE 0xJilVian+FGdtNcvtauU4PTADIzYopJ3DGe5hjpU/yttdt1bH2jpzaOBbWrBCYqZ6os XSWg== X-Gm-Message-State: AC+VfDx9VW14uEEO5IENlggh7qk0dV/w9VCzvHsRBwwGtdOYzMsGL/GC yQvDtEd+n3J16/IMKBDNxnGC3D2qajzu7I47PyumydaXAKm5/OsnZSQ2mE2xZfQhnHqlTC8Q/NI 6JmYScJu1OOE= X-Received: by 2002:a7b:cd15:0:b0:3f6:787:5e53 with SMTP id f21-20020a7bcd15000000b003f607875e53mr3581110wmj.20.1685519504181; Wed, 31 May 2023 00:51:44 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6bCBqd3WX5AOMaN61sImcYYeJ+NBjDRzwYuiEBhSn/dqrErSjURByTklhdc1d09OgLqsdFwA== X-Received: by 2002:a7b:cd15:0:b0:3f6:787:5e53 with SMTP id f21-20020a7bcd15000000b003f607875e53mr3581083wmj.20.1685519503784; Wed, 31 May 2023 00:51:43 -0700 (PDT) Received: from ?IPV6:2003:cb:c749:cb00:fc9f:d303:d4cc:9f26? (p200300cbc749cb00fc9fd303d4cc9f26.dip0.t-ipconnect.de. [2003:cb:c749:cb00:fc9f:d303:d4cc:9f26]) by smtp.gmail.com with ESMTPSA id y6-20020a05600c364600b003f420667807sm23170451wmq.11.2023.05.31.00.51.42 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 31 May 2023 00:51:43 -0700 (PDT) Message-ID: <499e30cc-d015-8353-1364-50d17da58f47@redhat.com> Date: Wed, 31 May 2023 09:51:41 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 Subject: Re: [PATCH 2/2] module: add support to avoid duplicates early on load To: Luis Chamberlain , Linus Torvalds Cc: Johan Hovold , Lucas De Marchi , Petr Pavlu , gregkh@linuxfoundation.org, rafael@kernel.org, song@kernel.org, lucas.de.marchi@gmail.com, christophe.leroy@csgroup.eu, peterz@infradead.org, rppt@kernel.org, dave@stgolabs.net, willy@infradead.org, vbabka@suse.cz, mhocko@suse.com, dave.hansen@linux.intel.com, colin.i.king@gmail.com, jim.cromie@gmail.com, catalin.marinas@arm.com, jbaron@akamai.com, rick.p.edgecombe@intel.com, yujie.liu@intel.com, tglx@linutronix.de, hch@lst.de, patches@lists.linux.dev, linux-modules@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, pmladek@suse.com, prarit@redhat.com, lennart@poettering.net References: <6gwjomw6sxxmlglxfoilelswv4hgygqelomevb4k4wrlrk3gtm@wrakbmwztgeu> From: David Hildenbrand Organization: Red Hat In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 5E7CB14000F X-Stat-Signature: hw7gsjozhob3yiwpdx5yhy7haif5tjuu X-HE-Tag: 1685519507-968755 X-HE-Meta: U2FsdGVkX1/30Dv5pUfiHh/nGZluu+d+2C6aNg9bZSQiCghrx+Eli3sXPqCe0lRTUSY1Z2+MHXHO+cujd6jJN/IzkKe0GOsGumY/b9ZNRpmA2R4Sb7d+u3+o07XvEyHdfZU1SE7uXDHjrmUvy9TnGztomGCdOjmJBhGxGNDcYYh+a4cgaDKql87FWTC6x8/hfs4sxEr3vENpepXA92TaPJxFKefDm9H2HryaYHHQ+dYhp4T9MUDngSZbhxi/pssR3sF7/g75gV3OORogfgCE1G25e5FIgBu1VZgthRmE7B3vkx/mKvzdIFF8j2gFVBnOmHs4D74j+EjOy6556wFUSC/5R5+2pQOaPhnPxkaUxg1r5A9Q/l9/AqcSPq0RP6YTsQqYwpaPaTiyaEkVYxuUSN/O3k11dSfGqAF9Kbpakyv+mBQHwlQIC3gFNhjAbKEWeSAYG4hCoGlx64GqwIXY5JWBMOdgwhY/s2Sbz2RERoyScYF2B60M8499m0w83DKd0QscphxpS/U4cH5O6/peFRUhyHmOD3tsf0vXNkY8Te5YGpEgfvhXlkDVQTPZauTRMLOdyjP2HRtLjnpwUxnmq8CLl9Xie6E4fOg6vxT0fJYu97VP7eL2Q/sDv3HzXpgfCU1kceULoqED8ieacsmYFj7sQj/S91gcGY/IoG6iS1giFAaFoWgTNbw5eK8ZEr4cqgpHU1hrFmhM9FYim4uZdNPddzM5EX5vVwdktx3XdIc3NnkOqVvSvy1QFpEIYH4bGl+oULlpHiLD70///6o7REY/SqvSfAYw5aANfrgCtROYL9rXeowxYBLz0xyEwecJl303sKfSCfOOIUCWa7848YTiWyQZPgR7eaV5dIItgV/lg+Ski1AOMcp6LS30aXn1veC23hJqtd3DjfHV2kUi2+sRBsB2mDiaFXHgfsA0dcONgKfdfTwr7Y8waA67yMpEROLmmWDpg1ZnB+5fXKN SVGZFV4T 2kH3w0LZ5zwUwybWyw4Bw+f8zj+1KyfPUIQcpQAh5xC2MB+KGu8fvR8sB7e0W8iPXJ8kbamL8m/signoW1dskwlqMHcG8JfsFgpWQPxl4cJKoD6PsuO6ic0jktceCxSmK6vmLUTvNACY/4V459XuXVNOV8Gge/xINXzCvvAjSF1NyWetCwsEA0u7aQQJ/wy0kkZxH9Gs9Y6349ugRKez3txX9ZpOocxbnY7UNM6sgmj9mRY0Q9FHk4G4RaBH7EZ1Fe2rTY74pArfLmSaRN41W1+/ufjqq06FQpmnF73aXVKxbE6urp4iJ+K3rB4TagxAW+nJMg8mtVN3pOjJgaP1KTEc7HgxrEg4HKdC5aJExtgkkTDmoukxoGLDuNsnHplqjLddO9Q1+f+sOv6u8bTbae+/i3RaWMS2P62Nn1T/zGICbU8NlqEwb6JkJuuvFr9KGmE7ezHOu+K3aMwuZpBrkOTGUCK/7TPHJw/ccyibpCgfJNZhzL9/DkQ2HdY6sMx2EcvK0Ze1Jn1XMCrRXFxw+KfwTw5M5svkxpv9/ 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: On 30.05.23 18:22, Luis Chamberlain wrote: > On Mon, May 29, 2023 at 09:55:15PM -0400, Linus Torvalds wrote: >> On Mon, May 29, 2023 at 11:18 AM Johan Hovold wrote: >>> >>> I took a closer look at some of the modules that failed to load and >>> noticed a pattern in that they have dependencies that are needed by more >>> than one device. >> >> Ok, this is a "maybe something like this" RFC series of two patches - >> one trivial one to re-organize things a bit so that we can then do the >> real one which uses a filter based on the inode pointer to return an >> "idempotent return value" for module loads that share the same inode. >> >> It's entirely untested, and since I'm on the road I'm going to not >> really be able to test it. It compiles for me, and the code looks >> fairly straightforward, but it's probably buggy. >> >> It's very loosely based on Luis' attempt, but it >> (a) is internal to module loading >> (b) uses a reliable cookie >> (c) doesn't leave the cookie around randomly for later >> (d) has seen absolutely no testing >> >> Put another way: if somebody wants to play with this, please treat it >> as a starting point, not the final thing. You might need to debug >> things, and fix silly mistakes. >> >> The idea is to just have a simple hash list of currently executing >> module loads, protected by a trivial spinlock. Every module loader >> adds itself to the right hash list, and if they were the *first* one >> (ie no other pending module loads for that inode), will actually do >> the module load. >> >> Everybody who *isn't* the first one will just wait for completion and >> return the same error code that the first one returned. > > That's also a hell much more snazzier MODULE_DEBUG_AUTOLOAD_DUPS if we > ever wanted to do something similar there if we wanted to also > join request_module() calls, instead of it hiding under debug. > >> This is technically bogus. The first one might fail due to arguments. > > For boot it's fine, as I can't think of boot wanting to support trying > to load a module with different arguments but who knows. But I can't > see it sensible to issue concurrent multiple requests for modules > with different arguments without waiting in userspace for the first > to fail. > > Even post-boot, doing that sounds rather insane, but it would certainly > be a compromise and should probably be clearly documented. I think just > a comment acknolwedging that corner case seems sensible. > > Because we won't be able to get the arguments until we process the > module, so it would be too late for this optimization on kread. So it is > why I had also stuck to the original feature being in kread, as then it > provides a uniq kread call and the caller is aware of it. But indeed I > had not considered the effects of arguments. > > Lucas, any thoughts from modules kmod userspace perspective into > supporting anyone likely issuing concurrent modules requests with > differing arguments? > >> So the cookie shouldn't be just the inode, it should be the inode and >> a hash of the arguments or something like that. > > Personally I think it's a fine optimization without the arguments. > >> But it is what it is, >> and apart from possible show-stopper bugs this is no worse than the >> failed "exclusive write deny" attempt. IOW - maybe worth trying? > > The only thing I can think of is allowing threads other than the > first one to complete before the one that actually loaded the > module. I thought about this race for module auto-loading, see > the comment in kmod_dup_request_announce(), so that just > further delays the completion to other thread with a stupid > queue_work(). That seems more important for module auto-loading > duplicates than for boot finit_module() duplicates. But not sure > if odering matters in the end due to a preemtible kernel and maybe > that concern is hysteria. > >> And if *that* didn't sell people on this patch series, I don't know >> what will. I should be in marketing! Two drink minimums, here I come! > > Sold: > > on 255 vcpus 0 duplicates found with this setup: > > root@kmod ~ # cat /sys/kernel/debug/modules/stats > Mods ever loaded 66 > Mods failed on kread 0 > Mods failed on decompress 0 > Mods failed on becoming 0 > Mods failed on load 0 > Total module size 11268096 > Total mod text size 4149248 > Failed kread bytes 0 > Failed decompress bytes 0 > Failed becoming bytes 0 > Failed kmod bytes 0 > Virtual mem wasted bytes 0 > Average mod size 170729 > Average mod text size 62868 > > So: > > Tested-by: Luis Chamberlain > > In terms of bootup timing: > > Before: > Startup finished in 41.653s (kernel) + 44.305s (userspace) = 1min 25.958s > graphical.target reached after 44.178s in userspace. > > After: > Startup finished in 23.995s (kernel) + 40.350s (userspace) = 1min 4.345s > graphical.target reached after 40.226s in userspace. I'll try grabbing the system where we saw the KASAN-related issues [1] and give it a churn with and without the two patches. Might take a bit (~1 day), unfortunately. [1] https://lkml.kernel.org/r/20221013180518.217405-1-david@redhat.com -- Thanks, David / dhildenb