From: Minchan Kim <minchan@kernel.org> To: Seth Jennings <sjenning@linux.vnet.ibm.com> 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 3/4] zsmalloc: add details to zs_map_object boiler plate Date: Wed, 11 Jul 2012 16:42:29 +0900 [thread overview] Message-ID: <4FFD2E65.5080307@kernel.org> (raw) In-Reply-To: <4FFC478C.4050505@linux.vnet.ibm.com> On 07/11/2012 12:17 AM, Seth Jennings wrote: > On 07/09/2012 09:35 PM, Minchan Kim wrote: >> On 07/03/2012 06:15 AM, Seth Jennings wrote: >>> Add information on the usage limits of zs_map_object() >>> >>> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com> >>> --- >>> drivers/staging/zsmalloc/zsmalloc-main.c | 7 ++++++- >>> 1 file changed, 6 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c >>> index 4942d41..abf7c13 100644 >>> --- a/drivers/staging/zsmalloc/zsmalloc-main.c >>> +++ b/drivers/staging/zsmalloc/zsmalloc-main.c >>> @@ -747,7 +747,12 @@ EXPORT_SYMBOL_GPL(zs_free); >>> * >>> * Before using an object allocated from zs_malloc, it must be mapped using >>> * this function. When done with the object, it must be unmapped using >>> - * zs_unmap_object >>> + * zs_unmap_object. >>> + * >>> + * Only one object can be mapped per cpu at a time. There is no protection >>> + * against nested mappings. >>> + * >>> + * This function returns with preemption and page faults disabled. >>> */ >>> void *zs_map_object(struct zs_pool *pool, unsigned long handle) >>> { >>> >> >> The comment is good but I hope we can detect it automatically with DEBUG >> option. It wouldn't be hard but it's a debug patch so not critical >> until we receive some report about the bug. > > Yes, we could implement some detection scheme later. > >> >> The possibility for nesting is that it is used by irq context. >> >> A uses the mapping >> . >> . >> . >> IRQ happen >> B uses the mapping in IRQ context >> . >> . >> . >> >> Maybe we need local_irq_save/restore in zs_[un]map_object path. > > I'd rather not disable interrupts since that will create > unnecessary interrupt latency for all users, even if they Agreed. Although we guide k[un]map atomic is so fast, it isn't necessary to force irq_[enable|disable]. Okay. > don't need interrupt protection. If a particular user uses > zs_map_object() in an interrupt path, it will be up to that > user to disable interrupts to ensure safety. Nope. It shouldn't do that. Any user in interrupt context can't assume that there isn't any other user using per-cpu buffer right before interrupt happens. The concern is that if such bug happens, it's very hard to find a bug. So, how about adding this? void zs_map_object(...) { BUG_ON(in_interrupt()); } > > Thanks, > Seth > > -- Kind regards, Minchan Kim
WARNING: multiple messages have this Message-ID (diff)
From: Minchan Kim <minchan@kernel.org> To: Seth Jennings <sjenning@linux.vnet.ibm.com> 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 3/4] zsmalloc: add details to zs_map_object boiler plate Date: Wed, 11 Jul 2012 16:42:29 +0900 [thread overview] Message-ID: <4FFD2E65.5080307@kernel.org> (raw) In-Reply-To: <4FFC478C.4050505@linux.vnet.ibm.com> On 07/11/2012 12:17 AM, Seth Jennings wrote: > On 07/09/2012 09:35 PM, Minchan Kim wrote: >> On 07/03/2012 06:15 AM, Seth Jennings wrote: >>> Add information on the usage limits of zs_map_object() >>> >>> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com> >>> --- >>> drivers/staging/zsmalloc/zsmalloc-main.c | 7 ++++++- >>> 1 file changed, 6 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c >>> index 4942d41..abf7c13 100644 >>> --- a/drivers/staging/zsmalloc/zsmalloc-main.c >>> +++ b/drivers/staging/zsmalloc/zsmalloc-main.c >>> @@ -747,7 +747,12 @@ EXPORT_SYMBOL_GPL(zs_free); >>> * >>> * Before using an object allocated from zs_malloc, it must be mapped using >>> * this function. When done with the object, it must be unmapped using >>> - * zs_unmap_object >>> + * zs_unmap_object. >>> + * >>> + * Only one object can be mapped per cpu at a time. There is no protection >>> + * against nested mappings. >>> + * >>> + * This function returns with preemption and page faults disabled. >>> */ >>> void *zs_map_object(struct zs_pool *pool, unsigned long handle) >>> { >>> >> >> The comment is good but I hope we can detect it automatically with DEBUG >> option. It wouldn't be hard but it's a debug patch so not critical >> until we receive some report about the bug. > > Yes, we could implement some detection scheme later. > >> >> The possibility for nesting is that it is used by irq context. >> >> A uses the mapping >> . >> . >> . >> IRQ happen >> B uses the mapping in IRQ context >> . >> . >> . >> >> Maybe we need local_irq_save/restore in zs_[un]map_object path. > > I'd rather not disable interrupts since that will create > unnecessary interrupt latency for all users, even if they Agreed. Although we guide k[un]map atomic is so fast, it isn't necessary to force irq_[enable|disable]. Okay. > don't need interrupt protection. If a particular user uses > zs_map_object() in an interrupt path, it will be up to that > user to disable interrupts to ensure safety. Nope. It shouldn't do that. Any user in interrupt context can't assume that there isn't any other user using per-cpu buffer right before interrupt happens. The concern is that if such bug happens, it's very hard to find a bug. So, how about adding this? void zs_map_object(...) { BUG_ON(in_interrupt()); } > > 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>
next prev parent reply other threads:[~2012-07-11 7:42 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 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 [this message] 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=4FFD2E65.5080307@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=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: linkBe 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.