On 2014-01-26 11:00, Ryusuke Konishi wrote:
> On Fri, 24 Jan 2014 21:00:03 +0100, Andreas Rohner wrote:
>> On 2014-01-24 19:26, Ryusuke Konishi wrote:
>>>> +          ret = nilfs_set_suinfo(nilfs, supv, n);
>>>> +          free(supv);
>>>> +          if (ret >= 0) {
>>>> +                  /* success, tell caller
>>>> +                   * to try another segment/s */
>>>> +                  ret = -EGCTRYAGAIN;
>>>> +                  goto out_lock;
>>>> +          }
> 
> Returning -EGCTRYAGAIN looks confusing, since posix error code Exxxxx
> is usually stored in errno variable in userland.  It also looks a bit
> hacky.

True. Vyacheslav had similar concerns.

> How about handling this as follows ?
> 
>  - Add a new structure to return result of GC, for instance, as
>    follows:
> 
> struct nilfs_reclaim_status {
>       unsigned int    ncleanedsegs; /* number of segments cleaned
>                                        with nilfs_clean_segments() */
>       unsigned int    ndeferredsegs;/* number of segments deferred
>                                        with nilfs_set_suinfo() */
>       unsigned long   nliveblocks;  /* sum of live virtual blocks and live
>                                        DAT blocks */
>       unsigned long   ndeadblocks;  /* number of reclaimable blocks */
>       unsigned long   nlivevblocks; /* live virtual blocks */
>       unsigned long   nfreevblocks; /* freed virtual block addresses */
>       ...
> };
> 
>  - Use the structure as an argument of the "new" API function
>    (i.e. variant of nilfs_reclaim_segment() )
> 
>  - Rewrite the variant API function so that it returns a success value
>    (0) also for the EGCTRYAGAIN case.
> 
>  - Store other status information in nilfs_reclaim_status structures.
> 
> 
> And, I think adding nilfs_set_suinfo() API should be a separate patch.

Yes EGCTRYAGAIN is not necessary for the nilfs_set_suinfo() API to work.
It is a good idea to use a separate patch for this.

br,
Andreas Rohner

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to