On Fri, Oct 03, 2014 at 05:25:33PM +0900, Kohji Okuno wrote:> Hi, > > At least in i386 && 9-stable, when we call pmap_mapdev() and > pmap_unmapdev(), kernel_pmap.pm_stats.resident_count is decreased > incorrectly. > > pmap_mapdev_attr()->pmap_kenter_attr(): > In this path, kernel_pmap.pm_stats.resident_count is not increlmented. > > pmap_unmapdev()->kmem_free(kernel_map)->vm_map_remove()->pmap_remove(): > But, in this path, kernel_pmap.pm_stats.resident_count is decreased in > pmap_remove_pte(). > > While I called pmap_mapdev() and pmap_unmapdev() repeatedly and > kernel_pmap.pm_stats.resident_count became `0', since pmap_remove() > returned without removing ptes, I encountered various kernel panics.I think you are right. The problem seems to be fixed in HEAD and 10, since mapdev was switched to use kva_alloc/kva_free. I added stable@ to Cc:.> > And, when I enabled INVARIANTS, I looked the following panic message. > panic: vm_page_free_toq: freeing mapped page 0xXXXXXXXX.Do you mean that this panic is related to missed pmap_remove() ? I doubt it, since pmap_mapdev() does not establish managed mappings.> > I think, I should change the following. > What do you think about this? > > > diff --git a/sys/i386/i386/pmap.c b/sys/i386/i386/pmap.c > index 2adc6f8..a0998e8 100644 > --- a/sys/i386/i386/pmap.c > +++ b/sys/i386/i386/pmap.c > @@ -5061,6 +5061,7 @@ pmap_mapdev_attr(vm_paddr_t pa, vm_size_t size, int mode) > { > vm_offset_t va, offset; > vm_size_t tmpsize; > + int kmem_allocated = 0; > > offset = pa & PAGE_MASK; > size = roundup(offset + size, PAGE_SIZE); > @@ -5068,13 +5069,18 @@ pmap_mapdev_attr(vm_paddr_t pa, vm_size_t size, int mode) > > if (pa < KERNLOAD && pa + size <= KERNLOAD) > va = KERNBASE + pa; > - else > + else { > va = kmem_alloc_nofault(kernel_map, size); > + kmem_allocated = 1;You could just do PMAP_LOCK(kernel_pmap); kernel_pmap.pm_stats.resident_count += OFF_TO_IDX(size); PMAP_UNLOCK(kernel_pmap); there. And, the same fix is needed for amd64.> + } > if (!va) > panic("pmap_mapdev: Couldn't alloc kernel virtual memory"); > > - for (tmpsize = 0; tmpsize < size; tmpsize += PAGE_SIZE) > + for (tmpsize = 0; tmpsize < size; tmpsize += PAGE_SIZE) { > pmap_kenter_attr(va + tmpsize, pa + tmpsize, mode); > + if (kmem_allocated) > + kernel_pmap.pm_stats.resident_count++; > + } > pmap_invalidate_range(kernel_pmap, va, va + tmpsize); > pmap_invalidate_cache_range(va, va + size); > return ((void *)(va + offset)); > _______________________________________________ > freebsd-current at freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to "freebsd-current-unsubscribe at freebsd.org"
Hi, Konstantin, Thank you for your comment. And, your change is better than mine.> Do you mean that this panic is related to missed pmap_remove() ? > I doubt it, since pmap_mapdev() does not establish managed mappings.Yes, pmap_mapdev() does not establish managed mappings. But, if kernel_pmap.pm_stats.resident_count is zero, then any managed pages (for example pipe_map, exec_map, or etc.) are not able to change unmanaged status, because pmap_remove() returns without calling pmap_remove_pte(). In this result, I encounterd the panic. Could you refer the following? Regards, Kohji Okuno int vm_map_delete(vm_map_t map, vm_offset_t start, vm_offset_t end) { >> SNIP << /** this pmap_remove() does not change for mappings! **/ pmap_remove(map->pmap, entry->start, entry->end); /* * Delete the entry only after removing all pmap * entries pointing to its pages. (Otherwise, its * page frames may be reallocated, and any modify bits * will be set in the wrong object!) */ /** this calls vm_page_free_toq() and causes panic! **/ vm_map_entry_delete(map, entry); entry = next; } return (KERN_SUCCESS); }> On Fri, Oct 03, 2014 at 05:25:33PM +0900, Kohji Okuno wrote: >> Hi, >> >> At least in i386 && 9-stable, when we call pmap_mapdev() and >> pmap_unmapdev(), kernel_pmap.pm_stats.resident_count is decreased >> incorrectly. >> >> pmap_mapdev_attr()->pmap_kenter_attr(): >> In this path, kernel_pmap.pm_stats.resident_count is not increlmented. >> >> pmap_unmapdev()->kmem_free(kernel_map)->vm_map_remove()->pmap_remove(): >> But, in this path, kernel_pmap.pm_stats.resident_count is decreased in >> pmap_remove_pte(). >> >> While I called pmap_mapdev() and pmap_unmapdev() repeatedly and >> kernel_pmap.pm_stats.resident_count became `0', since pmap_remove() >> returned without removing ptes, I encountered various kernel panics. > I think you are right. > > The problem seems to be fixed in HEAD and 10, since mapdev was switched > to use kva_alloc/kva_free. I added stable@ to Cc:. > >> >> And, when I enabled INVARIANTS, I looked the following panic message. >> panic: vm_page_free_toq: freeing mapped page 0xXXXXXXXX. > Do you mean that this panic is related to missed pmap_remove() ? > I doubt it, since pmap_mapdev() does not establish managed mappings. > >> >> I think, I should change the following. >> What do you think about this? >> >> >> diff --git a/sys/i386/i386/pmap.c b/sys/i386/i386/pmap.c >> index 2adc6f8..a0998e8 100644 >> --- a/sys/i386/i386/pmap.c >> +++ b/sys/i386/i386/pmap.c >> @@ -5061,6 +5061,7 @@ pmap_mapdev_attr(vm_paddr_t pa, vm_size_t size, int mode) >> { >> vm_offset_t va, offset; >> vm_size_t tmpsize; >> + int kmem_allocated = 0; >> >> offset = pa & PAGE_MASK; >> size = roundup(offset + size, PAGE_SIZE); >> @@ -5068,13 +5069,18 @@ pmap_mapdev_attr(vm_paddr_t pa, vm_size_t size, int mode) >> >> if (pa < KERNLOAD && pa + size <= KERNLOAD) >> va = KERNBASE + pa; >> - else >> + else { >> va = kmem_alloc_nofault(kernel_map, size); >> + kmem_allocated = 1; > > You could just do > PMAP_LOCK(kernel_pmap); > kernel_pmap.pm_stats.resident_count += OFF_TO_IDX(size); > PMAP_UNLOCK(kernel_pmap); > there. And, the same fix is needed for amd64. > >> + } >> if (!va) >> panic("pmap_mapdev: Couldn't alloc kernel virtual memory"); >> >> - for (tmpsize = 0; tmpsize < size; tmpsize += PAGE_SIZE) >> + for (tmpsize = 0; tmpsize < size; tmpsize += PAGE_SIZE) { >> pmap_kenter_attr(va + tmpsize, pa + tmpsize, mode); >> + if (kmem_allocated) >> + kernel_pmap.pm_stats.resident_count++; >> + } >> pmap_invalidate_range(kernel_pmap, va, va + tmpsize); >> pmap_invalidate_cache_range(va, va + size); >> return ((void *)(va + offset)); >> _______________________________________________ >> freebsd-current@freebsd.org mailing list >> http://lists.freebsd.org/mailman/listinfo/freebsd-current >> To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org" > _______________________________________________ > freebsd-current@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org"_______________________________________________ freebsd-stable@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-stable To unsubscribe, send any mail to "freebsd-stable-unsubscribe@freebsd.org"
Hi, Konstantin, Thank you for your comment. And, your change is better than mine.> Do you mean that this panic is related to missed pmap_remove() ? > I doubt it, since pmap_mapdev() does not establish managed mappings.Yes, pmap_mapdev() does not establish managed mappings. But, if kernel_pmap.pm_stats.resident_count is zero, then any managed pages (for example pipe_map, exec_map, or etc.) are not able to change unmanaged status, because pmap_remove() returns without calling pmap_remove_pte(). In this result, I encounterd the panic. Could you refer the following? Regards, Kohji Okuno int vm_map_delete(vm_map_t map, vm_offset_t start, vm_offset_t end) { >> SNIP << /** this pmap_remove() does not change for mappings! **/ pmap_remove(map->pmap, entry->start, entry->end); /* * Delete the entry only after removing all pmap * entries pointing to its pages. (Otherwise, its * page frames may be reallocated, and any modify bits * will be set in the wrong object!) */ /** this calls vm_page_free_toq() and causes panic! **/ vm_map_entry_delete(map, entry); entry = next; } return (KERN_SUCCESS); }> On Fri, Oct 03, 2014 at 05:25:33PM +0900, Kohji Okuno wrote: >> Hi, >> >> At least in i386 && 9-stable, when we call pmap_mapdev() and >> pmap_unmapdev(), kernel_pmap.pm_stats.resident_count is decreased >> incorrectly. >> >> pmap_mapdev_attr()->pmap_kenter_attr(): >> In this path, kernel_pmap.pm_stats.resident_count is not increlmented. >> >> pmap_unmapdev()->kmem_free(kernel_map)->vm_map_remove()->pmap_remove(): >> But, in this path, kernel_pmap.pm_stats.resident_count is decreased in >> pmap_remove_pte(). >> >> While I called pmap_mapdev() and pmap_unmapdev() repeatedly and >> kernel_pmap.pm_stats.resident_count became `0', since pmap_remove() >> returned without removing ptes, I encountered various kernel panics. > I think you are right. > > The problem seems to be fixed in HEAD and 10, since mapdev was switched > to use kva_alloc/kva_free. I added stable@ to Cc:. > >> >> And, when I enabled INVARIANTS, I looked the following panic message. >> panic: vm_page_free_toq: freeing mapped page 0xXXXXXXXX. > Do you mean that this panic is related to missed pmap_remove() ? > I doubt it, since pmap_mapdev() does not establish managed mappings. > >> >> I think, I should change the following. >> What do you think about this? >> >> >> diff --git a/sys/i386/i386/pmap.c b/sys/i386/i386/pmap.c >> index 2adc6f8..a0998e8 100644 >> --- a/sys/i386/i386/pmap.c >> +++ b/sys/i386/i386/pmap.c >> @@ -5061,6 +5061,7 @@ pmap_mapdev_attr(vm_paddr_t pa, vm_size_t size, int mode) >> { >> vm_offset_t va, offset; >> vm_size_t tmpsize; >> + int kmem_allocated = 0; >> >> offset = pa & PAGE_MASK; >> size = roundup(offset + size, PAGE_SIZE); >> @@ -5068,13 +5069,18 @@ pmap_mapdev_attr(vm_paddr_t pa, vm_size_t size, int mode) >> >> if (pa < KERNLOAD && pa + size <= KERNLOAD) >> va = KERNBASE + pa; >> - else >> + else { >> va = kmem_alloc_nofault(kernel_map, size); >> + kmem_allocated = 1; > > You could just do > PMAP_LOCK(kernel_pmap); > kernel_pmap.pm_stats.resident_count += OFF_TO_IDX(size); > PMAP_UNLOCK(kernel_pmap); > there. And, the same fix is needed for amd64. > >> + } >> if (!va) >> panic("pmap_mapdev: Couldn't alloc kernel virtual memory"); >> >> - for (tmpsize = 0; tmpsize < size; tmpsize += PAGE_SIZE) >> + for (tmpsize = 0; tmpsize < size; tmpsize += PAGE_SIZE) { >> pmap_kenter_attr(va + tmpsize, pa + tmpsize, mode); >> + if (kmem_allocated) >> + kernel_pmap.pm_stats.resident_count++; >> + } >> pmap_invalidate_range(kernel_pmap, va, va + tmpsize); >> pmap_invalidate_cache_range(va, va + size); >> return ((void *)(va + offset)); >> _______________________________________________ >> freebsd-current at freebsd.org mailing list >> http://lists.freebsd.org/mailman/listinfo/freebsd-current >> To unsubscribe, send any mail to "freebsd-current-unsubscribe at freebsd.org" > _______________________________________________ > freebsd-current at freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to "freebsd-current-unsubscribe at freebsd.org"
On Sat, Oct 04, 2014 at 05:00:36PM +0900, Kohji Okuno wrote:> Hi, Konstantin, > > Thank you for your comment. > And, your change is better than mine.At the end of the mail is commit candidate. I did not even compiled this. Can you test and report back, please ?> > > Do you mean that this panic is related to missed pmap_remove() ? > > I doubt it, since pmap_mapdev() does not establish managed mappings. > > Yes, pmap_mapdev() does not establish managed mappings. But, if > kernel_pmap.pm_stats.resident_count is zero, then any managed pages > (for example pipe_map, exec_map, or etc.) are not able to change > unmanaged status, because pmap_remove() returns without calling > pmap_remove_pte(). > > In this result, I encounterd the panic. Could you refer the following?Yes, kmem_back() indeed uses managed mapping. Index: amd64/amd64/pmap.c ==================================================================--- amd64/amd64/pmap.c (revision 272506) +++ amd64/amd64/pmap.c (working copy) @@ -5040,6 +5040,9 @@ pmap_mapdev_attr(vm_paddr_t pa, vm_size_t size, in pa = trunc_page(pa); for (tmpsize = 0; tmpsize < size; tmpsize += PAGE_SIZE) pmap_kenter_attr(va + tmpsize, pa + tmpsize, mode); + PMAP_LOCK(kernel_pmap); + kernel_pmap.pm_stats.resident_count += OFF_TO_IDX(size); + PMAP_UNLOCK(kernel_pmap); pmap_invalidate_range(kernel_pmap, va, va + tmpsize); pmap_invalidate_cache_range(va, va + tmpsize); return ((void *)(va + offset)); Index: i386/i386/pmap.c ==================================================================--- i386/i386/pmap.c (revision 272506) +++ i386/i386/pmap.c (working copy) @@ -5066,10 +5066,14 @@ pmap_mapdev_attr(vm_paddr_t pa, vm_size_t size, in size = roundup(offset + size, PAGE_SIZE); pa = pa & PG_FRAME; - if (pa < KERNLOAD && pa + size <= KERNLOAD) + if (pa < KERNLOAD && pa + size <= KERNLOAD) { va = KERNBASE + pa; - else + } else { va = kmem_alloc_nofault(kernel_map, size); + PMAP_LOCK(kernel_pmap); + kernel_pmap.pm_stats.resident_count += OFF_TO_IDX(size); + PMAP_UNLOCK(kernel_pmap); + } if (!va) panic("pmap_mapdev: Couldn't alloc kernel virtual memory"); _______________________________________________ freebsd-stable@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-stable To unsubscribe, send any mail to "freebsd-stable-unsubscribe@freebsd.org"