"vmalloc", friends and GFP flags

"vmalloc", friends and GFP flags

[Russell King]

Hi,

This mail is being copied to the XFS and PARISC mailing lists.  You should
probably trim the reply list as appropriate.

I've just been looking over the state of the ARM {dma,pci}_alloc_consistent
wrt interrupts etc in 2.5.53, and decided to have a look around this area
for other users.  I stumbled across the following unsafe areas.

a) parisc.  Looking at pa11_dma_alloc_consistent:

   pa11_dma_alloc_consistent
	-> map_uncached_pages
		-> (eventually) map_pmd_uncached
			-> pte_alloc_kernel
				-> pte_alloc_one_kernel

	static inline pte_t *
	pte_alloc_one_kernel(struct mm_struct *mm, unsigned long addr)
	{
	        pte_t *pte = (pte_t *) __get_free_page(GFP_KERNEL);
        	if (likely(pte != NULL))
	                clear_page(pte);
        	return pte;
	}

   End result is that if pa11_dma_alloc_consistent() is called from
   IRQ context, parisc calls __get_free_page using GFP_KERNEL.
   Calling __get_free_page with GFP_KERNEL from IRQ context appears
   to be unsafe (always has been.)

b) xfs. XFS contains this bit of code:

	static __inline unsigned int flag_convert(int flags)
	{
		...
	        if (flags & KM_NOSLEEP)
        	        return GFP_ATOMIC;
	        /* If we're in a transaction, FS activity is not ok */
        	else if ((current->flags & PF_FSTRANS) || (flags & KM_NOFS))
	                return GFP_NOFS;
        	else
	                return GFP_KERNEL;
	}

	void *kmem_alloc(size_t size, int flags)
	{
		...
                rval = __vmalloc(size, flag_convert(flags), PAGE_KERNEL);
		...
	}

   Ok, so xfs can pass GFP_ATOMIC, GFP_NOFS and GFP_KERNEL to __vmalloc.
   However, do these flags actually ensure what they suggest they do?

   Lets look at __vmalloc:

	void *__vmalloc(unsigned long size, int gfp_mask, pgprot_t prot)
	{
		...
        	area = get_vm_area(size, VM_ALLOC);
		...
	}

   and get_vm_area:

	struct vm_struct *get_vm_area(unsigned long size, unsigned long flags)
	{
		...
        	area = kmalloc(sizeof(*area), GFP_KERNEL);
		...
	}

   Also, look at the PMD and PTE allocation functions, which also use
   GFP_KERNEL.

   Answer to the above question: no.  Is this unsafe?  Probably.

Now, what does this all have to do with ARM and pci_alloc_consistent.
It's the same problem, only I have a BUG_ON() in there to catch uses
from IRQ context, which needs to disappear eventually.  (This is
mainly a requirement for USB to work.)

However, before this can happen, I believe I'd need the following:

1. allocation of vm areas (via get_vm_area and friends) needs to become
   IRQ-safe.

   - vmlist_lock needs to become IRQ safe (but its held during vread/vwrite)
   - kmalloc of vm_structs needs to have gfp flags passed in

2. allocation of page tables (via pmd_alloc_kernel / pte_alloc_kernel)
   needs to become IRQ-safe.

   - pmd_alloc_kernel / pte_alloc_kernel needs to have gfp flags passed in
   - map_vm_area would also need gfp flags

The same solution also fixes the two instances described above.

I suspect, however, that the vmlist_lock change will be unacceptable
to many people however.  This doesn't affect XFS nor parisc, but would
mean that I'd need to completely rewrite the ARM consistent memory
allocation to use none of the above (which I think is the right
approach, but means _all_ of the above becomes S.E.P. 8))

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: "vmalloc", friends and GFP flags

[James Bottomley]

> I've just been looking over the state of the ARM {dma,pci}_alloc_consis
> tent wrt interrupts etc in 2.5.53, and decided to have a look around
> this area for other users.  I stumbled across the following unsafe
> areas.

> a) parisc.  Looking at pa11_dma_alloc_consistent:

I agree with the flow analysis.  However, there is a condition attached to 
getting from pte_alloc_kernel to pte_alloc_one_kernel which makes the 
GFP_KERNEL allocation.  That condition looks to be that the pmd is not 
present.  I don't believe that's possible for an area of memory we just got 
from __get_free_pages (otherwise the kernel could fault when accessing that 
region which isn't allowed).

If the above analysis is correct (which I'll defer to better opinion) then we 
can in practice never get down the problem path, and dma_ allocation from 
interrupt is safe on parisc.

James

Re: [parisc-linux] "vmalloc", friends and GFP flags

[Grant Grundler]

On Wed, Jan 01, 2003 at 09:49:33PM +0000, Russell King wrote:
> a) parisc.  Looking at pa11_dma_alloc_consistent:

If James is wrong and it's un-safe to call dma_alloc_consistent() on
the interrupt path, I'm only slightly worried since I've only seen
dma_alloc_consistent() get called during driver initialization.
DMA-mapping.txt does not restrict what context "driver initialization"
might take place in.

grant