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 858A1C77B78 for ; Fri, 21 Apr 2023 18:45:40 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C3A846B0074; Fri, 21 Apr 2023 14:45:39 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BE9326B0075; Fri, 21 Apr 2023 14:45:39 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AB16A6B0078; Fri, 21 Apr 2023 14:45:39 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 991936B0074 for ; Fri, 21 Apr 2023 14:45:39 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 67D73405AF for ; Fri, 21 Apr 2023 18:45:39 +0000 (UTC) X-FDA: 80706276798.02.087C7E3 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) by imf12.hostedemail.com (Postfix) with ESMTP id D872F4000C for ; Fri, 21 Apr 2023 18:45:34 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=infradead.org header.s=bombadil.20210309 header.b=osGw+QeX; spf=none (imf12.hostedemail.com: domain of mcgrof@infradead.org has no SPF policy when checking 198.137.202.133) smtp.mailfrom=mcgrof@infradead.org; dmarc=fail reason="No valid SPF, DKIM not aligned (relaxed)" header.from=kernel.org (policy=none) ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1682102736; h=from:from:sender: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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=F45bDSwjMQFBIyyPup/mzrLU83zDLKudjeMCgm+n9uU=; b=nS5KVzsbU6bqIKOB4vWGPRcYlV/4SpY9hO0tQ3n+Fu9e33l0ah+fDcrrmJrvI73SgCVqS1 KcVWEK6MAQdkblsH13ojm87bAdA+BgM0iR+oBnAYl22O3k+8CtLSF/St2LRlRG8gbiQWN1 0HX1/7XtpPDzY9+SfXyCzR8oSTq874s= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=infradead.org header.s=bombadil.20210309 header.b=osGw+QeX; spf=none (imf12.hostedemail.com: domain of mcgrof@infradead.org has no SPF policy when checking 198.137.202.133) smtp.mailfrom=mcgrof@infradead.org; dmarc=fail reason="No valid SPF, DKIM not aligned (relaxed)" header.from=kernel.org (policy=none) ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1682102736; a=rsa-sha256; cv=none; b=g6+efcJxrdcKIafc5VmT4567KbUqU+kHfznzKNM2WQU+0yVDwZRuFSdBLjSMfEi0u9LFBz 5kOg6Ho2GTd0xuH4L3KF7J4FZFB4Ni87+QRkM7ANKhBSdqVqy1Ar23mgaUJXah94BhK/3i zd42ucah7ATIR8SoOV1sf8YXTPJ37Qk= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=F45bDSwjMQFBIyyPup/mzrLU83zDLKudjeMCgm+n9uU=; b=osGw+QeXAhyfhx4s/aSHya/bVu 4MjNm8ps5IYkBaiFFjBjwFyRkjHsdsqH1ofkZhYknco9iUWB8pYz9fZtT1FPwofmLmMafJguE/eGm AKAc0eHxwCgQRgCy377BMO4Lv5EY3zGM5M61Wv3KiEqQtl+6X3Jizlaclq/O3X0LMTCmxsI5gjXtR T0JNl67yNMfvAmKM+h8MGGRjhSb9zhYOxSmLaz7wv60fNVFQpAKg3hncCtonVN/9HhR650Npsrwuc Z4das1bJxPpMFfZsyvr11CkamiHmCBLxaaa+KTvP9OJoXrgYYwe/uNwohNG0PxAMFI0697PKiFzw5 lLVeDllQ==; Received: from mcgrof by bombadil.infradead.org with local (Exim 4.96 #2 (Red Hat Linux)) id 1ppvl4-00BeWM-1x; Fri, 21 Apr 2023 18:45:22 +0000 Date: Fri, 21 Apr 2023 11:45:22 -0700 From: Luis Chamberlain To: Lucas De Marchi Cc: Greg KH , david@redhat.com, patches@lists.linux.dev, linux-modules@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, pmladek@suse.com, petr.pavlu@suse.com, prarit@redhat.com, torvalds@linux-foundation.org, rafael@kernel.org, christophe.leroy@csgroup.eu, tglx@linutronix.de, peterz@infradead.org, song@kernel.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, j.granados@samsung.com Subject: Re: [PATCH] module: add debugging auto-load duplicate module support Message-ID: References: <20230418204636.791699-1-mcgrof@kernel.org> <2023041951-evolution-unwitting-1791@gregkh> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam03 X-Stat-Signature: 6qksbfgih49gicsy9beg5p5wa3ne515t X-Rspamd-Queue-Id: D872F4000C X-HE-Tag: 1682102734-975037 X-HE-Meta: U2FsdGVkX1/2sTl/KJO/WaOva9oc2LY9aNOwNE0h7zdURhMt1CYCvCb19vDwQSrtb9MWGm1XFDpc/hUHF8NW/oqqgvgaIoNgHuqa+m39EjWpbmRy3biXCvrUVEzh9ZysCpIPjo29+s/KMuMm0k4EXYJo8Naud4VmB+Z5ccMR68iQABvRR/fjj4dNcxhgN0Vg82jQ6xmyM0P0xQf1AlaHo2H09FRgBZtOFMMXAo3xcRPFuo9T9YG2Omusr9yGFn4qTKpq7ShYFjTbwfminV6gXp171Zi9daGoTN7Vf4/roz79V5PLDcP0e1gacbHEi15bPu8umOgcFO7sgLHdttTCJk9q2ZvHOf+Q3L3HWph2KqdcJzxKqrL7/XULX3O7GdgUwc8CRo++/g5N4Y6bQIeYaRq/999w5AgjF5lVJGyfhBl1QYGTRSyuQ0TLtJ5t6NRSrxdhwU3Nf6vaTsizDoJfxYc8GF+6Y+m/MIK3H5e08GjbuOUEkgS36R4xuP6SNHvbZRHhm2NJtmd29Qjw//wi/6qCNGyG5vXqBI7XaPS9nwFXdcBlY/2TKJE1w5cRj61cVgDL0YaPGY4r3d48d3CkGZGl04UABeR35pFmgKjNWK7seixnK/QaQvwNVq1TotjB6ziKn8pEBZHImtC1QYLmsaRn0ZDLNscsHUG6p5paYse6M5KgzZaUxkDdNyZeZK3EDVxJRcD26sT+EjPycdsf3AFhp7tvvLfUvE7VHgh7SFRPPV8cVPYIER3VxCJn9Jkj3Womu7ywmDlb1vCrXPAXaH+ST6b+fc19/dbEPc/+iotJM8OIaLIk2XfyZCFDp9JlmcB+wLOx90xZJNyf1Q8O1fv661X0Mvd+WmCZ6TjDu+33W3EN2KzySWbTt5eexyk8uz4lQi2LHWeGJ72NNM6p2eQGYljnmA8jzgYnNL8uh0kZ0zK8AjRJ1uqkibBvvL2GL01izEO1jM9qhZe+oQt ZLXTNFdT ysxTKuvBPdVJr+MpWTyzNpAg9CFyxsgBoyDan1hxUWJikLZpk5Yw4XhSrbFOdN9tY09lJ0N/i5C71S7qkgPGg0TCt5H1oQNdsD/tz+v6frfyduy5vazI0Vg8T/x7z2uAWyncTAlMVJBCLGCypHf+DOTIMaLV+QJ1ZzyeL2fztDcs2GlsYm9/He9QVKbT2rW3gNgioRzCoFQt4k/IDF9HkcdDw0eNPctvNTsewJIGgGg8R1Cmsn8F6wyadgIPL+oT4voJ63ybYPIQ+f5WFHxwbpPM4ZP8PtD1xW1MUvByHML2qKLHqD0hdEHdp2Z5lQrsEqlufxIqorTVqDcN/hy2nZWzZ19mYhtptSmMJI/BmBQ+AOs4bcmRujUE0AzYMw/sSEnfVBEN2PJQNO4uphtMeUyQFNM1dc7okYtRLl24y2fVcBgSNnnSdBm0hmHBfunt1DB9xuty+S+7bwBuVQLFkeCRx4+JkVp8CL7yPohMgfN2Guii0/u5U5L9fzw== 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 Fri, Apr 21, 2023 at 11:31:03AM -0700, Lucas De Marchi wrote: > On Fri, Apr 21, 2023 at 10:38:49AM -0700, Luis Chamberlain wrote: > > Just as with the kludge-of-concept I posted for kread [0], userspace > > also should have similar issues in mapping module name to arbitrary > > file names given: > > > > o a module can be in different paths and libkmod could for > > example at one point load a module in one path, then userspace > > removes it, and the next path is used. > > no, it can't. Unless you are doing out of tree modules and loading them > manually by path. There can only be one module with the same name in kmod's > database. If you have duplicate modules, depmod will use the dir > priority configured by the distro (see depmod.d(5)). > > Since we are talking about *udev* it's not a real possibility as > 1) the udev requests are serialized > 2) there is only 1 kmod ctx, so they use the same configuration, no > funky kmod_new("/another-rootfs", ...) type of thing. > > > o module names may differ from the filename slightly (in the kernel > > we replace dash with "_", refer to KBUILD_MODNAME > > this is taken care by depmod/libkmod too. All the aliases are mapped to > module names and then normalized. See modname_normalize() in kmod. Great! So this should be much simpler in userspace. > > [1] https://lore.kernel.org/all/ZDmAvwi+KNvie+OI@bombadil.infradead.org/T/#md172510af8fdf7e0f76f6caafee9c99f7a8b6de7 > > > > > libkmod only skips the call if the module is already in > > > the live state. > > > > It can do better, it can converge requests to avoid a kernel_read*() > > from using vmalloc space. Note that this was not well known before, > > but now it is clear. > > in userspace, if using the same context and using init_module() rather > than finit_module(), I **guess** we would have a similar thing due to > the memory pool for modules: we don't read the module again. That is not > true for finit_module() though as we just open and pass the fd. I think we could not not care about init_module() races for now. > > I realize though that this could mean sharing a context between all > > loads thoughs in udev, and such a change could take significant time > > and review to complete. > > But there is only one context. There aren't multiple paralell requests > from multiple sources. Probably need to Cc someone still changing > udev's builtin... but from a quick look, from what I remember about > that the last time I touched it and without data to prove me wrong, > it seems we are not looking at the right problem space to come up with a > solution. Data seems to indicate that somehow this might not be true. > > If we *wanted* to do this in kernel instead, I have already shown it's > > not hard. > > > > > It seems systemd-udev also duplicates the check > > > in src/shared/module-util.c:module_load_and_warn() > > > > Evidence is showing that does not suffice for the races which are > > currently possible. > > can you raise the udev verbosity and share? How do I do that? > All the kmod-builtin > calls will already be logged there. See > src/udev/udev-event.c:udev_event_execute_run() leading to > > log_device_debug(event->dev, "Running built-in command \"%s\"", command); > r = udev_builtin_run(event->dev, &event->rtnl, builtin_cmd, command, false); > > if you are rather seeing "Running command", ohh... then your udev was > built without libkmod and it will just fork/exec. Not what we want. I'm using debian testing everything vanilla packages except the kernel, using modules-next. > so it seems the easiest thing to do is collect the udev log. > > hopefully you don't have CONFIG_UEVENT_HELPER_PATH set or anything > mucking /sys/kernel/uevent_helper. Right? No. Luis