From: Uladzislau Rezki <urezki@gmail.com>
To: Manfred Spraul <manfred@colorfullife.com>
Cc: Matthew Wilcox <willy@infradead.org>,
Manfred Spraul <manfred@colorfullife.com>,
LKML <linux-kernel@vger.kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Vasily Averin <vvs@virtuozzo.com>,
cgel.zte@gmail.com, shakeelb@google.com, rdunlap@infradead.org,
dbueso@suse.de, unixbhaskar@gmail.com, chi.minghao@zte.com.cn,
arnd@arndb.de, Zeal Robot <zealci@zte.com.cn>,
linux-mm@kvack.org, 1vier1@web.de, stable@vger.kernel.org
Subject: Re: [PATCH] mm/util.c: Make kvfree() safe for calling while holding spinlocks
Date: Tue, 28 Dec 2021 20:45:22 +0100 [thread overview]
Message-ID: <YctpUurav74Ir5YS@pc638.lan> (raw)
In-Reply-To: <Ycis/J1U2DB6Zx7j@pc638.lan>
On Sun, Dec 26, 2021 at 06:57:16PM +0100, Uladzislau Rezki wrote:
> On Sat, Dec 25, 2021 at 10:58:29PM +0000, Matthew Wilcox wrote:
> > On Sat, Dec 25, 2021 at 07:54:12PM +0100, Uladzislau Rezki wrote:
> > > +static void drain_vmap_area(struct work_struct *work)
> > > +{
> > > + if (mutex_trylock(&vmap_purge_lock)) {
> > > + __purge_vmap_area_lazy(ULONG_MAX, 0);
> > > + mutex_unlock(&vmap_purge_lock);
> > > + }
> > > +}
> > > +
> > > +static DECLARE_WORK(drain_vmap_area_work, drain_vmap_area);
> >
> > Presuambly if the worker fails to get the mutex, it should reschedule
> > itself? And should it even trylock or just always lock?
> >
> mutex_trylock() has no sense here. It should just always get the lock.
> Otherwise we can miss the point to purge. Agree with your opinion.
>
Below the patch that address Matthew's points:
<snip>
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index d2a00ad4e1dd..b82db44fea60 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1717,17 +1717,10 @@ static bool __purge_vmap_area_lazy(unsigned long start, unsigned long end)
return true;
}
-/*
- * Kick off a purge of the outstanding lazy areas. Don't bother if somebody
- * is already purging.
- */
-static void try_purge_vmap_area_lazy(void)
-{
- if (mutex_trylock(&vmap_purge_lock)) {
- __purge_vmap_area_lazy(ULONG_MAX, 0);
- mutex_unlock(&vmap_purge_lock);
- }
-}
+static void purge_vmap_area_lazy(void);
+static void drain_vmap_area(struct work_struct *work);
+static DECLARE_WORK(drain_vmap_area_work, drain_vmap_area);
+static atomic_t drain_vmap_area_work_in_progress;
/*
* Kick off a purge of the outstanding lazy areas.
@@ -1740,6 +1733,22 @@ static void purge_vmap_area_lazy(void)
mutex_unlock(&vmap_purge_lock);
}
+static void drain_vmap_area(struct work_struct *work)
+{
+ mutex_lock(&vmap_purge_lock);
+ __purge_vmap_area_lazy(ULONG_MAX, 0);
+ mutex_unlock(&vmap_purge_lock);
+
+ /*
+ * Check if rearming is still required. If not, we are
+ * done and can let a next caller to initiate a new drain.
+ */
+ if (atomic_long_read(&vmap_lazy_nr) > lazy_max_pages())
+ schedule_work(&drain_vmap_area_work);
+ else
+ atomic_set(&drain_vmap_area_work_in_progress, 0);
+}
+
/*
* Free a vmap area, caller ensuring that the area has been unmapped
* and flush_cache_vunmap had been called for the correct range
@@ -1766,7 +1775,8 @@ static void free_vmap_area_noflush(struct vmap_area *va)
/* After this point, we may free va at any time */
if (unlikely(nr_lazy > lazy_max_pages()))
- try_purge_vmap_area_lazy();
+ if (!atomic_xchg(&drain_vmap_area_work_in_progress, 1))
+ schedule_work(&drain_vmap_area_work);
}
/*
<snip>
Manfred, could you please have a look and if you have a time test it?
I mean if it solves your issue. You can take over this patch and resend
it, otherwise i can send it myself later if we all agree with it.
--
Vlad Rezki
next prev parent reply other threads:[~2021-12-28 19:45 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-22 19:48 [PATCH] mm/util.c: Make kvfree() safe for calling while holding spinlocks Manfred Spraul
2021-12-23 3:40 ` Davidlohr Bueso
2021-12-23 7:21 ` Vasily Averin
2021-12-23 11:52 ` Manfred Spraul
2021-12-23 12:34 ` Vasily Averin
2021-12-25 18:54 ` Uladzislau Rezki
2021-12-25 22:58 ` Matthew Wilcox
2021-12-26 17:57 ` Uladzislau Rezki
2021-12-28 19:45 ` Uladzislau Rezki [this message]
2021-12-28 20:04 ` Manfred Spraul
2021-12-28 20:26 ` Uladzislau Rezki
2022-01-27 2:53 ` Andrew Morton
2022-01-27 5:59 ` Manfred Spraul
2022-01-27 8:25 ` Michal Hocko
2022-01-27 15:54 ` Uladzislau Rezki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YctpUurav74Ir5YS@pc638.lan \
--to=urezki@gmail.com \
--cc=1vier1@web.de \
--cc=akpm@linux-foundation.org \
--cc=arnd@arndb.de \
--cc=cgel.zte@gmail.com \
--cc=chi.minghao@zte.com.cn \
--cc=dbueso@suse.de \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=manfred@colorfullife.com \
--cc=rdunlap@infradead.org \
--cc=shakeelb@google.com \
--cc=stable@vger.kernel.org \
--cc=unixbhaskar@gmail.com \
--cc=vvs@virtuozzo.com \
--cc=willy@infradead.org \
--cc=zealci@zte.com.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).