Re: [PATCH 05/24] ibtrs: client: main functionality

[Bart Van Assche <Bart.VanAssche@wdc.com>]

On Fri, 2018-02-02 at 15:08 +0100, Roman Pen wrote:
> +static inline struct ibtrs_tag *
> +__ibtrs_get_tag(struct ibtrs_clt *clt, enum ibtrs_clt_con_type con_type)
> +{
> +	size_t max_depth = clt->queue_depth;
> +	struct ibtrs_tag *tag;
> +	int cpu, bit;
> +
> +	cpu = get_cpu();
> +	do {
> +		bit = find_first_zero_bit(clt->tags_map, max_depth);
> +		if (unlikely(bit >= max_depth)) {
> +			put_cpu();
> +			return NULL;
> +		}
> +
> +	} while (unlikely(test_and_set_bit_lock(bit, clt->tags_map)));
> +	put_cpu();
> +
> +	tag = GET_TAG(clt, bit);
> +	WARN_ON(tag->mem_id != bit);
> +	tag->cpu_id = cpu;
> +	tag->con_type = con_type;
> +
> +	return tag;
> +}
> +
> +static inline void __ibtrs_put_tag(struct ibtrs_clt *clt,
> +				   struct ibtrs_tag *tag)
> +{
> +	clear_bit_unlock(tag->mem_id, clt->tags_map);
> +}
> +
> +struct ibtrs_tag *ibtrs_clt_get_tag(struct ibtrs_clt *clt,
> +				    enum ibtrs_clt_con_type con_type,
> +				    int can_wait)
> +{
> +	struct ibtrs_tag *tag;
> +	DEFINE_WAIT(wait);
> +
> +	tag = __ibtrs_get_tag(clt, con_type);
> +	if (likely(tag) || !can_wait)
> +		return tag;
> +
> +	do {
> +		prepare_to_wait(&clt->tags_wait, &wait, TASK_UNINTERRUPTIBLE);
> +		tag = __ibtrs_get_tag(clt, con_type);
> +		if (likely(tag))
> +			break;
> +
> +		io_schedule();
> +	} while (1);
> +
> +	finish_wait(&clt->tags_wait, &wait);
> +
> +	return tag;
> +}
> +EXPORT_SYMBOL(ibtrs_clt_get_tag);
> +
> +void ibtrs_clt_put_tag(struct ibtrs_clt *clt, struct ibtrs_tag *tag)
> +{
> +	if (WARN_ON(!test_bit(tag->mem_id, clt->tags_map)))
> +		return;
> +
> +	__ibtrs_put_tag(clt, tag);
> +
> +	/*
> +	 * Putting a tag is a barrier, so we will observe
> +	 * new entry in the wait list, no worries.
> +	 */
> +	if (waitqueue_active(&clt->tags_wait))
> +		wake_up(&clt->tags_wait);
> +}
> +EXPORT_SYMBOL(ibtrs_clt_put_tag);

Do these functions have any advantage over the code in lib/sbitmap.c? If not,
please call the sbitmap functions instead of adding an additional tag allocator.

Thanks,

Bart.