Date: Tue, 15 Dec 2020 09:27:39 -0500 From: Mark Johnston <markj@freebsd.org> To: Hans Petter Selasky <hps@selasky.org> Cc: Bryan Drewery <bdrewery@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r368523 - head/sys/vm Message-ID: <X9jH20xQcS72vuDQ@raichu> In-Reply-To: <a5b983ca-73f6-a7eb-7b96-bb361c1a8514@selasky.org> References: <202012102044.0BAKiTHh011767@repo.freebsd.org> <a5b983ca-73f6-a7eb-7b96-bb361c1a8514@selasky.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Dec 15, 2020 at 01:59:22PM +0100, Hans Petter Selasky wrote: > On 12/10/20 9:44 PM, Bryan Drewery wrote: > > Author: bdrewery > > Date: Thu Dec 10 20:44:29 2020 > > New Revision: 368523 > > URL: https://svnweb.freebsd.org/changeset/base/368523 > > > > Log: > > contig allocs: Don't retry forever on M_WAITOK. > > > > This restores behavior from before domain iterators were added in > > r327895 and r327896. > > > > The vm_domainset_iter_policy() will do a vm_wait_doms() and then > > restart its iterator when M_WAITOK is set. It will also force > > the containing loop to have M_NOWAIT. So we get an unbounded > > retry loop rather than the intended bounded retries that > > kmem_alloc_contig_pages() already handles. > > > > This also restores M_WAITOK to the vmem_alloc() call in > > kmem_alloc_attr_domain() and kmem_alloc_contig_domain(). > > > > Reviewed by: markj, kib > > MFC after: 2 weeks > > Sponsored by: Dell EMC > > Differential Revision: https://reviews.freebsd.org/D27507 > > > > Modified: > > head/sys/vm/vm_kern.c > > > > Modified: head/sys/vm/vm_kern.c > > ============================================================================== > > --- head/sys/vm/vm_kern.c Thu Dec 10 20:44:05 2020 (r368522) > > +++ head/sys/vm/vm_kern.c Thu Dec 10 20:44:29 2020 (r368523) > > @@ -264,9 +264,15 @@ kmem_alloc_attr_domainset(struct domainset *ds, vm_siz > > { > > struct vm_domainset_iter di; > > vm_offset_t addr; > > - int domain; > > + int domain, iflags; > > > > - vm_domainset_iter_policy_init(&di, ds, &domain, &flags); > > + /* > > + * Do not allow the domainset iterator to override wait flags. The > > + * contiguous memory allocator defines special semantics for M_WAITOK > > + * that do not match the iterator's implementation. > > + */ > > + iflags = (flags & ~M_WAITOK) | M_NOWAIT; > > + vm_domainset_iter_policy_init(&di, ds, &domain, &iflags); > > do { > > addr = kmem_alloc_attr_domain(domain, size, flags, low, high, > > memattr); > > @@ -346,9 +352,15 @@ kmem_alloc_contig_domainset(struct domainset *ds, vm_s > > { > > struct vm_domainset_iter di; > > vm_offset_t addr; > > - int domain; > > + int domain, iflags; > > > > - vm_domainset_iter_policy_init(&di, ds, &domain, &flags); > > + /* > > + * Do not allow the domainset iterator to override wait flags. The > > + * contiguous memory allocator defines special semantics for M_WAITOK > > + * that do not match the iterator's implementation. > > + */ > > + iflags = (flags & ~M_WAITOK) | M_NOWAIT; > > + vm_domainset_iter_policy_init(&di, ds, &domain, &iflags); > > do { > > addr = kmem_alloc_contig_domain(domain, size, flags, low, high, > > alignment, boundary, memattr); > > > > Hi, > > Should "iflags" also be passed to kmem_alloc_contig_domain() !? It is intentional, iflags is modified to ensure that the domainset iterator does not loop until the allocation is successful, and kmem_alloc_contig_pages() implements its own M_WAITOK handling. > I'm seeing the following panic: > > panic("vm_wait in early boot") > vm_wait_domain() > kmem_alloc_contig_pages() > kmem_alloc_contig_domainset() > kmem_alloc_contig() > contigmalloc() > x86bios_alloc() > vesa_configure() > vesa_mod_event() > vesa_module_register_init() > mi_startup() Is it on a NUMA system? I see that the new logic won't work properly if there are empty domains, so this suggests that we really do need a special contig iterator as discussed in the review.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?X9jH20xQcS72vuDQ>