[sheepdog] [PATCH 3/6] lib: implement wrapper functions for handling EMFILE gracefully

Liu Yuan namei.unix at gmail.com
Mon Aug 19 10:32:39 CEST 2013


On Mon, Aug 19, 2013 at 05:26:52PM +0900, Hitoshi Mitake wrote:
> At Mon, 19 Aug 2013 16:19:53 +0800,
> Liu Yuan wrote:
> > 
> > On Mon, Aug 19, 2013 at 05:16:38PM +0900, Hitoshi Mitake wrote:
> > > At Mon, 19 Aug 2013 14:53:19 +0800,
> > > Liu Yuan wrote:
> > > > 
> > > > On Wed, Aug 14, 2013 at 01:27:38AM +0900, Hitoshi Mitake wrote:
> > > > > This patch adds wrapper functions of fd creaters for handling EMFILE
> > > > > gracefully. When they face EMFILE, they reap unused sockfd and retry
> > > > > creating new fds.
> > > > > 
> > > > > This patch also modifies Makefile of shepherd. Because this patch
> > > > > creates a new dependency in libsheepdog, shepherd must linked with
> > > > > pthread.
> > > > > 
> > > > > Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> > > > > ---
> > > > >  include/util.h       |  14 +++++++
> > > > >  lib/util.c           | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > >  shepherd/Makefile.am |   2 +-
> > > > >  3 files changed, 119 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/include/util.h b/include/util.h
> > > > > index ff1f0c7..729037d 100644
> > > > > --- a/include/util.h
> > > > > +++ b/include/util.h
> > > > > @@ -13,6 +13,9 @@
> > > > >  #include <sys/eventfd.h>
> > > > >  #include <pthread.h>
> > > > >  #include <errno.h>
> > > > > +#include <sys/types.h>
> > > > > +#include <sys/socket.h>
> > > > > +#include <fcntl.h>
> > > > >  
> > > > >  #include "logger.h"
> > > > >  #include "list.h"
> > > > > @@ -344,4 +347,15 @@ static inline bool is_stdout_console(void)
> > > > >  extern mode_t sd_def_fmode;
> > > > >  extern mode_t sd_def_dmode;
> > > > >  
> > > > > +/*
> > > > > + * The below x- prefixed wrappers are for handling EMFILE gracefully.
> > > > > + * They reap fds of sockfd cache and retry when they face EMFILE.
> > > > > + */
> > > > > +int xsocket(int domain, int type, int protocol);
> > > > > +int xaccept(int sockfd, struct sockaddr *addr, socklen_t *addrlen);
> > > > > +int xeventfd(int initval, int flags);
> > > > > +int xopen(const char *pathname, int flags, ...);
> > > > > +int xtimerfd_create(clockid_t clock_id, int flags);
> > > > > +int xepoll_create(int size);
> > > > > +
> > > > >  #endif
> > > > > diff --git a/lib/util.c b/lib/util.c
> > > > > index 9781fd8..b88f297 100644
> > > > > --- a/lib/util.c
> > > > > +++ b/lib/util.c
> > > > > @@ -22,8 +22,12 @@
> > > > >  #include <signal.h>
> > > > >  #include <sys/xattr.h>
> > > > >  #include <fcntl.h>
> > > > > +#include <stdarg.h>
> > > > > +#include <sys/timerfd.h>
> > > > > +#include <sys/epoll.h>
> > > > >  
> > > > >  #include "util.h"
> > > > > +#include "sockfd_cache.h"
> > > > >  
> > > > >  mode_t sd_def_dmode = S_IRUSR | S_IWUSR | S_IXUSR | S_IRGRP | S_IWGRP | S_IXGRP;
> > > > >  mode_t sd_def_fmode = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP;
> > > > > @@ -735,3 +739,103 @@ void list_sort(void *priv, struct list_head *head,
> > > > >  
> > > > >  	merge_and_restore_back_links(priv, cmp, head, part[max_lev], list);
> > > > >  }
> > > > > +
> > > > > +int xsocket(int domain, int type, int protocol)
> > > > > +{
> > > > > +	int fd;
> > > > > +
> > > > > +retry:
> > > > > +	fd = socket(domain, type, protocol);
> > > > > +	if (fd < 0) {
> > > > > +		if (errno == EINTR)
> > > > > +			goto retry;
> > > > > +
> > > > > +		if (errno == EMFILE && sockfd_shrink())
> > > > > +			goto retry;
> > > > > +	}
> > > > 
> > > > I think we should loop until some fds are reclaimed in the sockfd_shrink. After
> > > > it returns, it means we have free slots in the sockfd cache.
> > > 
> > > In theory, the situation that sockfd has no fds is possible and it can
> > > block users forever. I think returning with boolean result is a better
> > > way.
> > 
> > What exact sitatution it will block *forever*? If this happens, it is a bug to
> > fix.
> 
> e.g. fds are exhausted by files. Of course the situation would be hard
> to produce but I think it is a valid one.

This doesn't meant it loops *forever*.

In the case of all the fds are in use, we should actually wait, not error return
to the caller and ask the caller to handle this. For e.g,

xeventfd_create should not return to the caller in the case of fds are all in
use. What the caller would do might be panic the sheep.

Thanks
Yuan



More information about the sheepdog mailing list