From patchwork Wed Apr 5 20:35:04 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Chamberlain X-Patchwork-Id: 13202510 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3C180C77B62 for ; Wed, 5 Apr 2023 20:35:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233103AbjDEUfW (ORCPT ); Wed, 5 Apr 2023 16:35:22 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52446 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230228AbjDEUfT (ORCPT ); Wed, 5 Apr 2023 16:35:19 -0400 Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D12457282; Wed, 5 Apr 2023 13:35:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=TqXpp0mGR6pbQr82UrkZafoUwwcLEWn2SwPyhW5ccH4=; b=X5xdE8rLMlmIhymO9yURWuCRx9 w1LgYZs4/coLMo59/hY71k4+rqnnXzqaLc5HYdKHqVHeoeTjhEf7wUc+AnskcobUNmGIMxBiv8Nn8 4fequI0umG+GJm1tyfsebfRL+VKkmKOxEwafCKr8gSWS1hpUZbDjeFqnf5ue3DwhmTAMrdbOYR7Vs 1us/UOeklOrrDnWWlQ0mwgKBLyIuyRsaCVXegU/FjJbxeXni04d40TF9X1ydJQypCAQlOZPMknDfW luZOgQOsCu0wq15gDwNE23B8ohPubiurmXTgY3jEVQMDaFCaX4lsgzS0gc8tNvC4H8mXH5q3iqLZM 4fHLE72w==; Received: from mcgrof by bombadil.infradead.org with local (Exim 4.96 #2 (Red Hat Linux)) id 1pk9qT-005dWa-2g; Wed, 05 Apr 2023 20:35:05 +0000 From: Luis Chamberlain To: 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, gregkh@linuxfoundation.org, rafael@kernel.org Cc: 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, mcgrof@kernel.org Subject: [PATCH v2 1/2] Change DEFINE_SEMAPHORE() to take a number argument Date: Wed, 5 Apr 2023 13:35:04 -0700 Message-Id: <20230405203505.1343562-2-mcgrof@kernel.org> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20230405203505.1343562-1-mcgrof@kernel.org> References: <20230405203505.1343562-1-mcgrof@kernel.org> MIME-Version: 1.0 Sender: Luis Chamberlain Precedence: bulk List-ID: From: Peter Zijlstra Fundamentally semaphores are a counted primitive, but DEFINE_SEMAPHORE() does not expose this and explicitly creates a binary semaphore. Change DEFINE_SEMAPHORE() to take a number argument and use that in the few places that open-coded it using __SEMAPHORE_INITIALIZER(). Signed-off-by: Peter Zijlstra (Intel) [mcgrof: add some tribal knowledge about why some folks prefer binary sempahores over mutexes] Signed-off-by: Luis Chamberlain Reviewed-by: Davidlohr Bueso Reviewed-by: Sergey Senozhatsky # printk --- arch/mips/cavium-octeon/setup.c | 2 +- arch/x86/kernel/cpu/intel.c | 2 +- drivers/firmware/efi/runtime-wrappers.c | 2 +- drivers/firmware/efi/vars.c | 2 +- drivers/macintosh/adb.c | 2 +- drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 2 +- drivers/platform/x86/intel/ifs/sysfs.c | 2 +- drivers/scsi/esas2r/esas2r_ioctl.c | 2 +- .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 2 +- include/linux/semaphore.h | 11 +++++++++-- kernel/printk/printk.c | 2 +- net/rxrpc/call_object.c | 6 ++---- 12 files changed, 21 insertions(+), 16 deletions(-) diff --git a/arch/mips/cavium-octeon/setup.c b/arch/mips/cavium-octeon/setup.c index a71727f7a608..c5561016f577 100644 --- a/arch/mips/cavium-octeon/setup.c +++ b/arch/mips/cavium-octeon/setup.c @@ -72,7 +72,7 @@ extern void pci_console_init(const char *arg); static unsigned long long max_memory = ULLONG_MAX; static unsigned long long reserve_low_mem; -DEFINE_SEMAPHORE(octeon_bootbus_sem); +DEFINE_SEMAPHORE(octeon_bootbus_sem, 1); EXPORT_SYMBOL(octeon_bootbus_sem); static struct octeon_boot_descriptor *octeon_boot_desc_ptr; diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c index 291d4167fab8..12bad63822f0 100644 --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -1177,7 +1177,7 @@ static const struct { static struct ratelimit_state bld_ratelimit; static unsigned int sysctl_sld_mitigate = 1; -static DEFINE_SEMAPHORE(buslock_sem); +static DEFINE_SEMAPHORE(buslock_sem, 1); #ifdef CONFIG_PROC_SYSCTL static struct ctl_table sld_sysctls[] = { diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c index 1fba4e09cdcf..a400c4312c82 100644 --- a/drivers/firmware/efi/runtime-wrappers.c +++ b/drivers/firmware/efi/runtime-wrappers.c @@ -158,7 +158,7 @@ void efi_call_virt_check_flags(unsigned long flags, const char *call) * none of the remaining functions are actually ever called at runtime. * So let's just use a single lock to serialize all Runtime Services calls. */ -static DEFINE_SEMAPHORE(efi_runtime_lock); +static DEFINE_SEMAPHORE(efi_runtime_lock, 1); /* * Expose the EFI runtime lock to the UV platform diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c index bd75b87f5fc1..bfc5fa6aa47b 100644 --- a/drivers/firmware/efi/vars.c +++ b/drivers/firmware/efi/vars.c @@ -21,7 +21,7 @@ /* Private pointer to registered efivars */ static struct efivars *__efivars; -static DEFINE_SEMAPHORE(efivars_lock); +static DEFINE_SEMAPHORE(efivars_lock, 1); static efi_status_t check_var_size(bool nonblocking, u32 attributes, unsigned long size) diff --git a/drivers/macintosh/adb.c b/drivers/macintosh/adb.c index 23bd0c77ac1a..56599515d51a 100644 --- a/drivers/macintosh/adb.c +++ b/drivers/macintosh/adb.c @@ -80,7 +80,7 @@ static struct adb_driver *adb_controller; BLOCKING_NOTIFIER_HEAD(adb_client_list); static int adb_got_sleep; static int adb_inited; -static DEFINE_SEMAPHORE(adb_probe_mutex); +static DEFINE_SEMAPHORE(adb_probe_mutex, 1); static int sleepy_trackpad; static int autopoll_devs; int __adb_probe_sync; diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c index 5d1e4fe335aa..5a105bab4387 100644 --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c @@ -298,7 +298,7 @@ const u32 dmae_reg_go_c[] = { /* Global resources for unloading a previously loaded device */ #define BNX2X_PREV_WAIT_NEEDED 1 -static DEFINE_SEMAPHORE(bnx2x_prev_sem); +static DEFINE_SEMAPHORE(bnx2x_prev_sem, 1); static LIST_HEAD(bnx2x_prev_list); /* Forward declaration */ diff --git a/drivers/platform/x86/intel/ifs/sysfs.c b/drivers/platform/x86/intel/ifs/sysfs.c index ee636a76b083..4c3c642ee19a 100644 --- a/drivers/platform/x86/intel/ifs/sysfs.c +++ b/drivers/platform/x86/intel/ifs/sysfs.c @@ -13,7 +13,7 @@ * Protects against simultaneous tests on multiple cores, or * reloading can file while a test is in progress */ -static DEFINE_SEMAPHORE(ifs_sem); +static DEFINE_SEMAPHORE(ifs_sem, 1); /* * The sysfs interface to check additional details of last test diff --git a/drivers/scsi/esas2r/esas2r_ioctl.c b/drivers/scsi/esas2r/esas2r_ioctl.c index e003d923acbf..055d2e87a2c8 100644 --- a/drivers/scsi/esas2r/esas2r_ioctl.c +++ b/drivers/scsi/esas2r/esas2r_ioctl.c @@ -56,7 +56,7 @@ dma_addr_t esas2r_buffered_ioctl_addr; u32 esas2r_buffered_ioctl_size; struct pci_dev *esas2r_buffered_ioctl_pcid; -static DEFINE_SEMAPHORE(buffered_ioctl_semaphore); +static DEFINE_SEMAPHORE(buffered_ioctl_semaphore, 1); typedef int (*BUFFERED_IOCTL_CALLBACK)(struct esas2r_adapter *, struct esas2r_request *, struct esas2r_sg_context *, diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index cddcd3c596c9..1a656fdc9445 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -149,7 +149,7 @@ static char *g_fragments_base; static char *g_free_fragments; static struct semaphore g_free_fragments_sema; -static DEFINE_SEMAPHORE(g_free_fragments_mutex); +static DEFINE_SEMAPHORE(g_free_fragments_mutex, 1); static int vchiq_blocking_bulk_transfer(struct vchiq_instance *instance, unsigned int handle, void *data, diff --git a/include/linux/semaphore.h b/include/linux/semaphore.h index 6694d0019a68..2d6aa3fd7861 100644 --- a/include/linux/semaphore.h +++ b/include/linux/semaphore.h @@ -25,8 +25,15 @@ struct semaphore { .wait_list = LIST_HEAD_INIT((name).wait_list), \ } -#define DEFINE_SEMAPHORE(name) \ - struct semaphore name = __SEMAPHORE_INITIALIZER(name, 1) +/* + * There is a big difference between a binary semaphore and a mutex. + * You cannot call mutex_unlock() from IRQ context because it takes an + * internal mutex spin_lock in a non-IRQ-safe manner. Both try_lock() + * and unlock() can be called from IRQ context. A mutex must also be + * released in the same context that locked it. + */ +#define DEFINE_SEMAPHORE(_name, _n) \ + struct semaphore _name = __SEMAPHORE_INITIALIZER(_name, _n) static inline void sema_init(struct semaphore *sem, int val) { diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index fd0c9f913940..76987aaa5a45 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -89,7 +89,7 @@ static DEFINE_MUTEX(console_mutex); * console_sem protects updates to console->seq and console_suspended, * and also provides serialization for console printing. */ -static DEFINE_SEMAPHORE(console_sem); +static DEFINE_SEMAPHORE(console_sem, 1); HLIST_HEAD(console_list); EXPORT_SYMBOL_GPL(console_list); DEFINE_STATIC_SRCU(console_srcu); diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c index e9f1f49d18c2..3e5cc70884dd 100644 --- a/net/rxrpc/call_object.c +++ b/net/rxrpc/call_object.c @@ -40,10 +40,8 @@ const char *const rxrpc_call_completions[NR__RXRPC_CALL_COMPLETIONS] = { struct kmem_cache *rxrpc_call_jar; -static struct semaphore rxrpc_call_limiter = - __SEMAPHORE_INITIALIZER(rxrpc_call_limiter, 1000); -static struct semaphore rxrpc_kernel_call_limiter = - __SEMAPHORE_INITIALIZER(rxrpc_kernel_call_limiter, 1000); +static DEFINE_SEMAPHORE(rxrpc_call_limiter, 1000); +static DEFINE_SEMAPHORE(rxrpc_kernel_call_limiter, 1000); void rxrpc_poke_call(struct rxrpc_call *call, enum rxrpc_call_poke_trace what) { From patchwork Wed Apr 5 20:35:05 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Chamberlain X-Patchwork-Id: 13202509 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A44F5C76188 for ; Wed, 5 Apr 2023 20:35:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232739AbjDEUfV (ORCPT ); Wed, 5 Apr 2023 16:35:21 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52366 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233346AbjDEUfO (ORCPT ); Wed, 5 Apr 2023 16:35:14 -0400 Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 702E16A72; Wed, 5 Apr 2023 13:35:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=YNsOcxvyBsREks0tqN6x4wPg4m3MWZPixrfEIPIhg1U=; b=gfUuvSKg4yYImkFYNGmsyBbM9/ 98+9D7ZfdVo5oy0g+qCsN+zRZAdeIcw42D6VL+3B4EETojpzrqXxf2fPwN5IrpuXCzNsChVc8ePOW bzq20BGd/mVGhBbLILr3CvX48WxYtJL9sRJ3Oe1bzyk6981tRGzBPh8znM++Pop3RGrpUwwgqpGNi hczxpBcuxOjTfJsezWUl6mrDXv6j4w6WsSccnsoXFIJ7rVVSqVCKi1gjncinIWeBtXqnM8IMFIUvz 5B5Z79iMSHvBm3Ns+7ZzHWRzMhw7m440onLHSzpBwnCLi0BRl520qUOiTOIWiYocYXHd/dnrb5ALb yTVa8mag==; Received: from mcgrof by bombadil.infradead.org with local (Exim 4.96 #2 (Red Hat Linux)) id 1pk9qT-005dWc-2p; Wed, 05 Apr 2023 20:35:05 +0000 From: Luis Chamberlain To: 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, gregkh@linuxfoundation.org, rafael@kernel.org Cc: 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, mcgrof@kernel.org Subject: [PATCH v2 2/2] modules/kmod: replace implementation with a sempahore Date: Wed, 5 Apr 2023 13:35:05 -0700 Message-Id: <20230405203505.1343562-3-mcgrof@kernel.org> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20230405203505.1343562-1-mcgrof@kernel.org> References: <20230405203505.1343562-1-mcgrof@kernel.org> MIME-Version: 1.0 Sender: Luis Chamberlain Precedence: bulk List-ID: Simplfy the concurrency delimiter we user for kmod with the semaphore. I had used the kmod strategy to try to implement a similar concurrency delimiter for the kernel_read*() calls from the finit_module() path so to reduce vmalloc() memory pressure. That effort didn't provid yet conclusive results, but one thing that did became clear is we can use the suggested alternative solution with semaphores which Linus hinted at instead of using the atomic / wait strategy. I've stress tested this with kmod test 0008: time /data/linux-next/tools/testing/selftests/kmod/kmod.sh -t 0008 And I get only a *slight* delay. That delay however is small, a few seconds for a full test loop run that runs 150 times, for about ~30-40 seconds. The small delay is worth the simplfication IMHO. Signed-off-by: Luis Chamberlain Reviewed-by: Davidlohr Bueso Reviewed-by: David Hildenbrand --- kernel/module/kmod.c | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/kernel/module/kmod.c b/kernel/module/kmod.c index b717134ebe17..925eb85b8346 100644 --- a/kernel/module/kmod.c +++ b/kernel/module/kmod.c @@ -40,8 +40,7 @@ * effect. Systems like these are very unlikely if modules are enabled. */ #define MAX_KMOD_CONCURRENT 50 -static atomic_t kmod_concurrent_max = ATOMIC_INIT(MAX_KMOD_CONCURRENT); -static DECLARE_WAIT_QUEUE_HEAD(kmod_wq); +static DEFINE_SEMAPHORE(kmod_concurrent_max, MAX_KMOD_CONCURRENT); /* * This is a restriction on having *all* MAX_KMOD_CONCURRENT threads @@ -148,29 +147,18 @@ int __request_module(bool wait, const char *fmt, ...) if (ret) return ret; - if (atomic_dec_if_positive(&kmod_concurrent_max) < 0) { - pr_warn_ratelimited("request_module: kmod_concurrent_max (%u) close to 0 (max_modprobes: %u), for module %s, throttling...", - atomic_read(&kmod_concurrent_max), - MAX_KMOD_CONCURRENT, module_name); - ret = wait_event_killable_timeout(kmod_wq, - atomic_dec_if_positive(&kmod_concurrent_max) >= 0, - MAX_KMOD_ALL_BUSY_TIMEOUT * HZ); - if (!ret) { - pr_warn_ratelimited("request_module: modprobe %s cannot be processed, kmod busy with %d threads for more than %d seconds now", - module_name, MAX_KMOD_CONCURRENT, MAX_KMOD_ALL_BUSY_TIMEOUT); - return -ETIME; - } else if (ret == -ERESTARTSYS) { - pr_warn_ratelimited("request_module: sigkill sent for modprobe %s, giving up", module_name); - return ret; - } + ret = down_timeout(&kmod_concurrent_max, MAX_KMOD_ALL_BUSY_TIMEOUT); + if (ret) { + pr_warn_ratelimited("request_module: modprobe %s cannot be processed, kmod busy with %d threads for more than %d seconds now", + module_name, MAX_KMOD_CONCURRENT, MAX_KMOD_ALL_BUSY_TIMEOUT); + return ret; } trace_module_request(module_name, wait, _RET_IP_); ret = call_modprobe(module_name, wait ? UMH_WAIT_PROC : UMH_WAIT_EXEC); - atomic_inc(&kmod_concurrent_max); - wake_up(&kmod_wq); + up(&kmod_concurrent_max); return ret; }