Re: About pmap_mapdev() & pmap_unmapdev()
Hi Konstantin,
I tested your patch. It was no problem.
Thank you for your kind correspondence.
Regards,
Kohji Okuno
> On Sat, Oct 04, 2014 at 05:53:26PM +0900, Kohji Okuno wrote:
>> Hi, Konstantin,
>>
>> > At the end of the mail is commit candidate. I did not even compiled this.
>> > Can you test and report back, please ?
>>
>> Could you check the following?
>> (1) should use kernel_pmap->pm_stats.resident_count
>> ^^^
>> I'm sorry. My patch was wrong.
> As well as mine.
>
>>
>> (2) should use pmap_resident_count_inc() in amd64.
> Correct.
>
>>
>>
>> I will test, later.
>>
>> In addtion, I have one question.
>> In current and 10-stable, is vm_map_delete() called by kva_free()?
> No, kva_free() only releases the vmem backing, leaving the page
> tables intact. This is why I only did the stable/9 patch.
>
>> If vm_map_delete() is called, this fix is needed in current and
>> 10-stable, I think.
>
> Updated patch below.
>
> 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);
> + pmap_resident_count_inc(kernel_pmap, 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-current@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: About pmap_mapdev() & pmap_unmapdev()
Hi Konstantin, Thank you very much for your detailed explanatin. I understood the policy of vmem. Many thanks, Kohji Okuno > On Sun, Oct 05, 2014 at 01:15:12PM +0900, Kohji Okuno wrote: >> Hi, >> >> > On Sat, Oct 04, 2014 at 08:53:35PM +0900, Kohji Okuno wrote: >> >> Hi Konstantin, >> >> >> >> Thank you for your prompt response. >> >> I will test and report from next monday. >> >> >> >> >> In addtion, I have one question. >> >> >> In current and 10-stable, is vm_map_delete() called by kva_free()? >> >> > No, kva_free() only releases the vmem backing, leaving the page >> >> > tables intact. This is why I only did the stable/9 patch. >> >> >> >> Where are PTEs allocated by pmap_mapdev() freed in current and 10-stable? >> >> Could you please explain me? >> > They are not freed. The removal of the vmem which covers the address >> > space managed by corresponding ptes, allows the reuse of both KVA region >> > and corresponding PTEs in the tables. The only concern with the resident >> > page tables is to avoid two kva_alloc() to step over each other, and >> > this is ensured by vmem. >> >> I agree that normal pages are reusable. But, since the pages mapped by >> pmap_mapdev() are concerned with the physicall address of device (For >> example: video memory), these PTEs aren't reusable, I think. >> So, should we free these PTEs by pmap_unmapdev()? > There is no hold on any physical pages which were referenced by the ptes. > The only thing which is left out are the records in the page tables > which map the KVA to said device memory. It is harmless. > > When the KVA is reused, the ptes in page tables are overwritten. > > It might be argued that clearing PTEs, or at least removing the PG_V > bit catches erronous unintended accesses to the freed range, but by the > cost of the overhead of re-polluting the caches. And since clearing is > not effective without doing TLB flush, which requires broadcast IPI, > it is more trouble than advantage. > ___ > freebsd-current@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org" ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: About pmap_mapdev() & pmap_unmapdev()
On Sun, Oct 05, 2014 at 01:15:12PM +0900, Kohji Okuno wrote: > Hi, > > > On Sat, Oct 04, 2014 at 08:53:35PM +0900, Kohji Okuno wrote: > >> Hi Konstantin, > >> > >> Thank you for your prompt response. > >> I will test and report from next monday. > >> > >> >> In addtion, I have one question. > >> >> In current and 10-stable, is vm_map_delete() called by kva_free()? > >> > No, kva_free() only releases the vmem backing, leaving the page > >> > tables intact. This is why I only did the stable/9 patch. > >> > >> Where are PTEs allocated by pmap_mapdev() freed in current and 10-stable? > >> Could you please explain me? > > They are not freed. The removal of the vmem which covers the address > > space managed by corresponding ptes, allows the reuse of both KVA region > > and corresponding PTEs in the tables. The only concern with the resident > > page tables is to avoid two kva_alloc() to step over each other, and > > this is ensured by vmem. > > I agree that normal pages are reusable. But, since the pages mapped by > pmap_mapdev() are concerned with the physicall address of device (For > example: video memory), these PTEs aren't reusable, I think. > So, should we free these PTEs by pmap_unmapdev()? There is no hold on any physical pages which were referenced by the ptes. The only thing which is left out are the records in the page tables which map the KVA to said device memory. It is harmless. When the KVA is reused, the ptes in page tables are overwritten. It might be argued that clearing PTEs, or at least removing the PG_V bit catches erronous unintended accesses to the freed range, but by the cost of the overhead of re-polluting the caches. And since clearing is not effective without doing TLB flush, which requires broadcast IPI, it is more trouble than advantage. ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: About pmap_mapdev() & pmap_unmapdev()
Hi, > On Sat, Oct 04, 2014 at 08:53:35PM +0900, Kohji Okuno wrote: >> Hi Konstantin, >> >> Thank you for your prompt response. >> I will test and report from next monday. >> >> >> In addtion, I have one question. >> >> In current and 10-stable, is vm_map_delete() called by kva_free()? >> > No, kva_free() only releases the vmem backing, leaving the page >> > tables intact. This is why I only did the stable/9 patch. >> >> Where are PTEs allocated by pmap_mapdev() freed in current and 10-stable? >> Could you please explain me? > They are not freed. The removal of the vmem which covers the address > space managed by corresponding ptes, allows the reuse of both KVA region > and corresponding PTEs in the tables. The only concern with the resident > page tables is to avoid two kva_alloc() to step over each other, and > this is ensured by vmem. I agree that normal pages are reusable. But, since the pages mapped by pmap_mapdev() are concerned with the physicall address of device (For example: video memory), these PTEs aren't reusable, I think. So, should we free these PTEs by pmap_unmapdev()? Best regards, Kohji Okuno ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: About pmap_mapdev() & pmap_unmapdev()
On Sat, Oct 04, 2014 at 08:53:35PM +0900, Kohji Okuno wrote: > Hi Konstantin, > > Thank you for your prompt response. > I will test and report from next monday. > > >> In addtion, I have one question. > >> In current and 10-stable, is vm_map_delete() called by kva_free()? > > No, kva_free() only releases the vmem backing, leaving the page > > tables intact. This is why I only did the stable/9 patch. > > Where are PTEs allocated by pmap_mapdev() freed in current and 10-stable? > Could you please explain me? They are not freed. The removal of the vmem which covers the address space managed by corresponding ptes, allows the reuse of both KVA region and corresponding PTEs in the tables. The only concern with the resident page tables is to avoid two kva_alloc() to step over each other, and this is ensured by vmem. ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: About pmap_mapdev() & pmap_unmapdev()
Hi Konstantin,
Thank you for your prompt response.
I will test and report from next monday.
>> In addtion, I have one question.
>> In current and 10-stable, is vm_map_delete() called by kva_free()?
> No, kva_free() only releases the vmem backing, leaving the page
> tables intact. This is why I only did the stable/9 patch.
Where are PTEs allocated by pmap_mapdev() freed in current and 10-stable?
Could you please explain me?
Regards,
Kohji Okuno
> On Sat, Oct 04, 2014 at 05:53:26PM +0900, Kohji Okuno wrote:
>> Hi, Konstantin,
>>
>> > At the end of the mail is commit candidate. I did not even compiled this.
>> > Can you test and report back, please ?
>>
>> Could you check the following?
>> (1) should use kernel_pmap->pm_stats.resident_count
>> ^^^
>> I'm sorry. My patch was wrong.
> As well as mine.
>
>>
>> (2) should use pmap_resident_count_inc() in amd64.
> Correct.
>
>>
>>
>> I will test, later.
>>
>> In addtion, I have one question.
>> In current and 10-stable, is vm_map_delete() called by kva_free()?
> No, kva_free() only releases the vmem backing, leaving the page
> tables intact. This is why I only did the stable/9 patch.
>
>> If vm_map_delete() is called, this fix is needed in current and
>> 10-stable, I think.
>
> Updated patch below.
>
> 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);
> + pmap_resident_count_inc(kernel_pmap, 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-current@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: About pmap_mapdev() & pmap_unmapdev()
On Sat, Oct 04, 2014 at 05:53:26PM +0900, Kohji Okuno wrote:
> Hi, Konstantin,
>
> > At the end of the mail is commit candidate. I did not even compiled this.
> > Can you test and report back, please ?
>
> Could you check the following?
> (1) should use kernel_pmap->pm_stats.resident_count
> ^^^
> I'm sorry. My patch was wrong.
As well as mine.
>
> (2) should use pmap_resident_count_inc() in amd64.
Correct.
>
>
> I will test, later.
>
> In addtion, I have one question.
> In current and 10-stable, is vm_map_delete() called by kva_free()?
No, kva_free() only releases the vmem backing, leaving the page
tables intact. This is why I only did the stable/9 patch.
> If vm_map_delete() is called, this fix is needed in current and
> 10-stable, I think.
Updated patch below.
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);
+ pmap_resident_count_inc(kernel_pmap, 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-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: About pmap_mapdev() & pmap_unmapdev()
Hi, Konstantin,
> At the end of the mail is commit candidate. I did not even compiled this.
> Can you test and report back, please ?
Could you check the following?
(1) should use kernel_pmap->pm_stats.resident_count
^^^
I'm sorry. My patch was wrong.
(2) should use pmap_resident_count_inc() in amd64.
I will test, later.
In addtion, I have one question.
In current and 10-stable, is vm_map_delete() called by kva_free()?
If vm_map_delete() is called, this fix is needed in current and
10-stable, I think.
Regards,
Kohji Okuno
> 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-current@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: About pmap_mapdev() & pmap_unmapdev()
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-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: About pmap_mapdev() & pmap_unmapdev()
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 0x.
> 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-unsubscr...@freebsd.org"
> ___
> freebsd-current@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: About pmap_mapdev() & pmap_unmapdev()
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 0x.
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-unsubscr...@freebsd.org"
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
About pmap_mapdev() & pmap_unmapdev()
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.
And, when I enabled INVARIANTS, I looked the following panic message.
panic: vm_page_free_toq: freeing mapped page 0x.
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;
+ }
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-unsubscr...@freebsd.org"
