From: Minchan Kim <minchan@kernel.org>
To: Seth Jennings <sjenning@linux.vnet.ibm.com>
Cc: devel@driverdev.osuosl.org,
Dan Magenheimer <dan.magenheimer@oracle.com>,
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
Andrew Morton <akpm@linux-foundation.org>,
Robert Jennings <rcj@linux.vnet.ibm.com>,
Nitin Gupta <ngupta@vflare.org>,
Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH 1/4] zsmalloc: remove x86 dependency
Date: Wed, 11 Jul 2012 16:27:34 +0900 [thread overview]
Message-ID: <4FFD2AE6.70609@kernel.org> (raw)
In-Reply-To: <4FFC4A61.3020601@linux.vnet.ibm.com>
On 07/11/2012 12:29 AM, Seth Jennings wrote:
> On 07/09/2012 09:21 PM, Minchan Kim wrote:
>> On 07/03/2012 06:15 AM, Seth Jennings wrote:
> <snip>
>>> +static void zs_copy_map_object(char *buf, struct page *firstpage,
>>> + int off, int size)
>>
>> firstpage is rather misleading.
>> As you know, we use firstpage term for real firstpage of zspage but
>> in case of zs_copy_map_object, it could be a middle page of zspage.
>> So I would like to use "page" instead of firstpage.
>
> Accepted.
>
>>> +{
>>> + struct page *pages[2];
>>> + int sizes[2];
>>> + void *addr;
>>> +
>>> + pages[0] = firstpage;
>>> + pages[1] = get_next_page(firstpage);
>>> + BUG_ON(!pages[1]);
>>> +
>>> + sizes[0] = PAGE_SIZE - off;
>>> + sizes[1] = size - sizes[0];
>>> +
>>> + /* disable page faults to match kmap_atomic() return conditions */
>>> + pagefault_disable();
>>
>> If I understand your intention correctly, you want to prevent calling
>> this function on non-atomic context. Right?
>
> This is moved to zs_map_object() in a later patch, but the
> point is to provide uniform return conditions, regardless of
> whether the object to be mapped is contained in a single
> page or spans two pages. kmap_atomic() disables page
> faults, so I did it here to create symmetry. The result is
The one I want to comment out is why we should disable page fault.
ie, if we don't disable page fault, what's happen?
As I read the comment of kmap_atomic about pagefault_disable,
it seems that for preventing reentrant bug in preemptive kernel
while it catch page fault during atomic context in non-preemptive kernel.
But I'm not sure so Ccing Peter.
> that zs_map_object always returns with preemption and page
> faults disabled.
>
> Also, Greg already merged these patches so I'll have to
> incorporate these changes as a separate patch.
>
> Thanks,
> Seth
>
--
Kind regards,
Minchan Kim
--
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: Minchan Kim <minchan@kernel.org>
To: Seth Jennings <sjenning@linux.vnet.ibm.com>
Cc: devel@driverdev.osuosl.org,
Dan Magenheimer <dan.magenheimer@oracle.com>,
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
Andrew Morton <akpm@linux-foundation.org>,
Robert Jennings <rcj@linux.vnet.ibm.com>,
Nitin Gupta <ngupta@vflare.org>,
Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH 1/4] zsmalloc: remove x86 dependency
Date: Wed, 11 Jul 2012 16:27:34 +0900 [thread overview]
Message-ID: <4FFD2AE6.70609@kernel.org> (raw)
In-Reply-To: <4FFC4A61.3020601@linux.vnet.ibm.com>
On 07/11/2012 12:29 AM, Seth Jennings wrote:
> On 07/09/2012 09:21 PM, Minchan Kim wrote:
>> On 07/03/2012 06:15 AM, Seth Jennings wrote:
> <snip>
>>> +static void zs_copy_map_object(char *buf, struct page *firstpage,
>>> + int off, int size)
>>
>> firstpage is rather misleading.
>> As you know, we use firstpage term for real firstpage of zspage but
>> in case of zs_copy_map_object, it could be a middle page of zspage.
>> So I would like to use "page" instead of firstpage.
>
> Accepted.
>
>>> +{
>>> + struct page *pages[2];
>>> + int sizes[2];
>>> + void *addr;
>>> +
>>> + pages[0] = firstpage;
>>> + pages[1] = get_next_page(firstpage);
>>> + BUG_ON(!pages[1]);
>>> +
>>> + sizes[0] = PAGE_SIZE - off;
>>> + sizes[1] = size - sizes[0];
>>> +
>>> + /* disable page faults to match kmap_atomic() return conditions */
>>> + pagefault_disable();
>>
>> If I understand your intention correctly, you want to prevent calling
>> this function on non-atomic context. Right?
>
> This is moved to zs_map_object() in a later patch, but the
> point is to provide uniform return conditions, regardless of
> whether the object to be mapped is contained in a single
> page or spans two pages. kmap_atomic() disables page
> faults, so I did it here to create symmetry. The result is
The one I want to comment out is why we should disable page fault.
ie, if we don't disable page fault, what's happen?
As I read the comment of kmap_atomic about pagefault_disable,
it seems that for preventing reentrant bug in preemptive kernel
while it catch page fault during atomic context in non-preemptive kernel.
But I'm not sure so Ccing Peter.
> that zs_map_object always returns with preemption and page
> faults disabled.
>
> Also, Greg already merged these patches so I'll have to
> incorporate these changes as a separate patch.
>
> Thanks,
> Seth
>
--
Kind regards,
Minchan Kim
next prev parent reply other threads:[~2012-07-11 7:27 UTC|newest]
Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-07-02 21:15 [PATCH 0/4] zsmalloc improvements Seth Jennings
2012-07-02 21:15 ` Seth Jennings
2012-07-02 21:15 ` [PATCH 1/4] zsmalloc: remove x86 dependency Seth Jennings
2012-07-02 21:15 ` Seth Jennings
2012-07-10 2:21 ` Minchan Kim
2012-07-10 2:21 ` Minchan Kim
2012-07-10 15:29 ` Seth Jennings
2012-07-10 15:29 ` Seth Jennings
2012-07-11 7:27 ` Minchan Kim [this message]
2012-07-11 7:27 ` Minchan Kim
2012-07-11 18:26 ` Nitin Gupta
2012-07-11 18:26 ` Nitin Gupta
2012-07-11 20:32 ` Seth Jennings
2012-07-11 20:32 ` Seth Jennings
2012-07-11 22:42 ` Nitin Gupta
2012-07-11 22:42 ` Nitin Gupta
2012-07-12 0:23 ` Seth Jennings
2012-07-12 0:23 ` Seth Jennings
2012-07-02 21:15 ` [PATCH 2/4] zsmalloc: add single-page object fastpath in unmap Seth Jennings
2012-07-02 21:15 ` Seth Jennings
2012-07-10 2:25 ` Minchan Kim
2012-07-10 2:25 ` Minchan Kim
2012-07-02 21:15 ` [PATCH 3/4] zsmalloc: add details to zs_map_object boiler plate Seth Jennings
2012-07-02 21:15 ` Seth Jennings
2012-07-10 2:35 ` Minchan Kim
2012-07-10 2:35 ` Minchan Kim
2012-07-10 15:17 ` Seth Jennings
2012-07-10 15:17 ` Seth Jennings
2012-07-11 7:42 ` Minchan Kim
2012-07-11 7:42 ` Minchan Kim
2012-07-11 14:15 ` Seth Jennings
2012-07-11 14:15 ` Seth Jennings
2012-07-12 1:15 ` Minchan Kim
2012-07-12 1:15 ` Minchan Kim
2012-07-12 19:54 ` Dan Magenheimer
2012-07-12 19:54 ` Dan Magenheimer
2012-07-12 22:46 ` Dan Magenheimer
2012-07-12 22:46 ` Dan Magenheimer
2012-07-02 21:15 ` [PATCH 4/4] zsmalloc: add mapping modes Seth Jennings
2012-07-02 21:15 ` Seth Jennings
2012-07-04 5:33 ` [PATCH 0/4] zsmalloc improvements Minchan Kim
2012-07-04 5:33 ` Minchan Kim
2012-07-04 20:43 ` Konrad Rzeszutek Wilk
2012-07-04 20:43 ` Konrad Rzeszutek Wilk
2012-07-06 15:07 ` Seth Jennings
2012-07-06 15:07 ` Seth Jennings
2012-07-09 13:58 ` Seth Jennings
2012-07-09 13:58 ` Seth Jennings
2012-07-11 19:42 ` Konrad Rzeszutek Wilk
2012-07-11 19:42 ` Konrad Rzeszutek Wilk
2012-07-11 20:48 ` Seth Jennings
2012-07-11 20:48 ` Seth Jennings
2012-07-12 10:40 ` Konrad Rzeszutek Wilk
2012-07-12 10:40 ` Konrad Rzeszutek Wilk
2012-07-11 7:03 ` Minchan Kim
2012-07-11 7:03 ` Minchan Kim
2012-07-11 14:00 ` Seth Jennings
2012-07-11 14:00 ` Seth Jennings
2012-07-12 1:01 ` Minchan Kim
2012-07-12 1:01 ` Minchan Kim
2012-07-11 19:16 ` Seth Jennings
2012-07-11 19:16 ` Seth Jennings
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=4FFD2AE6.70609@kernel.org \
--to=minchan@kernel.org \
--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=ngupta@vflare.org \
--cc=peterz@infradead.org \
--cc=rcj@linux.vnet.ibm.com \
--cc=sjenning@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.