> From: Seth Jennings [mailto:sjenning@xxxxxxxxxxxxxxxxxx] > Subject: Re: [PATCHv2 1/9] staging: zsmalloc: add gfp flags to zs_create_pool > > On 01/24/2013 07:33 PM, Minchan Kim wrote: > > Hi Seth, frontswap guys > > > > On Tue, Jan 8, 2013 at 5:24 AM, Seth Jennings > > <sjenning@xxxxxxxxxxxxxxxxxx> wrote: > >> zs_create_pool() currently takes a gfp flags argument > >> that is used when growing the memory pool. However > >> it is not used in allocating the metadata for the pool > >> itself. That is currently hardcoded to GFP_KERNEL. > >> > >> zswap calls zs_create_pool() at swapon time which is done > >> in atomic context, resulting in a "might sleep" warning. > > > > I didn't review this all series, really sorry but totday I saw Nitin > > added Acked-by so I'm afraid Greg might get it under my radar. I'm not > > strong against but I would like know why we should call frontswap_init > > under swap_lock? Is there special reason? > > The call stack is: > > SYSCALL_DEFINE2(swapon.. <-- swapon_mutex taken here > enable_swap_info() <-- swap_lock taken here > frontswap_init() > __frontswap_init() > zswap_frontswap_init() > zs_create_pool() > > It isn't entirely clear to me why frontswap_init() is called under > lock. Then again, I'm not entirely sure what the swap_lock protects. > There are no comments near the swap_lock definition to tell me. > > I would guess that the intent is to block any writes to the swap > device until frontswap_init() has completed. > > Dan care to weigh in? I think frontswap's first appearance needs to be atomic, i.e. the transition from (a) frontswap is not present and will fail all calls, to (b) frontswap is fully functional... that transition must be atomic. And, once Konrad's module patches are in, the opposite transition must be atomic also. But there are most likely other ways to do those transitions atomically that don't need to hold swap_lock. Honestly, I never really focused on the initialization code so I am very open to improvements as long as they work for all the various frontswap backends. Dan _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel