Re: [PATCH] f2fs: introduce f2fs_kmem_cache_alloc to hide the unfailed kmem cache allocation

[Gu Zheng <guz.fnst@cn.fujitsu.com>]

On 10/22/2013 02:15 PM, Haicheng Li wrote:

> On Tue, Oct 22, 2013 at 01:34:26PM +0800, Gu Zheng wrote:
>> On 10/22/2013 01:16 PM, Haicheng Li wrote:
>>
>>> On Tue, Oct 22, 2013 at 11:49:58AM +0800, Gao feng wrote:
>>>> On 10/21/2013 03:24 PM, Gu Zheng wrote:
>>>>> +static inline void *f2fs_kmem_cache_alloc(struct kmem_cache *cachep,
>>>>> +						gfp_t flags)
>>>>> +{
>>>>> +	void *entry = kmem_cache_alloc(cachep, flags);
>>>>> +retry:
>>>>
>>>> retry after kmem_cache_alloc?
>>>
>>> Good catch.
>>>
>>> Sorry for the carelessness in my previous review. 
>>> Besides this, I also found another issue as below:
>>>  
>>>> On Mon, Oct 21, 2013 at 03:24:55PM +0800, Gu Zheng wrote: 
>>>>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c 
>>>>> index ef80f79..fe3cf8e 100644 
>>>>> --- a/fs/f2fs/node.c 
>>>>> +++ b/fs/f2fs/node.c 
>>>>> @@ -1308,11 +1308,7 @@ static int add_free_nid(struct f2fs_nm_info *nm_i, nid_t nid, bool build) 
>>>>>     if (allocated) 
>>>>>             return 0; 
>>>>>  retry: 
>>> -retry?
>>
>> Can be removed here, this tag still used by front goto jumping. But it
>> seems that we need to use another suitable name rather than "retry".
> 
> how about like this?
> ---
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index fe3cf8e..4fa3fd5 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -1296,18 +1296,17 @@ static int add_free_nid(struct f2fs_nm_info *nm_i, nid_t nid, bool build)
>  	if (nid == 0)
>  		return 0;
>  
> -	if (!build)
> -		goto retry;
> +	if (build) {
> +		/* do not add allocated nids */
> +		read_lock(&nm_i->nat_tree_lock);
> +		ne = __lookup_nat_cache(nm_i, nid);
> +		if (ne && nat_get_blkaddr(ne) != NULL_ADDR)
> +			allocated = true;
> +		read_unlock(&nm_i->nat_tree_lock);
> +		if (allocated)
> +			return 0;
> +	}

Good, it's more neat, I'll take this, thanks.:)

Regards,
Gu

>  
> -	/* do not add allocated nids */
> -	read_lock(&nm_i->nat_tree_lock);
> -	ne = __lookup_nat_cache(nm_i, nid);
> -	if (ne && nat_get_blkaddr(ne) != NULL_ADDR)
> -		allocated = true;
> -	read_unlock(&nm_i->nat_tree_lock);
> -	if (allocated)
> -		return 0;
> -retry:
>  	i = f2fs_kmem_cache_alloc(free_nid_slab, GFP_NOFS);
>  	i->nid = nid;
>  	i->state = NID_NEW;
> 



------------------------------------------------------------------------------
October Webinars: Code for Performance
Free Intel webinars can help you accelerate application performance.
Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from 
the latest Intel processors and coprocessors. See abstracts and register >
http://pubads.g.doubleclick.net/gampad/clk?id=60135991&iu=/4140/ostg.clktrk