[Sheepdog] [PATCH v4 04/12] farm: add trunk object

[Christoph Hellwig]

> +struct trunk_entry {
> +	uint64_t oid;
> +	unsigned char sha1[SHA1_LEN];
> +};
> +
> +struct __trunk_entry {
> +	struct trunk_entry raw;
> +	int flags;
> +	struct list_head active_list;
> +	struct hlist_node hash;
> +};

The naming seems a bit unfortunate.  It seems like the first one is
the on-disk variant, and the second the incore one.  Maybe all on-disk
structure should get a _disk postfix?  Also what about making the disk
format endian clean and always using __le or __be types like in the
Linux kernel?  The same also applies to the on the wire format.

> +#define trunk_entry_size() sizeof(struct __trunk_entry)

Does this helper really buy you anything?

> +
> +#define HASH_BITS	10
> +#define HASH_SIZE	(1 << HASH_BITS)
> +
> +static LIST_HEAD(trunk_active_list); /* no lock protection */
> +static struct hlist_head trunk_hashtable[HASH_SIZE];
> +static pthread_mutex_t hashtable_lock[HASH_SIZE] = { [0 ... HASH_SIZE - 1] = PTHREAD_MUTEX_INITIALIZER };
> +static unsigned int trunk_entry_active_nr;

Is a hash really a good data structure here?  A tree would scale much
better with the number of OIDs, which will be very different for
different deployments.  For example the xfsprogs code has a nice btree
library to borrow.

> +
> +static inline struct __trunk_entry *alloc_trunk_entry(void)
> +{
> +	return (struct __trunk_entry *)malloc(trunk_entry_size());
> +}


No need for the case here.

> +	struct dirent *d;
> +	uint64_t oid;
> +
> +	dir = opendir(obj_path);
> +	if (!dir)
> +		return -1;
> +
> +	while ((d = readdir(dir))) {
> +		if (!strncmp(d->d_name, ".", 1))
> +			continue;
> +		oid = strtoull(d->d_name, NULL, 16);
> +		if (oid == 0)
> +			continue;

You should also check for ULLONG_MAX in case we ended up with a
non-numerical entry in the directory for some reason.