At Mon, 23 Jul 2012 17:41:27 +0800,
Liu Yuan wrote:
>
> From: Liu Yuan <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
--
sheepdog mailing list
[email protected]
http://lists.wpkg.org/mailman/listinfo/sheepdog
