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;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
