Eliminate the unused arena parameter from kmem_alloc_contig()

[alc]

Summary

The title says it all.

[alc]

compat/linuxkpi/common/src/linux_page.c

170 ↗

(On Diff #46916)

Should this actually be a call to kmem_alloc_attr()?

[kib]

compat/linuxkpi/common/src/linux_page.c

170 ↗

(On Diff #46916)

How would kmem_alloc_attr() satisfy the requirement of the phys address in the low 4G ?

[alc]

compat/linuxkpi/common/src/linux_page.c

170 ↗

(On Diff #46916)

Recall that kmem_alloc_attr() takes a parameter vm_paddr_t high. It just doesn't guarantee that the allocated pages are physically contiguous. My question is really, "Does this caller actually require contiguity?"

[kib]

compat/linuxkpi/common/src/linux_page.c

170 ↗

(On Diff #46916)

I do not think that it requires contig physical allocation.

[• hselasky]

compat/linuxkpi/common/src/linux_page.c

170 ↗

(On Diff #46916)

Can you add full context to this patch?

170 ↗

(On Diff #46916)

If you look at the function:

linux_alloc_pages(gfp_t flags, unsigned int order)

and its corresponding

linux_free_pages(vm_page_t page, unsigned int order)

You'll see that the pages must be contiguous, else they can't be freed, because we only have one page pointer which is passed around, also when there is more than one page allocated.

Is this a problem?

[alc]

Increase the number of lines of context.

[jeff]

I did not do this before because I felt the ROI was low compared to the churn in ports. Have we contacted ports maintainers? The nvidia and virtualbox port are both going to need #ifdefs.

[alc]

compat/linuxkpi/common/src/linux_page.c

170 ↗

(On Diff #46916)

This snippet appears in linux_alloc_kmem(), not linux_alloc_pages(). If physical contiguity is required, then kmem_malloc() should not be used when GFP_DMA32 is clear, because it does not guarantee physical contiguity. If physical contiguity is not required, then kmem_alloc_attr() should be used when GFP_DMA32 is set, because it can often succeed when kmem_alloc_contig() cannot, because it doesn't require physical contiguity.

To be clear, this question is really orthogonal to this change. If linux_alloc_kmem() should call kmem_alloc_attr() instead of kmem_alloc_contig(), that should be committed as a stand-alone change.

[alc]

In D16799#357540, @jeff wrote:

I did not do this before because I felt the ROI was low compared to the churn in ports. Have we contacted ports maintainers? The nvidia and virtualbox port are both going to need #ifdefs.

I will followup with the ports maintainers. I had two reasons for this. Primarily, I want to implement stronger segregation for physical memory allocations that are permanent, e.g., physical pages backing UMA_ZONE_NOFREE. Specifically, I don't want to have to modify the kmem callers to specify a different arena if that arena is simply a placeholder. Secondarily, we use the arenas somewhat differently in older branches. So, mechanical MFCs may not do the correct thing anyway.

[alc]

In D16799#357563, @alc wrote:

In D16799#357540, @jeff wrote:

I did not do this before because I felt the ROI was low compared to the churn in ports. Have we contacted ports maintainers? The nvidia and virtualbox port are both going to need #ifdefs.

I will followup with the ports maintainers. I had two reasons for this. Primarily, I want to implement stronger segregation for physical memory allocations that are permanent, e.g., physical pages backing UMA_ZONE_NOFREE. Specifically, I don't want to have to modify the kmem callers to specify a different arena if that arena is simply a placeholder.

For example, until yesterday, we were pointlessly computing (and passing) an arena in uma_large_malloc_domain():

#if VM_NRESERVLEVEL > 0
	if (__predict_true((wait & M_EXEC) == 0))
		arena = kernel_arena;
	else
		arena = kernel_rwx_arena;
#else
	arena = kernel_arena;
#endif

	slab = zone_alloc_item(slabzone, NULL, domain, wait);
	if (slab == NULL)
		return (NULL);
	if (domain == UMA_ANYDOMAIN)
		addr = kmem_malloc(arena, size, wait);
	else
		addr = kmem_malloc_domain(arena, domain, size, wait);

I don't want to perpetuate and add further to this with a potential M_{NEVERFREED,PERMANENT} flag.

[jeff]

In D16799#357563, @alc wrote:

In D16799#357540, @jeff wrote:

I did not do this before because I felt the ROI was low compared to the churn in ports. Have we contacted ports maintainers? The nvidia and virtualbox port are both going to need #ifdefs.

I will followup with the ports maintainers. I had two reasons for this. Primarily, I want to implement stronger segregation for physical memory allocations that are permanent, e.g., physical pages backing UMA_ZONE_NOFREE. Specifically, I don't want to have to modify the kmem callers to specify a different arena if that arena is simply a placeholder. Secondarily, we use the arenas somewhat differently in older branches. So, mechanical MFCs may not do the correct thing anyway.

Do you intend to do kmem_malloc as well?

At this point in the release we should possibly also consult re@

[alc]

In D16799#357566, @jeff wrote:

In D16799#357563, @alc wrote:

In D16799#357540, @jeff wrote:

I did not do this before because I felt the ROI was low compared to the churn in ports. Have we contacted ports maintainers? The nvidia and virtualbox port are both going to need #ifdefs.

I will followup with the ports maintainers. I had two reasons for this. Primarily, I want to implement stronger segregation for physical memory allocations that are permanent, e.g., physical pages backing UMA_ZONE_NOFREE. Specifically, I don't want to have to modify the kmem callers to specify a different arena if that arena is simply a placeholder. Secondarily, we use the arenas somewhat differently in older branches. So, mechanical MFCs may not do the correct thing anyway.

Do you intend to do kmem_malloc as well?

Yes, and kmem_free().

At this point in the release we should possibly also consult re@

Sure, and I'll make it clear that this is not a functional change.

[jeff]

In D16799#357567, @alc wrote:

In D16799#357566, @jeff wrote:

In D16799#357563, @alc wrote:

In D16799#357540, @jeff wrote:

I did not do this before because I felt the ROI was low compared to the churn in ports. Have we contacted ports maintainers? The nvidia and virtualbox port are both going to need #ifdefs.

I will followup with the ports maintainers. I had two reasons for this. Primarily, I want to implement stronger segregation for physical memory allocations that are permanent, e.g., physical pages backing UMA_ZONE_NOFREE. Specifically, I don't want to have to modify the kmem callers to specify a different arena if that arena is simply a placeholder. Secondarily, we use the arenas somewhat differently in older branches. So, mechanical MFCs may not do the correct thing anyway.

Do you intend to do kmem_malloc as well?

Yes, and kmem_free().

At this point in the release we should possibly also consult re@

Sure, and I'll make it clear that this is not a functional change.

If you'd like I can write an awk script to convert these and we can fix it up by hand if necessary. There are quite a lot of those locations.

[alc]

In D16799#357568, @jeff wrote:

In D16799#357567, @alc wrote:

In D16799#357566, @jeff wrote:

In D16799#357563, @alc wrote:

In D16799#357540, @jeff wrote:

I did not do this before because I felt the ROI was low compared to the churn in ports. Have we contacted ports maintainers? The nvidia and virtualbox port are both going to need #ifdefs.

I will followup with the ports maintainers. I had two reasons for this. Primarily, I want to implement stronger segregation for physical memory allocations that are permanent, e.g., physical pages backing UMA_ZONE_NOFREE. Specifically, I don't want to have to modify the kmem callers to specify a different arena if that arena is simply a placeholder. Secondarily, we use the arenas somewhat differently in older branches. So, mechanical MFCs may not do the correct thing anyway.

Do you intend to do kmem_malloc as well?

Yes, and kmem_free().

At this point in the release we should possibly also consult re@

Sure, and I'll make it clear that this is not a functional change.

If you'd like I can write an awk script to convert these and we can fix it up by hand if necessary. There are quite a lot of those locations.

Sure, for kmem_malloc() that would be nice. Thank you.

[• hselasky]

compat/linuxkpi/common/src/linux_page.c

170 ↗

(On Diff #46916)

I see.