Re: svn commit: r368523 - head/sys/vm
On 12/15/2020 7:04 AM, Mark Johnston wrote:
> On Tue, Dec 15, 2020 at 03:33:09PM +0100, Hans Petter Selasky wrote:
>> On 12/15/20 3:27 PM, Mark Johnston wrote:
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.
>>
>> Yes, this is a numa system.
>>
>> I just noticed, that before r368523 "flags" was updated by
>> _vm_domainset_iter_policy_init() to always contain M_NOWAIT and that
>> avoids the wait logic, but I think x86bios_alloc() doesn't get its
>> memory then.
>
> Yes, but note that vm_domainset_iter_policy() will also call
> vm_wait_doms() if a M_NOWAIT allocation from each domain fails.
> x86bios_alloc() requests memory from the first 1MB of physical memory,
> but because contigmalloc() uses a round-robin iterator initialized from
> per-thread state it may try from the "wrong" domain first. So really a
> different solution to the original problem is needed.
>
>> I'm not sure if x86bios_alloc() needs to be attached a bit later anyway?
>>
>> --HPS
I have reverted the change in r368673 until we come up with a more
comprehensive fix.
--
Regards,
Bryan Drewery
OpenPGP_signature
Description: OpenPGP digital signature
Re: svn commit: r368523 - head/sys/vm
On Tue, Dec 15, 2020 at 03:33:09PM +0100, Hans Petter Selasky wrote:
> On 12/15/20 3:27 PM, Mark Johnston wrote:
> >> 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.
>
> Yes, this is a numa system.
>
> I just noticed, that before r368523 "flags" was updated by
> _vm_domainset_iter_policy_init() to always contain M_NOWAIT and that
> avoids the wait logic, but I think x86bios_alloc() doesn't get its
> memory then.
Yes, but note that vm_domainset_iter_policy() will also call
vm_wait_doms() if a M_NOWAIT allocation from each domain fails.
x86bios_alloc() requests memory from the first 1MB of physical memory,
but because contigmalloc() uses a round-robin iterator initialized from
per-thread state it may try from the "wrong" domain first. So really a
different solution to the original problem is needed.
> I'm not sure if x86bios_alloc() needs to be attached a bit later anyway?
>
> --HPS
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r368523 - head/sys/vm
On 12/15/20 3:27 PM, Mark Johnston wrote:
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.
Yes, this is a numa system.
I just noticed, that before r368523 "flags" was updated by
_vm_domainset_iter_policy_init() to always contain M_NOWAIT and that
avoids the wait logic, but I think x86bios_alloc() doesn't get its
memory then.
I'm not sure if x86bios_alloc() needs to be attached a bit later anyway?
--HPS
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r368523 - head/sys/vm
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(, ds, , );
> > + /*
> > +* 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(, ds, , );
> > 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(, ds, , );
> > + /*
> > +* 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(, ds, , );
> > 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.
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r368523 - head/sys/vm
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(, ds, , );
+ /*
+* 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(, ds, , );
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(, ds, , );
+ /*
+* 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(, ds, , );
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() !?
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()
--HPS
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
svn commit: r368523 - head/sys/vm
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(, ds, , );
+ /*
+* 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(, ds, , );
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(, ds, , );
+ /*
+* 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(, ds, , );
do {
addr = kmem_alloc_contig_domain(domain, size, flags, low, high,
alignment, boundary, memattr);
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
