From owner-svn-src-head@freebsd.org Tue Dec 15 14:27:43 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 4FBAF4B7C8B; Tue, 15 Dec 2020 14:27:43 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: from mail-qk1-x72c.google.com (mail-qk1-x72c.google.com [IPv6:2607:f8b0:4864:20::72c]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4CwLFW1WNPz3vvq; Tue, 15 Dec 2020 14:27:43 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: by mail-qk1-x72c.google.com with SMTP id 22so8959912qkf.9; Tue, 15 Dec 2020 06:27:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=37vbU/++S5+OutWJQnE4KZOGX8voI4GQablWFgtbTtg=; b=Zaq43VdAL56H0TYJPg7AxKXL5KZLdVABWdLROQVY8YiPPj49+i0v3o1fjsmIchBwCH W2iyqMboYjB9Gp94G5+tJT8svZUt5ySwwOUuZxKpYtf6Ck3DGFoObVGgYxvjIB54kDxk oRXf7QvLi7vXLCj9UhX03+cZ153pEvTKgBLQ6ARNLiXX1Qj+adomzLVKqrOgVncCQasP xwiEocIDeze2YpXFiUr+MqPETfbuzwSOCEEZvvG8N9yN4zc699H9vrSVaBcwCuJ8EDyI TSmEkFf5gkf9Q/a7A40dgsRPF+V8Vr8tUPBTtK5RVlk9A0z+rTVYoj03sroYILCyeAwy xYUQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to; bh=37vbU/++S5+OutWJQnE4KZOGX8voI4GQablWFgtbTtg=; b=kWOtDvAR60MSUp7bcvRu+plGOdtBAfCyfOdS0IEAutQa3j3JvC6FdcoUeyHTLaiaVR 61G29+5rAE4WbH79N7+H7i4lRueBZjXbgVVp6EIkRVEB4G+M2JBMNHKPxhYWIbaplHXQ +KH/uIl399AT8Z6mV/m1DMl19Kzpulqw5BbDVOd8YM231+s8FcBSFJu7Z2NnT8thsizo bodj2TMPm52tC3sinfbZwJS0ui/LGqN84gzy1GLkyKiLMrD8nnzljnqkIH0oHUv4mdCG btKDmQtnsTwf2ErNXD40ySfR24juDQoKqbIi1I6nVgF/wyx4cbUQK9FR6NZIR4bhq8MH M+Yg== X-Gm-Message-State: AOAM533yJb+uak8cSLOWuhCtm3mjaZ67XIkscOmtEOjIrWrlXo6qYu29 YI2YXV30uJbTDlptqUgMwdgOG5KySEU= X-Google-Smtp-Source: ABdhPJzVElht788OUEGR6Yb3EwWDzO+j23BojWT6fYKgi65HiNNiM3RcZnnHIFlt0Xet8WihjvwWkw== X-Received: by 2002:a37:5103:: with SMTP id f3mr38624172qkb.460.1608042462272; Tue, 15 Dec 2020 06:27:42 -0800 (PST) Received: from raichu ([142.126.164.150]) by smtp.gmail.com with ESMTPSA id c20sm2481161qtj.29.2020.12.15.06.27.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 15 Dec 2020 06:27:41 -0800 (PST) Sender: Mark Johnston Date: Tue, 15 Dec 2020 09:27:39 -0500 From: Mark Johnston To: Hans Petter Selasky Cc: Bryan Drewery , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r368523 - head/sys/vm Message-ID: References: <202012102044.0BAKiTHh011767@repo.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 4CwLFW1WNPz3vvq X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 15 Dec 2020 14:27:43 -0000 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.