At Mon, 23 Jul 2012 17:41:27 +0800, Liu Yuan wrote: > > From: Liu Yuan <tailai.ly at taobao.com> > > v2: > - fix memory leak in check_idx() > - remove extra whitespace > ---------------------------------- >8 > > This will scale sheep daemon to serve more VMs on one node and do > it adoptively and automatically. > > - fd count default to 16 instead of previous 8 > > Signed-off-by: Liu Yuan <tailai.ly at taobao.com> > --- > sheep/sheep.c | 1 + > sheep/sheep_priv.h | 1 + > sheep/sockfd_cache.c | 127 ++++++++++++++++++++++++++++++++++++++------------ > 3 files changed, 98 insertions(+), 31 deletions(-) > > diff --git a/sheep/sheep.c b/sheep/sheep.c > index df28a94..380a129 100644 > --- a/sheep/sheep.c > +++ b/sheep/sheep.c > @@ -344,6 +344,7 @@ int main(int argc, char **argv) > sys->recovery_wqueue = init_work_queue("recovery", true); > sys->deletion_wqueue = init_work_queue("deletion", true); > sys->block_wqueue = init_work_queue("block", true); > + sys->sockfd_wqueue = init_work_queue("sockfd", true); > if (!sys->gateway_wqueue || !sys->io_wqueue ||!sys->recovery_wqueue || > !sys->deletion_wqueue || !sys->block_wqueue) > exit(1); > diff --git a/sheep/sheep_priv.h b/sheep/sheep_priv.h > index e455d27..530fe14 100644 > --- a/sheep/sheep_priv.h > +++ b/sheep/sheep_priv.h > @@ -122,6 +122,7 @@ struct cluster_info { > struct work_queue *deletion_wqueue; > struct work_queue *recovery_wqueue; > struct work_queue *block_wqueue; > + struct work_queue *sockfd_wqueue; > }; > > struct siocb { > diff --git a/sheep/sockfd_cache.c b/sheep/sockfd_cache.c > index aa90299..c130ea6 100644 > --- a/sheep/sockfd_cache.c > +++ b/sheep/sockfd_cache.c > @@ -48,23 +48,33 @@ static struct sockfd_cache sockfd_cache = { > .lock = PTHREAD_RWLOCK_INITIALIZER, > }; > > +/* > + * Suppose request size from Guest is 512k, then 4M / 512k = 8, so at > + * most 8 requests can be issued to the same sheep object. Based on this > + * assumption, '16' would be effecient for servers that only host 2~4 > + * Guests. > + * > + * This fd count will be dynamically grown when the idx reaches watermark. > + */ > +#define DEFAULT_FDS_COUNT 16 > +#define DEFAULT_FDS_WATERMARK 12 Should comment that the value is (16 * 3 / 4)? > + > +/* How many FDs we cache for one node */ > +static int fds_count = DEFAULT_FDS_COUNT; > + > +struct sockfd_cache_fd { > + int fd; > + uint8_t fd_in_use; Simply, how about 'in_use'? > +}; > + > struct sockfd_cache_entry { > struct rb_node rb; > struct node_id nid; > -#define SOCKFD_CACHE_MAX_FD 8 /* How many FDs we cache for one node */ > - /* > - * FIXME: Make this macro configurable. > - * > - * Suppose request size from Guest is 512k, then 4M / 512k = 8, so at > - * most 8 requests can be issued to the same sheep object. Based on this > - * assumption, '8' would be effecient for servers that only host 4~8 > - * Guests, but for powerful servers that can host dozens of Guests, we > - * might consider bigger value. > - */ > - int fd[SOCKFD_CACHE_MAX_FD]; > - uint8_t fd_in_use[SOCKFD_CACHE_MAX_FD]; > + struct sockfd_cache_fd *fds; > }; > > +#define fds_bytes (sizeof(struct sockfd_cache_fd) * fds_count) > + > static struct sockfd_cache_entry * > sockfd_cache_insert(struct sockfd_cache_entry *new) > { > @@ -118,8 +128,8 @@ static inline int get_free_slot(struct sockfd_cache_entry *entry) > { > int idx = -1, i; > > - for (i = 0; i < SOCKFD_CACHE_MAX_FD; i++) { > - if (uatomic_cmpxchg(&entry->fd_in_use[i], 0, 1)) > + for (i = 0; i < fds_count; i++) { > + if (uatomic_cmpxchg(&entry->fds[i].fd_in_use, 0, 1)) > continue; > idx = i; > break; > @@ -155,8 +165,8 @@ out: > static inline bool slots_all_free(struct sockfd_cache_entry *entry) > { > int i; > - for (i = 0; i < SOCKFD_CACHE_MAX_FD; i++) > - if (uatomic_read(&entry->fd_in_use[i])) > + for (i = 0; i < fds_count; i++) > + if (uatomic_read(&entry->fds[i].fd_in_use)) > return false; > return true; > } > @@ -164,9 +174,9 @@ static inline bool slots_all_free(struct sockfd_cache_entry *entry) > static inline void destroy_all_slots(struct sockfd_cache_entry *entry) > { > int i; > - for (i = 0; i < SOCKFD_CACHE_MAX_FD; i++) > - if (entry->fd[i] != -1) > - close(entry->fd[i]); > + for (i = 0; i < fds_count; i++) > + if (entry->fds[i].fd != -1) > + close(entry->fds[i].fd); > } > > /* > @@ -220,11 +230,12 @@ void sockfd_cache_del(struct node_id *nid) > > static void sockfd_cache_add_nolock(struct node_id *nid) > { > - struct sockfd_cache_entry *new = xzalloc(sizeof(*new)); > + struct sockfd_cache_entry *new = xmalloc(sizeof(*new)); > int i; > > - for (i = 0; i < SOCKFD_CACHE_MAX_FD; i++) > - new->fd[i] = -1; > + new->fds = xzalloc(fds_bytes); Isn't it easier to understand to write directly new->fds = xzalloc(sizeof(struct sockfd_cache_fd) * fds_count) than using a macro? The other parts look good to me. Thanks, Kazutaka |