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 456A1C433F5 for ; Tue, 26 Apr 2022 16:42:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CB53E6B0073; Tue, 26 Apr 2022 12:42:50 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C64D86B0074; Tue, 26 Apr 2022 12:42:50 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B06156B0075; Tue, 26 Apr 2022 12:42:50 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.hostedemail.com [64.99.140.26]) by kanga.kvack.org (Postfix) with ESMTP id A20A86B0073 for ; Tue, 26 Apr 2022 12:42:50 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 7DD8BC93 for ; Tue, 26 Apr 2022 16:42:50 +0000 (UTC) X-FDA: 79399599300.05.F681327 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf03.hostedemail.com (Postfix) with ESMTP id 9617720040 for ; Tue, 26 Apr 2022 16:42:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1650991369; 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=D4Wo1BOUbiwCKjHDQuE2fXDkFkLp6gsv3e+wn4BHxY4=; b=Pd27yqTomkOY9tl4Q+xTBB8d5SpATukSD0GzbqBjk7E7UPJwRXU/kmMQWhNd0Z/a/ewu5L VX8aLH3fnYAJZqX8B+9UtoAxxx1/RtNYsbjb2Y8u+U5bWcvHMgbLyOCyDPSsYMAkYAPIl/ qGlpy95s7YYyEKaZI8a4gUhxo/6LRAM= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-124-lSOkO50GPb6zn_OBe-AHWA-1; Tue, 26 Apr 2022 12:42:48 -0400 X-MC-Unique: lSOkO50GPb6zn_OBe-AHWA-1 Received: by mail-wm1-f70.google.com with SMTP id k16-20020a7bc310000000b0038e6cf00439so1107380wmj.0 for ; Tue, 26 Apr 2022 09:42:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=D4Wo1BOUbiwCKjHDQuE2fXDkFkLp6gsv3e+wn4BHxY4=; b=5Z4rpONs4J340vhEQFSwwG0xRJ9Nm1f265+Jcu7nWiPsqtzcAKkJaoiWIVaLxY2BYN LZX8JptL5b2Kyk766pXUkB9j57wSXhEaFjwSWRCkdzCiaZC7Y+cW8Mn/krF5wANdCUqF 22GRVJA7e2zPx1Ae6wtghW/QK5LQ0tohD7XoN/k9IohG2A3uu8NtiRf0b+IFhMroWAbV MenpA3ZSgn54fSKVNnfEdTUgmZQHWT/vHXsALYpi+rus8B4c+61gmp3PcY2eE7O74s3V WrxdhBY2h2HlTDjLbWQnp4FpLGF3KAOZovvWJhbn0OKAXrQDY3SWyEdpScDkNHjU8vZj cV2Q== X-Gm-Message-State: AOAM5328mUKADOo+xcU1XdBV3sgDOGeilI39sBm7yF7UQWtT1afha8mI HUfDkhPGdfvaO6elikfdTd0adLPm58Xl3odxNF93Ktiz8wIOjK5eIFsYiTlGPAAkcju5Ds3i5Dt BH/gHgS+ic24= X-Received: by 2002:a05:600c:348f:b0:393:dcff:f95b with SMTP id a15-20020a05600c348f00b00393dcfff95bmr19418054wmq.76.1650991366012; Tue, 26 Apr 2022 09:42:46 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzw8SSeU6/it9DsVKT+zwFdAr+l3N0XPyXxX5+e0HOP9IQFGUbD8XN9KzX3ugSn+4PrPHPk3Q== X-Received: by 2002:a05:600c:348f:b0:393:dcff:f95b with SMTP id a15-20020a05600c348f00b00393dcfff95bmr19418039wmq.76.1650991365758; Tue, 26 Apr 2022 09:42:45 -0700 (PDT) Received: from ?IPv6:2a0c:5a80:1306:2f00:cfcf:62cf:6f38:dd92? ([2a0c:5a80:1306:2f00:cfcf:62cf:6f38:dd92]) by smtp.gmail.com with ESMTPSA id r20-20020adfb1d4000000b0020adba3a4d6sm5555643wra.77.2022.04.26.09.42.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 26 Apr 2022 09:42:45 -0700 (PDT) Message-ID: <0d6bf5a97777bec1e0b425f2fb33dbb80d848621.camel@redhat.com> Subject: Re: [PATCH 5/6] mm/page_alloc: Protect PCP lists with a spinlock From: Nicolas Saenz Julienne To: Mel Gorman Cc: Marcelo Tosatti , Vlastimil Babka , Michal Hocko , LKML , Linux-MM Date: Tue, 26 Apr 2022 18:42:43 +0200 In-Reply-To: <20220420095906.27349-6-mgorman@techsingularity.net> References: <20220420095906.27349-1-mgorman@techsingularity.net> <20220420095906.27349-6-mgorman@techsingularity.net> User-Agent: Evolution 3.42.4 (3.42.4-2.fc35) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=Pd27yqTo; spf=none (imf03.hostedemail.com: domain of nsaenzju@redhat.com has no SPF policy when checking 170.10.133.124) smtp.mailfrom=nsaenzju@redhat.com; dmarc=pass (policy=none) header.from=redhat.com X-Rspam-User: X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 9617720040 X-Stat-Signature: 173kabkgtyccooqtzqbo4nq6a5isfae9 X-HE-Tag: 1650991366-247375 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 Wed, 2022-04-20 at 10:59 +0100, Mel Gorman wrote: > Currently the PCP lists are protected by using local_lock_irqsave to > prevent migration and IRQ reentrancy but this is inconvenient. Remote > draining of the lists is impossible and a workqueue is required and > every task allocation/free must disable then enable interrupts which is > expensive. > > As preparation for dealing with both of those problems, protect the > lists with a spinlock. The IRQ-unsafe version of the lock is used > because IRQs are already disabled by local_lock_irqsave. spin_trylock > is used in preparation for a time when local_lock could be used instead > of lock_lock_irqsave. > > The per_cpu_pages still fits within the same number of cache lines after > this patch relative to before the series. > > struct per_cpu_pages { > spinlock_t lock; /* 0 4 */ > int count; /* 4 4 */ > int high; /* 8 4 */ > int batch; /* 12 4 */ > short int free_factor; /* 16 2 */ > short int expire; /* 18 2 */ > > /* XXX 4 bytes hole, try to pack */ > > struct list_head lists[13]; /* 24 208 */ > > /* size: 256, cachelines: 4, members: 7 */ > /* sum members: 228, holes: 1, sum holes: 4 */ > /* padding: 24 */ > } __attribute__((__aligned__(64))); > > Signed-off-by: Mel Gorman > --- > include/linux/mmzone.h | 1 + > mm/page_alloc.c | 155 +++++++++++++++++++++++++++++++++++------ > 2 files changed, 136 insertions(+), 20 deletions(-) > > diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h > index abe530748de6..8b5757735428 100644 > --- a/include/linux/mmzone.h > +++ b/include/linux/mmzone.h > @@ -385,6 +385,7 @@ enum zone_watermarks { > > /* Fields and list protected by pagesets local_lock in page_alloc.c */ > struct per_cpu_pages { > + spinlock_t lock; /* Protects lists field */ > int count; /* number of pages in the list */ > int high; /* high watermark, emptying needed */ > int batch; /* chunk size for buddy add/remove */ > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index dc0fdeb3795c..813c84b67c65 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -132,6 +132,17 @@ static DEFINE_PER_CPU(struct pagesets, pagesets) __maybe_unused = { > .lock = INIT_LOCAL_LOCK(lock), > }; > > +#ifdef CONFIG_SMP > +/* On SMP, spin_trylock is sufficient protection */ > +#define pcp_trylock_prepare(flags) do { } while (0) > +#define pcp_trylock_finish(flag) do { } while (0) > +#else > + > +/* UP spin_trylock always succeeds so disable IRQs to prevent re-entrancy. */ This is only needed on non-RT kernels. RT UP builds go through rt_spin_trylock(), which behaves like its SMP counterpart. > +#define pcp_trylock_prepare(flags) local_irq_save(flags) > +#define pcp_trylock_finish(flags) local_irq_restore(flags) > +#endif > + > #ifdef CONFIG_USE_PERCPU_NUMA_NODE_ID > DEFINE_PER_CPU(int, numa_node); > EXPORT_PER_CPU_SYMBOL(numa_node); > @@ -3082,15 +3093,22 @@ static int rmqueue_bulk(struct zone *zone, unsigned int order, > */ > void drain_zone_pages(struct zone *zone, struct per_cpu_pages *pcp) > { > - unsigned long flags; > int to_drain, batch; > > - local_lock_irqsave(&pagesets.lock, flags); > batch = READ_ONCE(pcp->batch); > to_drain = min(pcp->count, batch); > - if (to_drain > 0) > + if (to_drain > 0) { > + unsigned long flags; > + > + /* free_pcppages_bulk expects IRQs disabled for zone->lock */ > + local_irq_save(flags); Why dropping the local_lock? That approach is nicer to RT builds, and I don't think it makes a difference from a non-RT perspective. That said, IIUC, this will eventually disappear with subsequent patches, right? > + > + spin_lock(&pcp->lock); > free_pcppages_bulk(zone, to_drain, pcp, 0); > - local_unlock_irqrestore(&pagesets.lock, flags); > + spin_unlock(&pcp->lock); > + > + local_irq_restore(flags); > + } > } > #endif > [...] > @@ -3668,9 +3748,30 @@ struct page *__rmqueue_pcplist(struct zone *zone, unsigned int order, > int migratetype, > unsigned int alloc_flags, > struct per_cpu_pages *pcp, > - struct list_head *list) > + struct list_head *list, > + bool locked) > { > struct page *page; > + unsigned long __maybe_unused UP_flags; > + > + /* > + * spin_trylock is not necessary right now due to due to > + * local_lock_irqsave and is a preparation step for > + * a conversion to local_lock using the trylock to prevent > + * IRQ re-entrancy. If pcp->lock cannot be acquired, the caller > + * uses rmqueue_buddy. > + * > + * TODO: Convert local_lock_irqsave to local_lock. Care > + * is needed as the type of local_lock would need a > + * PREEMPT_RT version due to threaded IRQs. > + */ I missed this in our previous conversation, but as far as RT is concerned local_lock_irqsave() is the same as local_lock(), see in local_lock_internal.h: #define __local_lock_irqsave(lock, flags) \ do { \ typecheck(unsigned long, flags); \ flags = 0; \ __local_lock(lock); \ } while (0) Something similar happens with spin_lock_irqsave(), see in spinlock_rt.h: #define spin_lock_irqsave(lock, flags) \ do { \ typecheck(unsigned long, flags); \ flags = 0; \ spin_lock(lock); \ } while (0) IIUC, RT will run this code paths in threaded IRQ context, where we can think of RT spinlocks as mutexes (plus some extra priority inheritance magic). Regards, -- Nicolás Sáenz