At Mon, 27 Oct 2014 20:32:14 +0900,
tuji wrote:
> 
> Hi, mitake-san,
> 
> Thank you for update.
> I've tested this. and the problem was fixed.
> I add the tag.

Thanks for your tag. Applied this series.

Thanks,
Hitoshi

> 
> Cc: Philip Crotwell <[email protected]>
> Cc: Masahiro Tsuji <[email protected]>
> Signed-off-by: Hitoshi Mitake <[email protected]>
> Tested-by: Masahiro Tsuji <[email protected]>
> 
> 
> > purge_directory() can cause amount of disk I/O because of rmdir(2)
> > and unlink(2). Because they can slow down main thread significantly,
> > it should be done in worker threads for avoiding long request
> > blocking.
> > 
> > Cc: Philip Crotwell <[email protected]>
> > Cc: Masahiro Tsuji <[email protected]>
> > Signed-off-by: Hitoshi Mitake <[email protected]>
> > ---
> >  lib/util.c | 101 
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 93 insertions(+), 8 deletions(-)
> > 
> > v3:
> >  - fix an invalid parameter for realloc()
> > 
> > v2:
> >  - reduce memory consumption caused by work queuing
> > 
> > diff --git a/lib/util.c b/lib/util.c
> > index 82cf28c..21e0143 100644
> > --- a/lib/util.c
> > +++ b/lib/util.c
> > @@ -350,6 +350,56 @@ char *chomp(char *str)
> >     return str;
> >  }
> >  
> > +struct purge_work_unit {
> > +   bool is_dir;
> > +   char path[PATH_MAX];
> > +};
> > +
> > +struct purge_work {
> > +   struct work work;
> > +
> > +   int nr_units, units_size;
> > +   struct purge_work_unit *units;
> > +};
> > +
> > +static void purge_work_fn(struct work *work)
> > +{
> > +   struct purge_work *pw = container_of(work, struct purge_work, work);
> > +   int ret;
> > +
> > +   for (int i = 0 ; i < pw->nr_units; i++) {
> > +           struct purge_work_unit *unit;
> > +
> > +           unit = &pw->units[i];
> > +
> > +           if (unit->is_dir)
> > +                   ret = rmdir_r(unit->path);
> > +           else
> > +                   ret = unlink(unit->path);
> > +
> > +           if (ret)
> > +                   sd_err("failed to remove %s %s: %m",
> > +                          unit->is_dir ? "directory" : "file", unit->path);
> > +
> > +           /*
> > +            * We cannot check and do something even above rmdir_r() and
> > +            * unlink() cause error. Actually, sd_store->cleanup() (typical
> > +            * user of purge_directory()) call of
> > +            * cluster_recovery_completion() ignores its error code.
> > +            */
> > +   }
> > +}
> > +
> > +static void purge_work_done(struct work *work)
> > +{
> > +   struct purge_work *pw = container_of(work, struct purge_work, work);
> > +
> > +   sd_debug("purging work done, number of units: %d", pw->nr_units);
> > +
> > +   free(pw->units);
> > +   free(pw);
> > +}
> > +
> >  /* Purge directory recursively */
> >  int purge_directory(const char *dir_path)
> >  {
> > @@ -358,6 +408,7 @@ int purge_directory(const char *dir_path)
> >     DIR *dir;
> >     struct dirent *d;
> >     char path[PATH_MAX];
> > +   struct purge_work *w = NULL;
> >  
> >     dir = opendir(dir_path);
> >     if (!dir) {
> > @@ -366,6 +417,14 @@ int purge_directory(const char *dir_path)
> >             return -errno;
> >     }
> >  
> > +   if (util_wqueue) {
> > +           /* we have workqueue for it, don't unlink in this thread */
> > +           w = xzalloc(sizeof(*w));
> > +           w->nr_units = 0;
> > +           w->units_size = 512; /* should this value be configurable? */
> > +           w->units = xcalloc(w->units_size, sizeof(w->units[0]));
> > +   }
> > +
> >     while ((d = readdir(dir))) {
> >             if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
> >                     continue;
> > @@ -376,17 +435,43 @@ int purge_directory(const char *dir_path)
> >                     sd_err("failed to stat %s: %m", path);
> >                     goto out;
> >             }
> > -           if (S_ISDIR(s.st_mode))
> > -                   ret = rmdir_r(path);
> > -           else
> > -                   ret = unlink(path);
> >  
> > -           if (ret != 0) {
> > -                   sd_err("failed to remove %s %s: %m",
> > -                          S_ISDIR(s.st_mode) ? "directory" : "file", path);
> > -                   goto out;
> > +           if (util_wqueue) {
> > +                   struct purge_work_unit *unit;
> > +
> > +                   unit = &w->units[w->nr_units++];
> > +
> > +                   unit->is_dir = S_ISDIR(s.st_mode);
> > +                   strcpy(unit->path, path);
> > +
> > +                   if (w->nr_units == w->units_size) {
> > +                           w->units_size *= 2;
> > +                           w->units = xrealloc(w->units,
> > +                                       sizeof(struct purge_work_unit) *
> > +                                               w->units_size);
> > +                   }
> > +           } else {
> > +                   if (S_ISDIR(s.st_mode))
> > +                           ret = rmdir_r(path);
> > +                   else
> > +                           ret = unlink(path);
> > +
> > +                   if (ret != 0) {
> > +                           sd_err("failed to remove %s %s: %m",
> > +                                  S_ISDIR(s.st_mode) ?
> > +                                  "directory" : "file",
> > +                                  path);
> > +                           goto out;
> > +                   }
> >             }
> >     }
> > +
> > +   if (util_wqueue) {
> > +           w->work.fn = purge_work_fn;
> > +           w->work.done = purge_work_done;
> > +           queue_work(util_wqueue, &w->work);
> > +   }
> > +
> >  out:
> >     closedir(dir);
> >     return ret;
> > -- 
> > 1.8.3.2
> 
> --------------------------
> Masahiro Tsuji
> 
> A.T.WORKS, INC
> URL http://www.atworks.co.jp
> 
-- 
sheepdog mailing list
[email protected]
http://lists.wpkg.org/mailman/listinfo/sheepdog

Reply via email to