On Thu, 06 Feb 2014 01:09:25 +0100, Andreas Rohner wrote: > Hi Ryusuke, > > On 2014-02-05 03:16, Andreas Rohner wrote: >> This patch adds an additional parameter min_reclaimable_blks to >> the nilfs_reclaim_params structure. This parameter specifies the >> minimum number of reclaimable blocks in a segment, before it can be >> cleaned. If a segment is below this threshold, it is considered to >> be not worth cleaning, because all the live blocks would need to be >> moved to a new segment, which is expensive, and the number of >> reclaimable blocks is too low. But it is still necessary to update >> the segment usage information to turn the old segment into a new >> one. >> >> This is basically a shortcut to cleaning the segment. It is still >> necessary to read the segment summary information, but the writing >> of the live blocks can be skipped if it's not worth it. >> >> This optimization can be enabled and disabled by the >> use_set_suinfo switch in the configuration file. If the kernel >> doesn't support the set_suinfo ioctl it returns the error >> number ENOTTY, which also permanently disables the optimization. >> >> Signed-off-by: Andreas Rohner <[email protected]> >> --- >> include/nilfs_gc.h | 7 +++--- >> lib/gc.c | 63 >> +++++++++++++++++++++++++++++++++++++++++++++--- >> sbin/cleanerd/cleanerd.c | 18 +++++++++++--- >> 3 files changed, 77 insertions(+), 11 deletions(-) >> >> diff --git a/include/nilfs_gc.h b/include/nilfs_gc.h >> index e49cbf4..1196c55 100644 >> --- a/include/nilfs_gc.h >> +++ b/include/nilfs_gc.h >> @@ -17,18 +17,19 @@ >> /* flags for nilfs_reclaim_params struct */ >> #define NILFS_RECLAIM_PARAM_PROTSEQ (1UL << 0) >> #define NILFS_RECLAIM_PARAM_PROTCNO (1UL << 1) >> -#define __NR_NILFS_RECLAIM_PARAMS 2 >> +#define NILFS_RECLAIM_PARAM_DEFERRABLE (1UL << 2) >> +#define __NR_NILFS_RECLAIM_PARAMS 3 >> >> /** >> * struct nilfs_reclaim_params - structure to specify GC parameters >> * @flags: flags of valid fields >> - * @reserved: padding bytes >> + * @min_reclaimable_blks: minimum number of reclaimable blocks >> * @protseq: start of sequence number of protected segments >> * @protcno: start number of checkpoint to be protected >> */ >> struct nilfs_reclaim_params { >> unsigned long flags; >> - unsigned long reserved; >> + unsigned long min_reclaimable_blks; >> __u64 protseq; >> nilfs_cno_t protcno; >> }; >> diff --git a/lib/gc.c b/lib/gc.c >> index 71c7307..4aa6a2c 100644 >> --- a/lib/gc.c >> +++ b/lib/gc.c >> @@ -29,6 +29,10 @@ >> #include <syslog.h> >> #endif /* HAVE_SYSLOG_H */ >> >> +#if HAVE_SYS_TIME_H >> +#include <sys/time.h> >> +#endif /* HAVE_SYS_TIME */ >> + >> #include <errno.h> >> #include <assert.h> >> #include <stdarg.h> >> @@ -617,8 +621,11 @@ int nilfs_xreclaim_segment(struct nilfs *nilfs, >> struct nilfs_vector *vdescv, *bdescv, *periodv, *vblocknrv; >> sigset_t sigset, oldset, waitset; >> nilfs_cno_t protcno; >> - ssize_t n, ret = -1; >> + ssize_t n, i, ret = -1; >> size_t nblocks; >> + __u32 reclaimable_blocks; >> + struct nilfs_suinfo_update *supv; >> + struct timeval tv; >> >> if (!(params->flags & NILFS_RECLAIM_PARAM_PROTSEQ) || >> (params->flags & (~0UL << __NR_NILFS_RECLAIM_PARAMS))) { >> @@ -703,13 +710,16 @@ int nilfs_xreclaim_segment(struct nilfs *nilfs, >> if (ret < 0) >> goto out_lock; >> >> + reclaimable_blocks = (nilfs_get_blocks_per_segment(nilfs) * n) - >> + (nilfs_vector_get_size(vdescv) + >> + nilfs_vector_get_size(bdescv)); >> + >> if (stat) { >> stat->live_pblks = nilfs_vector_get_size(bdescv); >> stat->defunct_pblks = nblocks - stat->live_pblks; >> >> stat->live_blks = stat->live_vblks + stat->live_pblks; >> - stat->defunct_blks = n * nilfs_get_blocks_per_segment(nilfs) - >> - stat->live_blks; >> + stat->defunct_blks = reclaimable_blocks; >> } >> if (dryrun) >> goto out_lock; >> @@ -725,6 +735,51 @@ int nilfs_xreclaim_segment(struct nilfs *nilfs, >> goto out_lock; >> } >> >> + /* if there are less reclaimable blocks than the >> + * minimal threshold try to update suinfo >> + * instead of cleaning */ >> + if ((params->flags & NILFS_RECLAIM_PARAM_DEFERRABLE) && >> + reclaimable_blocks < params->min_reclaimable_blks * n) { >> + if (stat) >> + stat->deferred_segs = n; >> + >> + ret = gettimeofday(&tv, NULL); >> + if (ret < 0) >> + goto out_lock; >> + >> + supv = malloc(sizeof(struct nilfs_suinfo_update) * n); >> + if (supv == NULL) { >> + ret = -1; >> + goto out_lock; >> + } > > I have already changed nilfs_xreclaim_segments to use > nilfs_vector_create for supv instead of malloc and free directly. Don't > know why I originally used malloc, since the rest of the function uses > the nilfs_vector API.
Ok, I skip this one to review new patches. The current comments on this patch are as follows: 1. Multiline comments should be as follows: /* * if there are less reclaimable blocks than the minimal * threshold try to update suinfo instead of cleaning */ 2. The flag name "NILFS_RECLAIM_PARAM_DEFERRABLE" looks confusing. Why not use NILFS_RECLAIM_PARAM_MIN_RECLAIMABLE_BLKS ? Regards, Ryusuke Konishi -- 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
