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 08D67C77B7E for ; Sat, 27 May 2023 19:28:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3F848900003; Sat, 27 May 2023 15:28:34 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 38305900002; Sat, 27 May 2023 15:28:34 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1FAAE900003; Sat, 27 May 2023 15:28:34 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 09579900002 for ; Sat, 27 May 2023 15:28:34 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id BB28B1402CA for ; Sat, 27 May 2023 19:28:33 +0000 (UTC) X-FDA: 80837021706.18.72CF614 Received: from mail-wr1-f47.google.com (mail-wr1-f47.google.com [209.85.221.47]) by imf19.hostedemail.com (Postfix) with ESMTP id DDD641A000F for ; Sat, 27 May 2023 19:28:31 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=gmail.com header.s=20221208 header.b=Z519cgcx; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf19.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.221.47 as permitted sender) smtp.mailfrom=lstoakes@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1685215712; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=oY9FsVN+XQNtHrXCb3xqlTXvFOmRY2t/2hvvaIVJnYE=; b=HBsDpLNaXMId1YAv97zKxUimAn2T4C5f0klebUlJQczSTCZZha5nDZRIyRW2UXefF2OQAD xDTLeUYAh0TKnjMtchrAGrF76q4cCpaEW7y3jsUIcR7DHoQwhZYPWyE/ypeyQuvshQ1YRa qfuit4p9leSzPbAJbNuXLlfSrCLbLCQ= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=gmail.com header.s=20221208 header.b=Z519cgcx; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf19.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.221.47 as permitted sender) smtp.mailfrom=lstoakes@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1685215712; a=rsa-sha256; cv=none; b=Sk552Df/VeFo1YuJvkipgJ9Y/YyozJqgkAdJ0qJ8Hir7/zSM7HQ9zPNTn74211euc+kWN3 Ep2jz/do2xorhpktuj+mxWDAx0IpKsztlsLrRThTQwpmj5Ipigm+6/MdeK+oBs2jdaYd/N pQXVDEzIV5MoOeu2+LiPFJnwYgXhStk= Received: by mail-wr1-f47.google.com with SMTP id ffacd0b85a97d-30ae4ec1ac7so285850f8f.2 for ; Sat, 27 May 2023 12:28:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1685215710; x=1687807710; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=oY9FsVN+XQNtHrXCb3xqlTXvFOmRY2t/2hvvaIVJnYE=; b=Z519cgcxeTi9/IGk+aLBU/RoF27mBj/ja9GzU6cU4ViZ2lBpXo9jI+lOMgpMfKqFZW Sq4VDO+P+xmFWJLBtCb/4o4btWt4L9OrjExUPmV/g5uXFOnBhwt0R0bAcRNieo/fkVQY R9rZkFp4dgR5CH1QCPUm2GWfNldn7cks3c+HU9qwGKumaoTfc1x0Fxn7jYA6Otu1gDf/ G9Kd1+lxa/oavDGSeK3qYVBENHEPTIsrtarSRN3BbmQDGTlyZnHbjgY583x031GUwdrg BHJCNUuvdIKZJSJX2pF9V10/4nUxhZg0ZawKiiVQISkGaUUCuu2Vr2WNTHRjsxv3y8eD r/SA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685215710; x=1687807710; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=oY9FsVN+XQNtHrXCb3xqlTXvFOmRY2t/2hvvaIVJnYE=; b=AFFxw10s6JJNpB9xTFjqOBnLsS9r2hp+P4sjITAE4Yq51uNLl6TdoX4Cid1cgwEnOD Rm8OaScV92vymt2Q1Yhliwlj09I9+vyzqnS8IdH1yUgtpFnIU3lUL/+7kH83p6A4ENSx cpLKoccqQDbsYx17hfj2E6xNsp5Iry2LabxwYPFMc2/Jb05MtcNshA/F1xLbrH5q25/q DrLHL4tVksjMyWY7fUXmiEfaecG2IDNbBvlMIIhnzIDF1o7sSj77YYNUfQ67AruaaFHE C8QUWhd0uXSIRIpIhKWTGH3gjWtjKixPb/NX9eqSpg84OvvCLh0XxP8nbIS/E4sV70oI 4CNA== X-Gm-Message-State: AC+VfDwJp/Kl0Zkc6gaDdDmdrLCHYNnAomVQccAGeaGAUh094EH5u8rg Jt1OQah6PEap9a2Rjcf85wWWZgym57A= X-Google-Smtp-Source: ACHHUZ4zj/g7cjnOLXymWXTBXCGo+F/okoy0dL2joY+2A+jGojae0Jb6/aAdBSwZfyhxg8WEB/2yaw== X-Received: by 2002:a05:6000:14d:b0:2ce:d84d:388f with SMTP id r13-20020a056000014d00b002ced84d388fmr4676093wrx.40.1685215710151; Sat, 27 May 2023 12:28:30 -0700 (PDT) Received: from localhost (host81-154-179-160.range81-154.btcentralplus.com. [81.154.179.160]) by smtp.gmail.com with ESMTPSA id j12-20020adff54c000000b0030630de6fbdsm8791498wrp.13.2023.05.27.12.28.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 27 May 2023 12:28:29 -0700 (PDT) Date: Sat, 27 May 2023 20:28:28 +0100 From: Lorenzo Stoakes To: Thomas Gleixner Cc: linux-mm@kvack.org, Andrew Morton , Christoph Hellwig , Uladzislau Rezki , Peter Zijlstra , Baoquan He Subject: Re: [V2 patch 6/6] mm/vmalloc: Dont purge usable blocks unnecessarily Message-ID: <7c646195-ba53-42cb-a8e1-52be65c68830@lucifer.local> References: <20230525122342.109672430@linutronix.de> <20230525124504.864005691@linutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230525124504.864005691@linutronix.de> X-Rspamd-Queue-Id: DDD641A000F X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: c3eac95wafdxsnfqnp48zxja873fb1ec X-HE-Tag: 1685215711-79614 X-HE-Meta: U2FsdGVkX1/QOzrNA/PPRz4gofCsSeklcvriEKsXzvnUT0Ggu5G4VD01hHo/YVBqNeeOmPVOhL04ah4b2RnmdM6FsrCyPCjj/YWDKr2I4mbpYywbHEJeQyafsibXPpKFK4Q1X6BEmi4VVt/wMZuHj8MXNoPMNcofqmIVPSdpVx56UCMdnEjZXBoF6PGos49huPO/CKMx1HsQO7bhiWz1Q5QaHRwBq8EgdrmJIMK4b4bkT8nhOLC5Ubk1Dso62XMFqDjmDhVznbBn/8M1eH4hq85BGFQPnxhOFEwsM8GMY0XIY2ndpjhw3RMsKN+EUV2shmeUT8Ksn97ZXkpQUCtafGej8kvLfafQmi0snPAsvDxKfbZzjnEr3qNAlm98XUXlCWQ5Q6+pXQhmqdVUsguTbhwdbmiHVJ5O9Hzs5FFSLgn7gv4SiH+yBsgsKwAwJvS5i3D7zZtFBAYgr+X4mul3R8hIx3SzID/vs6x8rdFkeMUuNKb3rha4QbWLAqkgJk5O49PytP4MgdYQKkPrqTGAj/ExD9LfSB1bmEHbEQpLrpjnQbVkZ8uClKmFRO2ZXL5Y0lcfsyAbRdmSu5LTlJxWdwNW37xH9gi7kGUx7hjao4q9a7bsO9qZxtLBOEctrw+08eYUPTUViCJUeSrEnhhytxmY4Wnazm9sYK58KG2cUYbpcrgJ60AziT3mNwA7fEY7AIcysxDo2rOxLFs2KGhZfOYuqcPPZs9ZHsH5vwEm5dhBlIsBJfQ4mC119cgBDWYOChnL3T/M+GE5FNNfdF3ue9Kc0qEVO7I5Wvwka/OmF/oU13YRmWaIIJEswkShz5QPwI8ApJ49OQUohu5mdYZPswNIvj27jVfhZdGjYMWQ/ZzwSQaEsIB8flCqxB4ux8HDAOgKaz8X2zIJfJMJ0lKwUr3MQokiPo9A+YnC8It8BPCniPWihCL8gPFQgxMfW9i9StLoUQWwEEWIF+sAcY0 qq1Y5E6v HLs90InkCCFcEDhS6QnoRyQVO30kdRHDkiEkH40pMZADWlDshSXy2qMdlKkQ85O29ayRNiXdUVfEhwcHejmN2gMeW60Zg3YXSjoJVdWlkE5WB+HWTwP1dDheHfD+Vf/ucpqF6rdrpBOgKBfcuIfZJ70ycxIHSF4yrF/+m1VfMXXbkcA+ix5h0fnaYG89Vuv6+xrHiVdGMLQuIWmHsK9KoqqmWqRZvwkwG6wBZHdeWgoBm4h9A7o2iUPIjU1wTaPOsD/ktlIubot17u0wrMpIGyuu5CWuOKHVrOCC+8HllQdUekjq5mrH/21EsCQyY8aBnxLv7qf0reUTgzWEp2VtdRcccMT0kefyAzYtc+XyDXQHfOoO9f6NDkAYJUOAmE1xttv3R1I/ehPMCtOY= 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 Thu, May 25, 2023 at 02:57:09PM +0200, Thomas Gleixner wrote: > Purging fragmented blocks is done unconditionally in several contexts: > > 1) From drain_vmap_area_work(), when the number of lazy to be freed > vmap_areas reached the threshold > > 2) Reclaiming vmalloc address space from pcpu_get_vm_areas() > > 3) _vm_unmap_aliases() > > #1 There is no reason to zap fragmented vmap blocks unconditionally, simply > because reclaiming all lazy areas drains at least > > 32MB * fls(num_online_cpus()) > > per invocation which is plenty. > > #2 Reclaiming when running out of space or due to memory pressure makes a > lot of sense > > #3 _unmap_aliases() requires to touch everything because the caller has no > clue which vmap_area used a particular page last and the vmap_area lost > that information too. > > Except for the vfree + VM_FLUSH_RESET_PERMS case, which removes the > vmap area first and then cares about the flush. That in turn requires > a full walk of _all_ vmap areas including the one which was just > added to the purge list. > > But as this has to be flushed anyway this is an opportunity to combine > outstanding TLB flushes and do the housekeeping of purging freed areas, > but like #1 there is no real good reason to zap usable vmap blocks > unconditionally. > > Add a @force_purge argument to the newly split out block purge function and > if not true only purge fragmented blocks which have less than 1/4 of their > capacity left. > > Rename purge_vmap_area_lazy() to reclaim_and_purge_vmap_areas() to make it > clear what the function does. > > Signed-off-by: Thomas Gleixner > --- > V2: Add the missing force_purge argument in _vm_unmap_aliases() > Remove force_purge argument from the reclaim path - Baoquan > --- > mm/vmalloc.c | 28 ++++++++++++++++++++-------- > 1 file changed, 20 insertions(+), 8 deletions(-) > > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > @@ -791,7 +791,7 @@ get_subtree_max_size(struct rb_node *nod > RB_DECLARE_CALLBACKS_MAX(static, free_vmap_area_rb_augment_cb, > struct vmap_area, rb_node, unsigned long, subtree_max_size, va_size) > > -static void purge_vmap_area_lazy(void); > +static void reclaim_and_purge_vmap_areas(void); > static BLOCKING_NOTIFIER_HEAD(vmap_notify_list); > static void drain_vmap_area_work(struct work_struct *work); > static DECLARE_WORK(drain_vmap_work, drain_vmap_area_work); > @@ -1649,7 +1649,7 @@ static struct vmap_area *alloc_vmap_area > > overflow: > if (!purged) { > - purge_vmap_area_lazy(); > + reclaim_and_purge_vmap_areas(); > purged = 1; > goto retry; > } > @@ -1785,9 +1785,10 @@ static bool __purge_vmap_area_lazy(unsig > } > > /* > - * Kick off a purge of the outstanding lazy areas. > + * Reclaim vmap areas by purging fragmented blocks and purge_vmap_area_list. > */ This comment feels a little redundant as it simply describes the code below. Perhaps worth highlighting that the purge is unconditional and why it would be invoked? Nitty, but I'm also not entirely sure about this use of 'reclaim', as it is rather an overloaded term and you'd expect it to refer to the activities of vmscan.c :) yes we're trying to 'reclaim' vmap areas in a sense but perhaps better to either stick with original or something less overloaded (I would suggest something but naming is hard... :) > -static void purge_vmap_area_lazy(void) > +static void reclaim_and_purge_vmap_areas(void) > + > { > mutex_lock(&vmap_purge_lock); > purge_fragmented_blocks_allcpus(); > @@ -1908,6 +1909,12 @@ static struct vmap_area *find_unlink_vma > > #define VMAP_BLOCK_SIZE (VMAP_BBMAP_BITS * PAGE_SIZE) > > +/* > + * Purge threshold to prevent overeager purging of fragmented blocks for > + * regular operations: Purge if vb->free is less than 1/4 of the capacity. > + */ 'Purge if vb->free is less than 1/4 of the capacity' appears to be contradicted by the actual conditional in purge_fragmented_block() below (see comment below) unless I'm missing something/simply confused :) > +#define VMAP_PURGE_THRESHOLD (VMAP_BBMAP_BITS / 4) > + > #define VMAP_RAM 0x1 /* indicates vm_map_ram area*/ > #define VMAP_BLOCK 0x2 /* mark out the vmap_block sub-type*/ > #define VMAP_FLAGS_MASK 0x3 > @@ -2087,12 +2094,17 @@ static void free_vmap_block(struct vmap_ > } > > static bool purge_fragmented_block(struct vmap_block *vb, > - struct vmap_block_queue *vbq, struct list_head *purge_list) > + struct vmap_block_queue *vbq, struct list_head *purge_list, > + bool force_purge) > { > if (vb->free + vb->dirty != VMAP_BBMAP_BITS || > vb->dirty == VMAP_BBMAP_BITS) > return false; > > + /* Don't overeagerly purge usable blocks unless requested */ > + if (!force_purge && vb->free < VMAP_PURGE_THRESHOLD) Little confused by this - if free capacity is less than the threshold, we _don't_ purge? I thought we should purge in this instance? Should this be an >=? > + return false; > + > /* prevent further allocs after releasing lock */ > WRITE_ONCE(vb->free, 0); > /* prevent purging it again */ > @@ -2132,7 +2144,7 @@ static void purge_fragmented_blocks(int > continue; > > spin_lock(&vb->lock); > - purge_fragmented_block(vb, vbq, &purge); > + purge_fragmented_block(vb, vbq, &purge, true); > spin_unlock(&vb->lock); > } > rcu_read_unlock(); > @@ -2269,7 +2281,7 @@ static void _vm_unmap_aliases(unsigned l > * not purgeable, check whether there is dirty > * space to be flushed. > */ > - if (!purge_fragmented_block(vb, vbq, &purge_list) && > + if (!purge_fragmented_block(vb, vbq, &purge_list, false) && > vb->dirty_max && vb->dirty != VMAP_BBMAP_BITS) { > unsigned long va_start = vb->va->va_start; > unsigned long s, e; > @@ -4175,7 +4187,7 @@ struct vm_struct **pcpu_get_vm_areas(con > overflow: > spin_unlock(&free_vmap_area_lock); > if (!purged) { > - purge_vmap_area_lazy(); > + reclaim_and_purge_vmap_areas(); > purged = true; > > /* Before "retry", check if we recover. */ > >