On 07/19/2010 10:06 AM, Minchan Kim wrote: > Hi Nitin, > > On Sun, Jul 18, 2010 at 5:21 PM, Nitin Gupta <ngupta@xxxxxxxxxx> wrote: >> On 07/18/2010 01:23 PM, Pekka Enberg wrote: >>> Nitin Gupta wrote: >>>> @@ -528,17 +581,32 @@ static int zcache_store_page(struct zcache_inode_rb *znode, >>>> goto out; >>>> } >>>> >>>> - dest_data = kmap_atomic(zpage, KM_USER0); >>>> + local_irq_save(flags); >>> >>> Does xv_malloc() required interrupts to be disabled? If so, why doesn't the function do it by itself? >>> >> >> >> xvmalloc itself doesn't require disabling interrupts but zcache needs that since >> otherwise, we can have deadlock between xvmalloc pool lock and mapping->tree_lock >> which zcache_put_page() is called. OTOH, zram does not require this disabling of >> interrupts. So, interrupts are disable separately for zcache case. > > cleancache_put_page always is called with spin_lock_irq. > Couldn't we replace spin_lock_irq_save with spin_lock? > I was missing this point regarding cleancache_put(). So, we can now: - take plain (non-irq) spin_lock in zcache_put_page() - take non-irq rwlock in zcache_inode_create() which is called only by zcache_put_page(). - Same applies to zcache_store_page(). So, we can also get rid of unnecessary preempt_disable()/enable() in this function. I will put up a comment for all these functions and make these changes. Thanks, Nitin -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxxx For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>