Re: [PATCH] staging: zsmalloc: Ensure handle is never 0 on success

[Olav Haugan]

On 11/6/2013 12:56 PM, Nitin Gupta wrote:
> On Tue, Nov 5, 2013 at 5:17 PM, David Cohen
> <david.a.cohen@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
>>> ---
>>>   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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/