lkml.org 
[lkml]   [2013]   [Nov]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] staging: zsmalloc: Ensure handle is never 0 on success
From
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


\
 
 \ /
  Last update: 2013-11-06 22:21    [W:0.861 / U:0.312 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site