Messages in this thread | | | Date | Tue, 18 Nov 2014 15:01:38 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH] zsmalloc: correct fragile [kmap|kunmap]_atomic use |
| |
On Fri, 14 Nov 2014 09:07:32 -0600 Seth Jennings <sjennings@variantweb.net> wrote:
> On Fri, Nov 14, 2014 at 10:11:01AM +0900, Minchan Kim wrote: > > The kunmap_atomic should use virtual address getting by kmap_atomic. > > However, some pieces of code in zsmalloc uses modified address, > > not the one got by kmap_atomic for kunmap_atomic. > > > > It's okay for working because zsmalloc modifies the address > > inner PAGE_SIZE bounday so it works with current kmap_atomic's > > implementation. But it's still fragile with potential changing > > of kmap_atomic so let's correct it.
It is a bit alarming, but I've seen code elsewhere in which a modified pointer is passed to kunmap_atomic(). So the kunmap_atomic() interface is "kvaddr should point somewhere into the page" and that won't be changing without a big effort.
> Seems like you could just use PAGE_MASK to get the base page address > from link like this:
I think Minchan's approach is better: it explicitly retains the kmap_atomic() return value for passing to kunmap_atomic(). That's nicer than modifying it and then setting it back again.
I mean, a cleaner way of implementing your suggestion would be
void kunmap_atomic_unaligned(void *p) { kunmap_atomic(void *)((unsigned long)p & PAGE_MASK); }
but then one looks at
void __kunmap_atomic(void *kvaddr) { unsigned long vaddr = (unsigned long) kvaddr & PAGE_MASK;
and asks "what the heck".
So I dunno. We could leave the code as-is. I have no strong feelings either way. Minchan's patch has no effect on zsmalloc.o section sizes with my compiler.
| |