kmap_atomic and preemption

Hi,

I was staring at some recent ARC highmem crashes and see that kmap_atomic() disables preemption even when page is in lowmem and call returns right away. This seems to be true for other arches as well.

arch/arc/mm/highmem.c:

void *kmap_atomic(struct page *page)

{
	int idx, cpu_idx;
	unsigned long vaddr;

	preempt_disable();
	pagefault_disable();
	if (!PageHighMem(page))
		return page_address(page);

        /* do the highmem foo ... */

..
}

I would really like to implement a inline fastpath for !PageHighMem(page) case and do the highmem foo out-of-line.

Is preemption disabling a requirement of kmap_atomic() callers independent of where page is or is it only needed when page is in highmem and can trigger page faults or TLB Misses between kmap_atomic() and kunmap_atomic and wants protection against reschedules etc.

-Vineet

Traditionally kmap_atomic() disables preemption; and the reason is that the returned pointer must stay valid. This had a side effect in that it also disabled pagefaults.

We've since de-coupled the pagefault from the preemption thing, so you could disable pagefaults while leaving preemption enabled.

Now, I've also done preemptible kmap_atomic() on -rt; which appears to work, suggesting nothing relies on it disabling preemption (on -rt).

So sure; you can try and leave preemption enabled for lowmem pages, see what comes apart -- if anything. It gives weird semantics for kmap_atomic() though, and I'm not sure the cost of doing that preempt_disable/preempt_enable() is worth the pain.

If you want a fast-slow path splt, you can easily do something like:

static inline void *kmap_atomic(struct page *page)

{
	preempt_disable();
	pagefault_disable();
	if (!PageHighMem(page))
		return page_address(page);

	return __kmap_atomic(page);
}

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Hello,

This is essentially what has been done on ARM in commit 9ff0bb5ba60638a688a46e93df8c5009896672eb, showing a pretty significant improvement in network workloads.

Best regards,

Thomas

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

ARC already has that semantically - only not inline ! I really want to avoid 2 needless LD-ADD-ST for the disabling of preemption and page fault for the low mem pages by returning early !

static inline void *kmap_atomic(struct page *page)

{
        if (!PageHighMem(page))
		return page_address(page);

	preempt_disable();
	pagefault_disable();
	
	return __kmap_atomic(page);

}

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

But how could the ptr possibly get invalid. Say despite the disable calls, we could actually take the page fault (or TLB Miss on ARC) - the pagefault_disable() only makes do_page_fault() do reduced handling vs. calling handle_mm_fault() etc. It is essentially restricting the fault handling to a kernel mode fixup only.

Now if we didn't do disable, on ARC the semantics of do_page_fault() are still the same - since the address would be for fixmap which is handled under "kernel" only category as well.

void do_page_fault(unsigned long address, struct pt_regs *regs) {

    if (address >= VMALLOC_START) {
        ret = handle_kernel_vaddr_fault(address);
        return;
...
    if (faulthandler_disabled() || !mm)
        goto no_context;

...

Right - I've seen that patch set from David H.

I actually want to return early for !PageHighMem and avoid the pointless 2 LD-ADD-ST to memory for map and 2 LD-SUB-ST for unmap for regular pages for such cases.

So I'm fairly sure people rely on the fact you cannot have pagefault inside a kmap_atomic().

But you could potentially get away with leaving preemption enabled. Give it a try, see if something goes *bang* ;-)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

So this translates to: any hardware page faults inside kmap_atomic() can't lead to do_page_fault() taking a lock - those can only be ex_table fixups, yes ? Could you please also help explain your earlier comment about kmap_atomic needing to disable preemption so that "returned pointer stayed valid". I can't quite fathom how that can happen

So tried patch further below: on a quad core slowish FPGA setup, concurrent hackbench and LMBench seem to run w/o issues - so it is not obviously broken even if not proven otherwise. But the point is highmem page is a slow path anyways - needs a PTE update, new TLB entry etc. I hoped to not wiggle even a single cache line for the low page - but seems like that is not possible.

OTOH, if we do keep the status quo - then making these 2 cache lines into 1 is not possible either. From reading the orig "decoupling of prremption and page fault" thread it seems to be because preempt count is per cpu on x86.

@@ -67,7 +67,6 @@ void *kmap_atomic(struct page *page)

        int idx, cpu_idx;
        unsigned long vaddr;

-       preempt_disable();
        pagefault_disable();
        if (!PageHighMem(page))
                return page_address(page);
@@ -107,7 +106,6 @@ void __kunmap_atomic(void *kv)
        }
 
        pagefault_enable();

- preempt_enable();
}

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

A lowmem page should never change its page_address(), so that much is safe. I think the question is whether there is any driver code which assumes that preemption is unconditionally disabled between a kmap_atomic() has been called.

That wouldn't be an unreasonable assumption given the name of the function, so I'd suggest caution with making kmap_atomic() have these kinds of differing behaviours depending on whether we're asking to kmap a high or lowmem page.

If we are going to allow this, I think it at least needs to be well documented.

RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Agreed..

right, this and consistency. Having the function disable preemption sometimes is just plain weird.

So for -rt I did a preemptible kmap_atomic() for x86 (and maybe someone did other archs too, I cannot remember), now -rt is funny wrt locking anyway, but I cannot remember anything breaking because of this, so there is some hope it will actually work.

Indeed.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>