From: Dan Magenheimer <dan.magenheimer@oracle.com> To: Seth Jennings <sjenning@linux.vnet.ibm.com> Cc: Greg Kroah-Hartman <gregkh@suse.de>, Marcus Klemm <marcus.klemm@googlemail.com>, Konrad Wilk <konrad.wilk@oracle.com>, linux-kernel@vger.kernel.org, devel@linuxdriverproject.org, kvm@vger.kernel.org, linux-mm <linux-mm@kvack.org>, Brian King <brking@linux.vnet.ibm.com>, Robert Jennings <rcj@linux.vnet.ibm.com> Subject: RE: [PATCH v2] staging: zcache: support multiple clients, prep for KVM and RAMster Date: Wed, 10 Aug 2011 08:08:57 -0700 (PDT) [thread overview] Message-ID: <2f1abd2b-4c58-46b4-83bd-18c5338de28e@default> (raw) In-Reply-To: <4E429945.1020008@linux.vnet.ibm.com> > From: Seth Jennings [mailto:sjenning@linux.vnet.ibm.com] > Subject: Re: [PATCH v2] staging: zcache: support multiple clients, prep for KVM and RAMster > > > This crash is hit every time a high memory page is swapped out. > > > > I have no solution right now other that to revert this patch and > > restore the original signatures. Hi Seth -- Thanks for your testing. I haven't done much testing on 32-bit. > Sorry for the noise, but I noticed right after I sent this that > the tmem layer doesn't DO anything with the data parameter. So > a possible solution is to just pass the page pointer instead of > the virtual address. After all, pointers are pointers. Yes, this looks like a good patch. > --- a/drivers/staging/zcache/zcache.c > +++ b/drivers/staging/zcache/zcache.c > @@ -1153,7 +1153,7 @@ static void *zcache_pampd_create(char *data, size_t size, > size_t clen; > int ret; > unsigned long count; > - struct page *page = virt_to_page(data); > + struct page *page = (struct page *)(data); > struct zcache_client *cli = pool->client; > uint16_t client_id = get_client_id_from_client(cli); > unsigned long zv_mean_zsize; > @@ -1220,7 +1220,7 @@ static int zcache_pampd_get_data(char *data, size_t *bufsi > int ret = 0; > > BUG_ON(is_ephemeral(pool)); > - zv_decompress(virt_to_page(data), pampd); > + zv_decompress((struct page *)(data), pampd); > return ret; > } > > @@ -1532,7 +1532,7 @@ static int zcache_put_page(int cli_id, int pool_id, struct > goto out; > if (!zcache_freeze && zcache_do_preload(pool) == 0) { > /* preload does preempt_disable on success */ > - ret = tmem_put(pool, oidp, index, page_address(page), > + ret = tmem_put(pool, oidp, index, (char *)(page), > PAGE_SIZE, 0, is_ephemeral(pool)); > if (ret < 0) { > if (is_ephemeral(pool)) > @@ -1565,7 +1565,7 @@ static int zcache_get_page(int cli_id, int pool_id, struct > pool = zcache_get_pool_by_id(cli_id, pool_id); > if (likely(pool != NULL)) { > if (atomic_read(&pool->obj_count) > 0) > - ret = tmem_get(pool, oidp, index, page_address(page), > + ret = tmem_get(pool, oidp, index, (char *)(page), > &size, 0, is_ephemeral(pool)); > zcache_put_pool(pool); > } > > I tested this and it works. > > Dan, does this mess anything else up? Acked-by: Dan Magenheimer <dan.magenheimer@oracle.com> > > What was the rationale for the signature changes? > > Seth The change on the tmem side allows tmem to handle pre-compressed pages, which is useful to RAMster and possibly for KVM. The new "raw" parameter identifies that case, but for zcache "raw" is always zero so your solution looks fine. Seth, could you submit an "official" patch (i.e. proper subject field, signed-off-by) and I will ack that and ask GregKH to queue it up for a 3.1-rc? Subject something like: staging: zcache: fix highmem crash on 32-bit Thanks, Dan
WARNING: multiple messages have this Message-ID (diff)
From: Dan Magenheimer <dan.magenheimer@oracle.com> To: Seth Jennings <sjenning@linux.vnet.ibm.com> Cc: Greg Kroah-Hartman <gregkh@suse.de>, Marcus Klemm <marcus.klemm@googlemail.com>, Konrad Wilk <konrad.wilk@oracle.com>, linux-kernel@vger.kernel.org, devel@linuxdriverproject.org, kvm@vger.kernel.org, linux-mm <linux-mm@kvack.org>, Brian King <brking@linux.vnet.ibm.com>, Robert Jennings <rcj@linux.vnet.ibm.com> Subject: RE: [PATCH v2] staging: zcache: support multiple clients, prep for KVM and RAMster Date: Wed, 10 Aug 2011 08:08:57 -0700 (PDT) [thread overview] Message-ID: <2f1abd2b-4c58-46b4-83bd-18c5338de28e@default> (raw) In-Reply-To: <4E429945.1020008@linux.vnet.ibm.com> > From: Seth Jennings [mailto:sjenning@linux.vnet.ibm.com] > Subject: Re: [PATCH v2] staging: zcache: support multiple clients, prep for KVM and RAMster > > > This crash is hit every time a high memory page is swapped out. > > > > I have no solution right now other that to revert this patch and > > restore the original signatures. Hi Seth -- Thanks for your testing. I haven't done much testing on 32-bit. > Sorry for the noise, but I noticed right after I sent this that > the tmem layer doesn't DO anything with the data parameter. So > a possible solution is to just pass the page pointer instead of > the virtual address. After all, pointers are pointers. Yes, this looks like a good patch. > --- a/drivers/staging/zcache/zcache.c > +++ b/drivers/staging/zcache/zcache.c > @@ -1153,7 +1153,7 @@ static void *zcache_pampd_create(char *data, size_t size, > size_t clen; > int ret; > unsigned long count; > - struct page *page = virt_to_page(data); > + struct page *page = (struct page *)(data); > struct zcache_client *cli = pool->client; > uint16_t client_id = get_client_id_from_client(cli); > unsigned long zv_mean_zsize; > @@ -1220,7 +1220,7 @@ static int zcache_pampd_get_data(char *data, size_t *bufsi > int ret = 0; > > BUG_ON(is_ephemeral(pool)); > - zv_decompress(virt_to_page(data), pampd); > + zv_decompress((struct page *)(data), pampd); > return ret; > } > > @@ -1532,7 +1532,7 @@ static int zcache_put_page(int cli_id, int pool_id, struct > goto out; > if (!zcache_freeze && zcache_do_preload(pool) == 0) { > /* preload does preempt_disable on success */ > - ret = tmem_put(pool, oidp, index, page_address(page), > + ret = tmem_put(pool, oidp, index, (char *)(page), > PAGE_SIZE, 0, is_ephemeral(pool)); > if (ret < 0) { > if (is_ephemeral(pool)) > @@ -1565,7 +1565,7 @@ static int zcache_get_page(int cli_id, int pool_id, struct > pool = zcache_get_pool_by_id(cli_id, pool_id); > if (likely(pool != NULL)) { > if (atomic_read(&pool->obj_count) > 0) > - ret = tmem_get(pool, oidp, index, page_address(page), > + ret = tmem_get(pool, oidp, index, (char *)(page), > &size, 0, is_ephemeral(pool)); > zcache_put_pool(pool); > } > > I tested this and it works. > > Dan, does this mess anything else up? Acked-by: Dan Magenheimer <dan.magenheimer@oracle.com> > > What was the rationale for the signature changes? > > Seth The change on the tmem side allows tmem to handle pre-compressed pages, which is useful to RAMster and possibly for KVM. The new "raw" parameter identifies that case, but for zcache "raw" is always zero so your solution looks fine. Seth, could you submit an "official" patch (i.e. proper subject field, signed-off-by) and I will ack that and ask GregKH to queue it up for a 3.1-rc? Subject something like: staging: zcache: fix highmem crash on 32-bit Thanks, Dan -- 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2011-08-10 15:09 UTC|newest] Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top 2011-06-30 19:01 [PATCH v2] staging: zcache: support multiple clients, prep for KVM and RAMster Dan Magenheimer 2011-06-30 19:01 ` Dan Magenheimer 2011-06-30 19:23 ` Greg KH 2011-06-30 19:23 ` Greg KH 2011-06-30 22:40 ` Dan Carpenter 2011-06-30 22:40 ` Dan Carpenter 2011-06-30 22:40 ` Dan Carpenter 2011-06-30 23:28 ` Dan Magenheimer 2011-06-30 23:28 ` Dan Magenheimer 2011-07-01 8:38 ` Dan Carpenter 2011-07-01 8:38 ` Dan Carpenter 2011-07-01 14:31 ` Dan Magenheimer 2011-07-01 14:31 ` Dan Magenheimer 2011-07-01 16:58 ` Dan Carpenter 2011-07-01 16:58 ` Dan Carpenter 2011-07-06 3:30 ` Greg KH 2011-07-06 3:30 ` Greg KH 2011-08-10 14:21 ` Seth Jennings 2011-08-10 14:21 ` Seth Jennings 2011-08-10 14:44 ` Seth Jennings 2011-08-10 14:44 ` Seth Jennings 2011-08-10 15:08 ` Dan Magenheimer [this message] 2011-08-10 15:08 ` Dan Magenheimer 2011-08-10 15:40 ` Seth Jennings 2011-08-10 15:40 ` Seth Jennings [not found] <1d15f28a-56df-4cf4-9dd9-1032f211c0d0@default20110630224019.GC2544@shale.localdomain>
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=2f1abd2b-4c58-46b4-83bd-18c5338de28e@default \ --to=dan.magenheimer@oracle.com \ --cc=brking@linux.vnet.ibm.com \ --cc=devel@linuxdriverproject.org \ --cc=gregkh@suse.de \ --cc=konrad.wilk@oracle.com \ --cc=kvm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=marcus.klemm@googlemail.com \ --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.