Messages in this thread | | | Date | Wed, 17 Oct 2018 11:04:01 -0400 (EDT) | From | Mathieu Desnoyers <> | Subject | Re: [RFC PATCH for 4.21 04/16] mm: Introduce vm_map_user_ram, vm_unmap_user_ram |
| |
----- On Oct 17, 2018, at 11:00 AM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:
> ----- On Oct 16, 2018, at 8:27 PM, Sergey Senozhatsky > sergey.senozhatsky.work@gmail.com wrote: > >> On (10/16/18 14:30), Steven Rostedt wrote: >>> > +void vm_unmap_user_ram(const void *mem, unsigned int count) >>> > +{ >>> > + unsigned long size = (unsigned long)count << PAGE_SHIFT; >>> > + unsigned long addr = (unsigned long)mem; >>> > + struct vmap_area *va; >>> > + >>> > + might_sleep(); >>> > + BUG_ON(!addr); >>> > + BUG_ON(addr < VMALLOC_START); >>> > + BUG_ON(addr > VMALLOC_END); >>> > + BUG_ON(!PAGE_ALIGNED(addr)); >>> > + >>> > + debug_check_no_locks_freed(mem, size); >>> > + va = find_vmap_area(addr); >>> > + BUG_ON(!va); >>> > + free_unmap_vmap_area(va); >>> > +} >>> > +EXPORT_SYMBOL(vm_unmap_user_ram); >>> > + >>> >>> Noticing this from Sergey's question in another patch, why are you >>> using BUG_ON()? That's rather extreme and something we are trying to >>> avoid adding more of (I still need to remove the BUG_ON()s I've added >>> over ten years ago). I don't see why all these BUG_ON's can't be turned >>> into: >> >> +1 >> >>> if (WARN_ON(x)) >>> return; >> >> Given that this somewhat MM-related, I'd may be say VM_WARN_ON().
I notice that VM_WARN_ON() casts the result of WARN_ON() to (void), so it cannot be used in a if () statement.
VM_WARN_ON() will only warn if CONFIG_DEBUG_VM is set.
Is it really what we want ?
Thanks,
Mathieu
> > Good point, will do! > > So I'll do one cleanup patch for vm_unmap_ram(), and I'll modify the new > vm_unmap_user_ram(). > > Thanks, > > Mathieu > >> >> -ss > > -- > Mathieu Desnoyers > EfficiOS Inc. > http://www.efficios.com
-- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
| |