* [PATCH] staging: zsmalloc: Ensure handle is never 0 on success @ 2013-11-06 0:54 Olav Haugan 2013-11-06 1:17 ` David Cohen 2013-11-06 1:56 ` Greg KH 0 siblings, 2 replies; 12+ messages in thread From: Olav Haugan @ 2013-11-06 0:54 UTC (permalink / raw) To: gregkh; +Cc: ngupta, sjenning, linux-kernel, minchan, linux-arm-msm, Olav Haugan zsmalloc encodes a handle using the page pfn and an object index. On some hardware platforms the pfn could be 0 and this causes the encoded handle to be 0 which is interpreted as an allocation failure. To prevent this false error we ensure that the encoded handle will not be 0 when allocation succeeds. Change-Id: Ifff930dcf254915b497aec5cb36f152a5e5365d6 Signed-off-by: Olav Haugan <ohaugan@codeaurora.org> --- drivers/staging/zsmalloc/zsmalloc-main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c index 523b937..0e32c0f 100644 --- a/drivers/staging/zsmalloc/zsmalloc-main.c +++ b/drivers/staging/zsmalloc/zsmalloc-main.c @@ -441,7 +441,7 @@ static void *obj_location_to_handle(struct page *page, unsigned long obj_idx) } handle = page_to_pfn(page) << OBJ_INDEX_BITS; - handle |= (obj_idx & OBJ_INDEX_MASK); + handle |= ((obj_idx + 1) & OBJ_INDEX_MASK); return (void *)handle; } @@ -451,7 +451,7 @@ static void obj_handle_to_location(unsigned long handle, struct page **page, unsigned long *obj_idx) { *page = pfn_to_page(handle >> OBJ_INDEX_BITS); - *obj_idx = handle & OBJ_INDEX_MASK; + *obj_idx = (handle & OBJ_INDEX_MASK) - 1; } static unsigned long obj_idx_to_offset(struct page *page, -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] staging: zsmalloc: Ensure handle is never 0 on success 2013-11-06 0:54 [PATCH] staging: zsmalloc: Ensure handle is never 0 on success Olav Haugan @ 2013-11-06 1:17 ` David Cohen 2013-11-06 20:56 ` Nitin Gupta 2013-11-06 1:56 ` Greg KH 1 sibling, 1 reply; 12+ messages in thread From: David Cohen @ 2013-11-06 1:17 UTC (permalink / raw) To: Olav Haugan Cc: gregkh, ngupta, sjenning, linux-kernel, minchan, linux-arm-msm Hi Olav, On 11/05/2013 04:54 PM, Olav Haugan wrote: > zsmalloc encodes a handle using the page pfn and an object > index. On some hardware platforms the pfn could be 0 and this > causes the encoded handle to be 0 which is interpreted as an > allocation failure. > > To prevent this false error we ensure that the encoded handle > will not be 0 when allocation succeeds. > > Change-Id: Ifff930dcf254915b497aec5cb36f152a5e5365d6 > Signed-off-by: Olav Haugan <ohaugan@codeaurora.org> > --- > drivers/staging/zsmalloc/zsmalloc-main.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c > index 523b937..0e32c0f 100644 > --- a/drivers/staging/zsmalloc/zsmalloc-main.c > +++ b/drivers/staging/zsmalloc/zsmalloc-main.c > @@ -441,7 +441,7 @@ static void *obj_location_to_handle(struct page *page, unsigned long obj_idx) > } > > handle = page_to_pfn(page) << OBJ_INDEX_BITS; > - handle |= (obj_idx & OBJ_INDEX_MASK); > + handle |= ((obj_idx + 1) & OBJ_INDEX_MASK); As suggestion you could use a macro instead of hardcoded 1. I am not familiar with this code, but if it's a valid test to verify if the resulting address is page aligned, you might want to set this offset macro to a page aligned value as well. > > return (void *)handle; > } > @@ -451,7 +451,7 @@ static void obj_handle_to_location(unsigned long handle, struct page **page, > unsigned long *obj_idx) > { > *page = pfn_to_page(handle >> OBJ_INDEX_BITS); > - *obj_idx = handle & OBJ_INDEX_MASK; > + *obj_idx = (handle & OBJ_INDEX_MASK) - 1; Ditto. Br, David Cohen ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] staging: zsmalloc: Ensure handle is never 0 on success 2013-11-06 1:17 ` David Cohen @ 2013-11-06 20:56 ` Nitin Gupta 2013-11-06 23:46 ` Olav Haugan 0 siblings, 1 reply; 12+ messages in thread From: Nitin Gupta @ 2013-11-06 20:56 UTC (permalink / raw) To: David Cohen Cc: Olav Haugan, Greg Kroah-Hartman, Seth Jennings, linux-kernel, Minchan Kim, linux-arm-msm On Tue, Nov 5, 2013 at 5:17 PM, David Cohen <david.a.cohen@linux.intel.com> wrote: > Hi Olav, > > > On 11/05/2013 04:54 PM, Olav Haugan wrote: >> >> zsmalloc encodes a handle using the page pfn and an object >> index. On some hardware platforms the pfn could be 0 and this >> causes the encoded handle to be 0 which is interpreted as an >> allocation failure. >> >> To prevent this false error we ensure that the encoded handle >> will not be 0 when allocation succeeds. >> >> Change-Id: Ifff930dcf254915b497aec5cb36f152a5e5365d6 >> Signed-off-by: Olav Haugan <ohaugan@codeaurora.org> >> --- >> drivers/staging/zsmalloc/zsmalloc-main.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c >> b/drivers/staging/zsmalloc/zsmalloc-main.c >> index 523b937..0e32c0f 100644 >> --- a/drivers/staging/zsmalloc/zsmalloc-main.c >> +++ b/drivers/staging/zsmalloc/zsmalloc-main.c >> @@ -441,7 +441,7 @@ static void *obj_location_to_handle(struct page *page, >> unsigned long obj_idx) >> } >> >> handle = page_to_pfn(page) << OBJ_INDEX_BITS; >> - handle |= (obj_idx & OBJ_INDEX_MASK); >> + handle |= ((obj_idx + 1) & OBJ_INDEX_MASK); > > > As suggestion you could use a macro instead of hardcoded 1. > > I am not familiar with this code, but if it's a valid test to verify if > the resulting address is page aligned, you might want to set this > offset macro to a page aligned value as well. > > Using a hardcoded 1 looks fine in this case. But the patch description should also be added as a comment for this function. Otherwise, the patch looks good to me. >> >> return (void *)handle; >> } >> @@ -451,7 +451,7 @@ static void obj_handle_to_location(unsigned long >> handle, struct page **page, >> unsigned long *obj_idx) >> { >> *page = pfn_to_page(handle >> OBJ_INDEX_BITS); >> - *obj_idx = handle & OBJ_INDEX_MASK; >> + *obj_idx = (handle & OBJ_INDEX_MASK) - 1; > > > Ditto. > Thanks, Nitin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] staging: zsmalloc: Ensure handle is never 0 on success 2013-11-06 20:56 ` Nitin Gupta @ 2013-11-06 23:46 ` Olav Haugan 0 siblings, 0 replies; 12+ messages in thread From: Olav Haugan @ 2013-11-06 23:46 UTC (permalink / raw) To: Nitin Gupta, David Cohen Cc: Greg Kroah-Hartman, Seth Jennings, linux-kernel, Minchan Kim, linux-arm-msm On 11/6/2013 12:56 PM, Nitin Gupta wrote: > On Tue, Nov 5, 2013 at 5:17 PM, David Cohen > <david.a.cohen@linux.intel.com> wrote: >> Hi Olav, >> >> >> On 11/05/2013 04:54 PM, Olav Haugan wrote: >>> >>> zsmalloc encodes a handle using the page pfn and an object >>> index. On some hardware platforms the pfn could be 0 and this >>> causes the encoded handle to be 0 which is interpreted as an >>> allocation failure. >>> >>> To prevent this false error we ensure that the encoded handle >>> will not be 0 when allocation succeeds. >>> >>> Change-Id: Ifff930dcf254915b497aec5cb36f152a5e5365d6 >>> Signed-off-by: Olav Haugan <ohaugan@codeaurora.org> >>> --- >>> drivers/staging/zsmalloc/zsmalloc-main.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c >>> b/drivers/staging/zsmalloc/zsmalloc-main.c >>> index 523b937..0e32c0f 100644 >>> --- a/drivers/staging/zsmalloc/zsmalloc-main.c >>> +++ b/drivers/staging/zsmalloc/zsmalloc-main.c >>> @@ -441,7 +441,7 @@ static void *obj_location_to_handle(struct page *page, >>> unsigned long obj_idx) >>> } >>> >>> handle = page_to_pfn(page) << OBJ_INDEX_BITS; >>> - handle |= (obj_idx & OBJ_INDEX_MASK); >>> + handle |= ((obj_idx + 1) & OBJ_INDEX_MASK); >> >> >> As suggestion you could use a macro instead of hardcoded 1. >> >> I am not familiar with this code, but if it's a valid test to verify if >> the resulting address is page aligned, you might want to set this >> offset macro to a page aligned value as well. >> >> > > Using a hardcoded 1 looks fine in this case. But the patch description > should also be added as a comment for this function. Otherwise, the patch > looks good to me. > Sure, I can add a comment above obj_location_to_handle() and obj_handle_to_location(). Olav Haugan -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] staging: zsmalloc: Ensure handle is never 0 on success 2013-11-06 0:54 [PATCH] staging: zsmalloc: Ensure handle is never 0 on success Olav Haugan 2013-11-06 1:17 ` David Cohen @ 2013-11-06 1:56 ` Greg KH 2013-11-06 21:09 ` Nitin Gupta 2013-11-07 0:00 ` Olav Haugan 1 sibling, 2 replies; 12+ messages in thread From: Greg KH @ 2013-11-06 1:56 UTC (permalink / raw) To: Olav Haugan; +Cc: ngupta, sjenning, linux-kernel, minchan, linux-arm-msm On Tue, Nov 05, 2013 at 04:54:12PM -0800, Olav Haugan wrote: > zsmalloc encodes a handle using the page pfn and an object > index. On some hardware platforms the pfn could be 0 and this > causes the encoded handle to be 0 which is interpreted as an > allocation failure. What platforms specifically have this issue? > > To prevent this false error we ensure that the encoded handle > will not be 0 when allocation succeeds. > > Change-Id: Ifff930dcf254915b497aec5cb36f152a5e5365d6 What is this? What can anyone do with it? > Signed-off-by: Olav Haugan <ohaugan@codeaurora.org> > --- > drivers/staging/zsmalloc/zsmalloc-main.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c > index 523b937..0e32c0f 100644 > --- a/drivers/staging/zsmalloc/zsmalloc-main.c > +++ b/drivers/staging/zsmalloc/zsmalloc-main.c > @@ -441,7 +441,7 @@ static void *obj_location_to_handle(struct page *page, unsigned long obj_idx) > } > > handle = page_to_pfn(page) << OBJ_INDEX_BITS; > - handle |= (obj_idx & OBJ_INDEX_MASK); > + handle |= ((obj_idx + 1) & OBJ_INDEX_MASK); > > return (void *)handle; > } > @@ -451,7 +451,7 @@ static void obj_handle_to_location(unsigned long handle, struct page **page, > unsigned long *obj_idx) > { > *page = pfn_to_page(handle >> OBJ_INDEX_BITS); > - *obj_idx = handle & OBJ_INDEX_MASK; > + *obj_idx = (handle & OBJ_INDEX_MASK) - 1; > } I need someone who knows how to test this code to ack it before I can take it... And I thought we were deleting zsmalloc anyway, why are you using this code? Isn't it no longer needed anymore? greg k-h ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] staging: zsmalloc: Ensure handle is never 0 on success 2013-11-06 1:56 ` Greg KH @ 2013-11-06 21:09 ` Nitin Gupta 2013-11-06 22:10 ` Greg KH 2013-11-07 0:00 ` Olav Haugan 1 sibling, 1 reply; 12+ messages in thread From: Nitin Gupta @ 2013-11-06 21:09 UTC (permalink / raw) To: Greg KH Cc: Olav Haugan, Seth Jennings, linux-kernel, Minchan Kim, linux-arm-msm On Tue, Nov 5, 2013 at 5:56 PM, Greg KH <gregkh@linuxfoundation.org> wrote: > On Tue, Nov 05, 2013 at 04:54:12PM -0800, Olav Haugan wrote: >> zsmalloc encodes a handle using the page pfn and an object >> index. On some hardware platforms the pfn could be 0 and this >> causes the encoded handle to be 0 which is interpreted as an >> allocation failure. > > What platforms specifically have this issue? > >> >> To prevent this false error we ensure that the encoded handle >> will not be 0 when allocation succeeds. >> >> Change-Id: Ifff930dcf254915b497aec5cb36f152a5e5365d6 > > What is this? What can anyone do with it? > >> Signed-off-by: Olav Haugan <ohaugan@codeaurora.org> >> --- >> drivers/staging/zsmalloc/zsmalloc-main.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c >> index 523b937..0e32c0f 100644 >> --- a/drivers/staging/zsmalloc/zsmalloc-main.c >> +++ b/drivers/staging/zsmalloc/zsmalloc-main.c >> @@ -441,7 +441,7 @@ static void *obj_location_to_handle(struct page *page, unsigned long obj_idx) >> } >> >> handle = page_to_pfn(page) << OBJ_INDEX_BITS; >> - handle |= (obj_idx & OBJ_INDEX_MASK); >> + handle |= ((obj_idx + 1) & OBJ_INDEX_MASK); >> >> return (void *)handle; >> } >> @@ -451,7 +451,7 @@ static void obj_handle_to_location(unsigned long handle, struct page **page, >> unsigned long *obj_idx) >> { >> *page = pfn_to_page(handle >> OBJ_INDEX_BITS); >> - *obj_idx = handle & OBJ_INDEX_MASK; >> + *obj_idx = (handle & OBJ_INDEX_MASK) - 1; >> } > > I need someone who knows how to test this code to ack it before I can > take it... > > And I thought we were deleting zsmalloc anyway, why are you using this > code? Isn't it no longer needed anymore? > zsmalloc is used by zram. Other zstuff has switched to zbud since they need to do shrinking which is much easier to implement with simpler design of zbud. For zram, which is a block device, we don't do such active shrinking, so uses zsmalloc which provides much better density. Nitin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] staging: zsmalloc: Ensure handle is never 0 on success 2013-11-06 21:09 ` Nitin Gupta @ 2013-11-06 22:10 ` Greg KH 2013-11-06 23:46 ` Nitin Gupta 0 siblings, 1 reply; 12+ messages in thread From: Greg KH @ 2013-11-06 22:10 UTC (permalink / raw) To: Nitin Gupta Cc: Olav Haugan, Seth Jennings, linux-kernel, Minchan Kim, linux-arm-msm On Wed, Nov 06, 2013 at 01:09:59PM -0800, Nitin Gupta wrote: > On Tue, Nov 5, 2013 at 5:56 PM, Greg KH <gregkh@linuxfoundation.org> wrote: > > On Tue, Nov 05, 2013 at 04:54:12PM -0800, Olav Haugan wrote: > >> zsmalloc encodes a handle using the page pfn and an object > >> index. On some hardware platforms the pfn could be 0 and this > >> causes the encoded handle to be 0 which is interpreted as an > >> allocation failure. > > > > What platforms specifically have this issue? > > > >> > >> To prevent this false error we ensure that the encoded handle > >> will not be 0 when allocation succeeds. > >> > >> Change-Id: Ifff930dcf254915b497aec5cb36f152a5e5365d6 > > > > What is this? What can anyone do with it? > > > >> Signed-off-by: Olav Haugan <ohaugan@codeaurora.org> > >> --- > >> drivers/staging/zsmalloc/zsmalloc-main.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c > >> index 523b937..0e32c0f 100644 > >> --- a/drivers/staging/zsmalloc/zsmalloc-main.c > >> +++ b/drivers/staging/zsmalloc/zsmalloc-main.c > >> @@ -441,7 +441,7 @@ static void *obj_location_to_handle(struct page *page, unsigned long obj_idx) > >> } > >> > >> handle = page_to_pfn(page) << OBJ_INDEX_BITS; > >> - handle |= (obj_idx & OBJ_INDEX_MASK); > >> + handle |= ((obj_idx + 1) & OBJ_INDEX_MASK); > >> > >> return (void *)handle; > >> } > >> @@ -451,7 +451,7 @@ static void obj_handle_to_location(unsigned long handle, struct page **page, > >> unsigned long *obj_idx) > >> { > >> *page = pfn_to_page(handle >> OBJ_INDEX_BITS); > >> - *obj_idx = handle & OBJ_INDEX_MASK; > >> + *obj_idx = (handle & OBJ_INDEX_MASK) - 1; > >> } > > > > I need someone who knows how to test this code to ack it before I can > > take it... > > > > And I thought we were deleting zsmalloc anyway, why are you using this > > code? Isn't it no longer needed anymore? > > > > zsmalloc is used by zram. Other zstuff has switched to zbud since they > need to do shrinking which is much easier to implement with simpler > design of zbud. For zram, which is a block device, we don't do such > active shrinking, so uses zsmalloc which provides much better density. Ok, so what's the plan of getting these other things out of staging? I'm getting really tired of them hanging around in here for many years now... Should I just remove them if no one is working on getting them merged "properly"? thanks, greg k-h ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] staging: zsmalloc: Ensure handle is never 0 on success 2013-11-06 22:10 ` Greg KH @ 2013-11-06 23:46 ` Nitin Gupta 2013-11-07 3:05 ` Greg KH 0 siblings, 1 reply; 12+ messages in thread From: Nitin Gupta @ 2013-11-06 23:46 UTC (permalink / raw) To: Greg KH Cc: Olav Haugan, Seth Jennings, linux-kernel, Minchan Kim, linux-arm-msm On Wed, Nov 6, 2013 at 2:10 PM, Greg KH <gregkh@linuxfoundation.org> wrote: > On Wed, Nov 06, 2013 at 01:09:59PM -0800, Nitin Gupta wrote: >> On Tue, Nov 5, 2013 at 5:56 PM, Greg KH <gregkh@linuxfoundation.org> wrote: >> > On Tue, Nov 05, 2013 at 04:54:12PM -0800, Olav Haugan wrote: >> >> zsmalloc encodes a handle using the page pfn and an object >> >> index. On some hardware platforms the pfn could be 0 and this >> >> causes the encoded handle to be 0 which is interpreted as an >> >> allocation failure. >> > >> > What platforms specifically have this issue? >> > >> >> >> >> To prevent this false error we ensure that the encoded handle >> >> will not be 0 when allocation succeeds. >> >> >> >> Change-Id: Ifff930dcf254915b497aec5cb36f152a5e5365d6 >> > >> > What is this? What can anyone do with it? >> > >> >> Signed-off-by: Olav Haugan <ohaugan@codeaurora.org> >> >> --- >> >> drivers/staging/zsmalloc/zsmalloc-main.c | 4 ++-- >> >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> >> >> diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c >> >> index 523b937..0e32c0f 100644 >> >> --- a/drivers/staging/zsmalloc/zsmalloc-main.c >> >> +++ b/drivers/staging/zsmalloc/zsmalloc-main.c >> >> @@ -441,7 +441,7 @@ static void *obj_location_to_handle(struct page *page, unsigned long obj_idx) >> >> } >> >> >> >> handle = page_to_pfn(page) << OBJ_INDEX_BITS; >> >> - handle |= (obj_idx & OBJ_INDEX_MASK); >> >> + handle |= ((obj_idx + 1) & OBJ_INDEX_MASK); >> >> >> >> return (void *)handle; >> >> } >> >> @@ -451,7 +451,7 @@ static void obj_handle_to_location(unsigned long handle, struct page **page, >> >> unsigned long *obj_idx) >> >> { >> >> *page = pfn_to_page(handle >> OBJ_INDEX_BITS); >> >> - *obj_idx = handle & OBJ_INDEX_MASK; >> >> + *obj_idx = (handle & OBJ_INDEX_MASK) - 1; >> >> } >> > >> > I need someone who knows how to test this code to ack it before I can >> > take it... >> > >> > And I thought we were deleting zsmalloc anyway, why are you using this >> > code? Isn't it no longer needed anymore? >> > >> >> zsmalloc is used by zram. Other zstuff has switched to zbud since they >> need to do shrinking which is much easier to implement with simpler >> design of zbud. For zram, which is a block device, we don't do such >> active shrinking, so uses zsmalloc which provides much better density. > > Ok, so what's the plan of getting these other things out of staging? Other zstuff: zswap and zcache 1) zswap (along with zbud allocator, frontcache, cleancache) is already out of staging into mm/ (by Seth Jennings) 2) zcache seems to have been completely removed (not sure if Dan ever wants to reintroduce it) > I'm getting really tired of them hanging around in here for many years > now... > Minchan has tried many times to promote zram out of staging. This was his most recent attempt: https://lkml.org/lkml/2013/8/21/54 There he provided arguments for zram inclusion, how it can help in situations where zswap can't and why generalizing /dev/ramX would not be a great idea. So, cannot say why it wasn't picked up for inclusion at that time. > Should I just remove them if no one is working on getting them merged > "properly"? > Please refer the mail thread (link above) and see Minchan's justifications for zram. If they don't sound convincing enough then please remove zram+zsmalloc from staging. Nitin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] staging: zsmalloc: Ensure handle is never 0 on success 2013-11-06 23:46 ` Nitin Gupta @ 2013-11-07 3:05 ` Greg KH 0 siblings, 0 replies; 12+ messages in thread From: Greg KH @ 2013-11-07 3:05 UTC (permalink / raw) To: Nitin Gupta Cc: Olav Haugan, Seth Jennings, linux-kernel, Minchan Kim, linux-arm-msm On Wed, Nov 06, 2013 at 03:46:19PM -0800, Nitin Gupta wrote: > I'm getting really tired of them hanging around in here for many years > > now... > > > > Minchan has tried many times to promote zram out of staging. This was > his most recent attempt: > > https://lkml.org/lkml/2013/8/21/54 > > There he provided arguments for zram inclusion, how it can help in > situations where zswap can't and why generalizing /dev/ramX would > not be a great idea. So, cannot say why it wasn't picked up > for inclusion at that time. > > > Should I just remove them if no one is working on getting them merged > > "properly"? > > > > Please refer the mail thread (link above) and see Minchan's > justifications for zram. > If they don't sound convincing enough then please remove zram+zsmalloc > from staging. You don't need to be convincing me, you need to be convincing the maintainers of the area of the kernel you are working with. And since the last time you all tried to get this merged was back in August, I'm feeling that you all have given up, so it needs to be deleted. I'll go do that for 3.14, and if someone wants to pick it up and merge it properly, they can easily revert it. thanks, greg k-h ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] staging: zsmalloc: Ensure handle is never 0 on success 2013-11-06 1:56 ` Greg KH 2013-11-06 21:09 ` Nitin Gupta @ 2013-11-07 0:00 ` Olav Haugan 2013-11-07 3:06 ` Greg KH 1 sibling, 1 reply; 12+ messages in thread From: Olav Haugan @ 2013-11-07 0:00 UTC (permalink / raw) To: Greg KH; +Cc: ngupta, sjenning, linux-kernel, minchan, linux-arm-msm On 11/5/2013 5:56 PM, Greg KH wrote: > On Tue, Nov 05, 2013 at 04:54:12PM -0800, Olav Haugan wrote: >> zsmalloc encodes a handle using the page pfn and an object >> index. On some hardware platforms the pfn could be 0 and this >> causes the encoded handle to be 0 which is interpreted as an >> allocation failure. > > What platforms specifically have this issue? Currently some of Qualcomm SoC's have physical memory that starts at address 0x0 which causes this problem. I believe this could be a problem on any platforms if memory is configured to be starting at physical address 0x0 for these platforms. >> >> To prevent this false error we ensure that the encoded handle >> will not be 0 when allocation succeeds. >> >> Change-Id: Ifff930dcf254915b497aec5cb36f152a5e5365d6 > > What is this? What can anyone do with it? This is an identifier used by "Gerrit Code Review" to track changes to the same patch. I will remove it. Olav Haugan -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] staging: zsmalloc: Ensure handle is never 0 on success 2013-11-07 0:00 ` Olav Haugan @ 2013-11-07 3:06 ` Greg KH 2013-11-07 22:57 ` Olav Haugan 0 siblings, 1 reply; 12+ messages in thread From: Greg KH @ 2013-11-07 3:06 UTC (permalink / raw) To: Olav Haugan; +Cc: ngupta, sjenning, linux-kernel, minchan, linux-arm-msm On Wed, Nov 06, 2013 at 04:00:02PM -0800, Olav Haugan wrote: > On 11/5/2013 5:56 PM, Greg KH wrote: > > On Tue, Nov 05, 2013 at 04:54:12PM -0800, Olav Haugan wrote: > >> zsmalloc encodes a handle using the page pfn and an object > >> index. On some hardware platforms the pfn could be 0 and this > >> causes the encoded handle to be 0 which is interpreted as an > >> allocation failure. > > > > What platforms specifically have this issue? > > Currently some of Qualcomm SoC's have physical memory that starts at > address 0x0 which causes this problem. Then say this, and list the exact SoC's that can have this problem so people know how to evaluate the bugfix and see if it is relevant for their systems. > I believe this could be a problem > on any platforms if memory is configured to be starting at physical > address 0x0 for these platforms. Have you seen this be a problem? So it's just a theoretical issue at this point in time? > >> To prevent this false error we ensure that the encoded handle > >> will not be 0 when allocation succeeds. > >> > >> Change-Id: Ifff930dcf254915b497aec5cb36f152a5e5365d6 > > > > What is this? What can anyone do with it? > > This is an identifier used by "Gerrit Code Review" to track changes to > the same patch. I will remove it. Please do so, it has no place in kernel patches submitted for acceptance. Please fix up the changelog, and the rest based on the other comments and resend. thanks, greg k-h ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] staging: zsmalloc: Ensure handle is never 0 on success 2013-11-07 3:06 ` Greg KH @ 2013-11-07 22:57 ` Olav Haugan 0 siblings, 0 replies; 12+ messages in thread From: Olav Haugan @ 2013-11-07 22:57 UTC (permalink / raw) To: Greg KH; +Cc: ngupta, sjenning, linux-kernel, minchan, linux-arm-msm On 11/6/2013 7:06 PM, Greg KH wrote: > On Wed, Nov 06, 2013 at 04:00:02PM -0800, Olav Haugan wrote: >> On 11/5/2013 5:56 PM, Greg KH wrote: >>> On Tue, Nov 05, 2013 at 04:54:12PM -0800, Olav Haugan wrote: >>>> zsmalloc encodes a handle using the page pfn and an object >>>> index. On some hardware platforms the pfn could be 0 and this >>>> causes the encoded handle to be 0 which is interpreted as an >>>> allocation failure. >>> >>> What platforms specifically have this issue? >> >> Currently some of Qualcomm SoC's have physical memory that starts at >> address 0x0 which causes this problem. > > Then say this, and list the exact SoC's that can have this problem so > people know how to evaluate the bugfix and see if it is relevant for > their systems. > >> I believe this could be a problem >> on any platforms if memory is configured to be starting at physical >> address 0x0 for these platforms. > > Have you seen this be a problem? So it's just a theoretical issue at > this point in time? Yes, I can consistently reproduce it. It is not just theoretical. Thanks, Olav Haugan -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-11-07 22:57 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-11-06 0:54 [PATCH] staging: zsmalloc: Ensure handle is never 0 on success Olav Haugan 2013-11-06 1:17 ` David Cohen 2013-11-06 20:56 ` Nitin Gupta 2013-11-06 23:46 ` Olav Haugan 2013-11-06 1:56 ` Greg KH 2013-11-06 21:09 ` Nitin Gupta 2013-11-06 22:10 ` Greg KH 2013-11-06 23:46 ` Nitin Gupta 2013-11-07 3:05 ` Greg KH 2013-11-07 0:00 ` Olav Haugan 2013-11-07 3:06 ` Greg KH 2013-11-07 22:57 ` Olav Haugan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).