All of lore.kernel.org
 help / color / mirror / Atom feed
From: Seth Jennings <sjenning@linux.vnet.ibm.com>
To: Minchan Kim <minchan@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Dan Magenheimer <dan.magenheimer@oracle.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Nitin Gupta <ngupta@vflare.org>,
	Robert Jennings <rcj@linux.vnet.ibm.com>,
	linux-mm@kvack.org, devel@driverdev.osuosl.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/4] staging: zsmalloc: add gfp flags to zs_create_pool
Date: Mon, 28 Jan 2013 11:01:28 -0600	[thread overview]
Message-ID: <5106AEE8.4060003@linux.vnet.ibm.com> (raw)
In-Reply-To: <20130128033944.GB3321@blaptop>

On 01/27/2013 09:39 PM, Minchan Kim wrote:
> Hi Seth,
> 
> On Fri, Jan 25, 2013 at 11:46:15AM -0600, Seth Jennings 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.
>>
>> This patch changes the meaning of the flags argument in
>> zs_create_pool() to mean the flags for the metadata allocation,
>> and adds a flags argument to zs_malloc that will be used for
>> memory pool growth if required.
> 
> As I mentioned, I'm not strongly against with this patch but it
> should be last resort in case of not being able to address
> frontswap's init routine's dependency with swap_lock.
> 
> I sent a patch and am waiting reply of Konrand or Dan.
> If we can fix frontswap, it would be better rather than
> changing zsmalloc.

I agree that moving the call to frontswap_init() out of the swap_lock
would be a good thing.  However, it doesn't mean that we still
shouldn't allow the users to control the gfp mask for the allocation
done by zs_create_pool(). While moving the frontswap_init() outside
the lock removes the _need_ for this patch, I think that is it good
API design to allow the user to specify the gfp mask.

Seth

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

WARNING: multiple messages have this Message-ID (diff)
From: Seth Jennings <sjenning@linux.vnet.ibm.com>
To: Minchan Kim <minchan@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Dan Magenheimer <dan.magenheimer@oracle.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Nitin Gupta <ngupta@vflare.org>,
	Robert Jennings <rcj@linux.vnet.ibm.com>,
	linux-mm@kvack.org, devel@driverdev.osuosl.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/4] staging: zsmalloc: add gfp flags to zs_create_pool
Date: Mon, 28 Jan 2013 11:01:28 -0600	[thread overview]
Message-ID: <5106AEE8.4060003@linux.vnet.ibm.com> (raw)
In-Reply-To: <20130128033944.GB3321@blaptop>

On 01/27/2013 09:39 PM, Minchan Kim wrote:
> Hi Seth,
> 
> On Fri, Jan 25, 2013 at 11:46:15AM -0600, Seth Jennings 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.
>>
>> This patch changes the meaning of the flags argument in
>> zs_create_pool() to mean the flags for the metadata allocation,
>> and adds a flags argument to zs_malloc that will be used for
>> memory pool growth if required.
> 
> As I mentioned, I'm not strongly against with this patch but it
> should be last resort in case of not being able to address
> frontswap's init routine's dependency with swap_lock.
> 
> I sent a patch and am waiting reply of Konrand or Dan.
> If we can fix frontswap, it would be better rather than
> changing zsmalloc.

I agree that moving the call to frontswap_init() out of the swap_lock
would be a good thing.  However, it doesn't mean that we still
shouldn't allow the users to control the gfp mask for the allocation
done by zs_create_pool(). While moving the frontswap_init() outside
the lock removes the _need_ for this patch, I think that is it good
API design to allow the user to specify the gfp mask.

Seth


  parent reply	other threads:[~2013-01-28 17:05 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-25 17:46 [PATCH 0/4] staging: zsmalloc: various cleanups/improvments Seth Jennings
2013-01-25 17:46 ` Seth Jennings
2013-01-25 17:46 ` [PATCH 1/4] staging: zsmalloc: add gfp flags to zs_create_pool Seth Jennings
2013-01-25 17:46   ` Seth Jennings
2013-01-28  3:39   ` Minchan Kim
2013-01-28  3:39     ` Minchan Kim
2013-01-28 15:16     ` Konrad Rzeszutek Wilk
2013-01-28 15:16       ` Konrad Rzeszutek Wilk
2013-01-28 23:33       ` Minchan Kim
2013-01-28 23:33         ` Minchan Kim
2013-01-28 17:01     ` Seth Jennings [this message]
2013-01-28 17:01       ` Seth Jennings
2013-01-28 23:38       ` Minchan Kim
2013-01-28 23:38         ` Minchan Kim
2013-01-25 17:46 ` [PATCH 2/4] staging: zsmalloc: remove unused pool name Seth Jennings
2013-01-25 17:46   ` Seth Jennings
2013-01-28  3:40   ` Minchan Kim
2013-01-28  3:40     ` Minchan Kim
2013-01-30  4:16   ` Greg Kroah-Hartman
2013-01-30  4:16     ` Greg Kroah-Hartman
2013-01-30 15:36     ` [PATCH] " Seth Jennings
2013-01-30 15:36       ` Seth Jennings
2013-01-30 15:41       ` Seth Jennings
2013-01-30 15:41         ` Seth Jennings
2013-01-30 17:21         ` Greg Kroah-Hartman
2013-01-30 17:21           ` Greg Kroah-Hartman
2013-01-30 17:29           ` Konrad Rzeszutek Wilk
2013-01-30 17:29             ` Konrad Rzeszutek Wilk
2013-01-31  5:32             ` Greg Kroah-Hartman
2013-01-31  5:32               ` Greg Kroah-Hartman
2013-02-01  2:11               ` Konrad Rzeszutek Wilk
2013-02-01  2:11                 ` Konrad Rzeszutek Wilk
2013-01-30 17:33           ` Seth Jennings
2013-01-30 17:33             ` Seth Jennings
2013-01-25 17:46 ` [PATCH 3/4] staging: zsmalloc: add page alloc/free callbacks Seth Jennings
2013-01-25 17:46   ` Seth Jennings
2013-01-28  3:42   ` Minchan Kim
2013-01-28  3:42     ` Minchan Kim
2013-01-25 17:46 ` [PATCH 4/4] staging: zsmalloc: make CLASS_DELTA relative to PAGE_SIZE Seth Jennings
2013-01-25 17:46   ` Seth Jennings
2013-01-28  3:44   ` Minchan Kim
2013-01-28  3:44     ` Minchan Kim
2013-01-28  3:47 ` [PATCH 0/4] staging: zsmalloc: various cleanups/improvments Minchan Kim
2013-01-28  3:47   ` Minchan Kim
2013-01-28 17:07   ` Seth Jennings
2013-01-28 17:07     ` Seth Jennings
2013-01-28 23:44     ` Minchan Kim
2013-01-28 23:44       ` Minchan Kim
2013-01-30  4:17   ` Greg Kroah-Hartman
2013-01-30  4:17     ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5106AEE8.4060003@linux.vnet.ibm.com \
    --to=sjenning@linux.vnet.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=dan.magenheimer@oracle.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=minchan@kernel.org \
    --cc=ngupta@vflare.org \
    --cc=rcj@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.