Re: [PATCH v2 09/32] elx: libefc: Emulex FC discovery library APIs and definitions

[Hannes Reinecke <hare@suse.de>]

On 12/20/19 11:37 PM, James Smart wrote:
> This patch continues the libefc library population.
> 
> This patch adds library interface definitions for:
> - SLI/Local FC port objects
> - efc_domain_s: FC domain (aka fabric) objects
> - efc_node_s: FC node (aka remote ports) objects
> - A sparse vector interface that manages lookup tables
>   for the objects.
> 
> Signed-off-by: Ram Vegesna <ram.vegesna@broadcom.com>
> Signed-off-by: James Smart <jsmart2021@gmail.com>
> ---
>  drivers/scsi/elx/libefc/efc.h     |  99 ++++++
>  drivers/scsi/elx/libefc/efc_lib.c | 131 ++++++++
>  drivers/scsi/elx/libefc/efclib.h  | 637 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 867 insertions(+)
>  create mode 100644 drivers/scsi/elx/libefc/efc.h
>  create mode 100644 drivers/scsi/elx/libefc/efc_lib.c
>  create mode 100644 drivers/scsi/elx/libefc/efclib.h
> 
> diff --git a/drivers/scsi/elx/libefc/efc.h b/drivers/scsi/elx/libefc/efc.h
> new file mode 100644
> index 000000000000..ef7c83e44167
> --- /dev/null
> +++ b/drivers/scsi/elx/libefc/efc.h
> @@ -0,0 +1,99 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2019 Broadcom. All Rights Reserved. The term
> + * “Broadcom” refers to Broadcom Inc. and/or its subsidiaries.
> + */
> +
> +#ifndef __EFC_H__
> +#define __EFC_H__
> +
> +#include "../include/efc_common.h"
> +#include "efclib.h"
> +#include "efc_sm.h"
> +#include "efc_domain.h"
> +#include "efc_sport.h"
> +#include "efc_node.h"
> +#include "efc_fabric.h"
> +#include "efc_device.h"
> +
> +#define EFC_MAX_REMOTE_NODES			2048
> +
> +enum efc_hw_rtn {
> +	EFC_HW_RTN_SUCCESS = 0,
> +	EFC_HW_RTN_SUCCESS_SYNC = 1,
> +	EFC_HW_RTN_ERROR = -1,
> +	EFC_HW_RTN_NO_RESOURCES = -2,
> +	EFC_HW_RTN_NO_MEMORY = -3,
> +	EFC_HW_RTN_IO_NOT_ACTIVE = -4,
> +	EFC_HW_RTN_IO_ABORT_IN_PROGRESS = -5,
> +	EFC_HW_RTN_IO_PORT_OWNED_ALREADY_ABORTED = -6,
> +	EFC_HW_RTN_INVALID_ARG = -7,
> +};
> +

(Silent applause for the named enum :-)

> +#define EFC_HW_RTN_IS_ERROR(e) ((e) < 0)
> +
> +enum efc_scsi_del_initiator_reason {
> +	EFC_SCSI_INITIATOR_DELETED,
> +	EFC_SCSI_INITIATOR_MISSING,
> +};
> +
> +enum efc_scsi_del_target_reason {
> +	EFC_SCSI_TARGET_DELETED,
> +	EFC_SCSI_TARGET_MISSING,
> +};
> +
> +#define EFC_SCSI_CALL_COMPLETE			0
> +#define EFC_SCSI_CALL_ASYNC			1
> +
> +#define EFC_FC_ELS_DEFAULT_RETRIES		3
> +
> +/* Timeouts */
> +#define EFC_FC_ELS_SEND_DEFAULT_TIMEOUT		0
> +#define EFC_FC_FLOGI_TIMEOUT_SEC		5
> +#define EFC_FC_DOMAIN_SHUTDOWN_TIMEOUT_USEC	30000000
> +
> +#define domain_sm_trace(domain) \
> +	efc_log_debug(domain->efc, "[domain:%s] %-20s %-20s\n", \
> +		      domain->display_name, __func__, efc_sm_event_name(evt)) \
> +
> +#define domain_trace(domain, fmt, ...) \
> +	efc_log_debug(domain->efc, \
> +		      "[%s]" fmt, domain->display_name, ##__VA_ARGS__) \
> +
> +#define node_sm_trace() \
> +	efc_log_debug(node->efc, \
> +		"[%s] %-20s\n", node->display_name, efc_sm_event_name(evt)) \
> +
> +#define sport_sm_trace(sport) \
> +	efc_log_debug(sport->efc, \
> +		"[%s] %-20s\n", sport->display_name, efc_sm_event_name(evt)) \
> +
> +/**
> + * Sparse Vector API
> + *
> + * This is a trimmed down sparse vector implementation tuned to the problem of
> + * 24-bit FC_IDs. In this case, the 24-bit index value is broken down in three
> + * 8-bit values. These values are used to index up to three 256 element arrays.
> + * Arrays are allocated, only when needed. @n @n

@n @n ?

> + * The lookup can complete in constant time (3 indexed array references). @n @n
> + * A typical use case would be that the fabric/directory FC_IDs would cause two
> + * rows to be allocated, and the fabric assigned remote nodes would cause two
> + * rows to be allocated, with the root row always allocated. This gives five
> + * rows of 256 x sizeof(void*), resulting in 10k.
> + */
> +
> +struct sparse_vector {
> +	struct efc *efc;
> +	u32 max_idx;
> +	void **array;
> +};
> +
> +#define SPV_ROWLEN	256
> +#define SPV_DIM		3
> +
Hmm. One wonders if xarrays wouldn't work better (and simpler to implement).

Have you looked at that?

> +void efc_spv_del(struct sparse_vector *spv);
> +struct sparse_vector *efc_spv_new(struct efc *efc);
> +void efc_spv_set(struct sparse_vector *sv, u32 idx, void *value);
> +void *efc_spv_get(struct sparse_vector *sv, u32 idx);
> +
> +#endif /* __EFC_H__ */
> diff --git a/drivers/scsi/elx/libefc/efc_lib.c b/drivers/scsi/elx/libefc/efc_lib.c
> new file mode 100644
> index 000000000000..9ab8538d6e1f
> --- /dev/null
> +++ b/drivers/scsi/elx/libefc/efc_lib.c
> @@ -0,0 +1,131 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 Broadcom. All Rights Reserved. The term
> + * “Broadcom” refers to Broadcom Inc. and/or its subsidiaries.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include "efc.h"
> +
> +int efcport_init(struct efc *efc)
> +{
> +	u32 rc = 0;
> +
> +	spin_lock_init(&efc->lock);
> +	INIT_LIST_HEAD(&efc->vport_list);
> +
> +	/* Create Node pool */
> +	rc = efc_node_create_pool(efc, EFC_MAX_REMOTE_NODES);
> +	if (rc)
> +		efc_log_err(efc, "Can't allocate node pool\n");
> +
> +	return rc;
> +}
> +
> +void efcport_destroy(struct efc *efc)
> +{
> +	efc_node_free_pool(efc);
> +}
> +
> +static void **efc_spv_new_row(u32 rowcount)
> +{
> +	return kzalloc(sizeof(void *) * rowcount, GFP_ATOMIC);
> +}
> +
> +/* Recursively delete the rows in this sparse vector */
> +static void
> +_efc_spv_del(struct efc *efc, void **a, u32 n, u32 depth)
> +{
> +	if (a) {
> +		if (depth) {
> +			u32 i;
> +
> +			for (i = 0; i < n; i++)
> +				_efc_spv_del(efc, a[i], n, depth - 1);
> +
> +			kfree(a);
> +		}
> +	}
> +}
> +
> +void
> +efc_spv_del(struct sparse_vector *spv)
> +{
> +	if (spv) {
> +		_efc_spv_del(spv->efc, spv->array, SPV_ROWLEN, SPV_DIM);
> +		kfree(spv);
> +	}
> +}
> +
> +struct sparse_vector
> +*efc_spv_new(struct efc *efc)
> +{
> +	struct sparse_vector *spv;
> +	u32 i;
> +
> +	spv = kzalloc(sizeof(*spv), GFP_ATOMIC);
> +	if (!spv)
> +		return NULL;
> +
> +	spv->efc = efc;
> +	spv->max_idx = 1;
> +	for (i = 0; i < SPV_DIM; i++)
> +		spv->max_idx *= SPV_ROWLEN;
> +
> +	return spv;
> +}
> +
> +static void
> +*efc_spv_new_cell(struct sparse_vector *sv, u32 idx, bool alloc_new_rows)
> +{
> +	void **p;
> +	u32 a = (idx >> 16) & 0xff;
> +	u32 b = (idx >>  8) & 0xff;
> +	u32 c = (idx >>  0) & 0xff;
> +
> +	if (idx >= sv->max_idx)
> +		return NULL;
> +
> +	if (!sv->array) {
> +		sv->array = (alloc_new_rows ?
> +			     efc_spv_new_row(SPV_ROWLEN) : NULL);
> +		if (!sv->array)
> +			return NULL;
> +	}
> +	p = sv->array;
> +	if (!p[a]) {
> +		p[a] = (alloc_new_rows ? efc_spv_new_row(SPV_ROWLEN) : NULL);
> +		if (!p[a])
> +			return NULL;
> +	}
> +	p = p[a];
> +	if (!p[b]) {
> +		p[b] = (alloc_new_rows ? efc_spv_new_row(SPV_ROWLEN) : NULL);
> +		if (!p[b])
> +			return NULL;
> +	}
> +	p = p[b];
> +
> +	return &p[c];
> +}
> +
> +void
> +efc_spv_set(struct sparse_vector *sv, u32 idx, void *value)
> +{
> +	void **ref = efc_spv_new_cell(sv, idx, true);
> +
> +	if (ref)
> +		*ref = value;
> +}
> +
> +void
> +*efc_spv_get(struct sparse_vector *sv, u32 idx)
> +{
> +	void **ref = efc_spv_new_cell(sv, idx, false);
> +
> +	if (ref)
> +		return *ref;
> +
> +	return NULL;
> +}
What about locking?

But maybe it'll clarify itself with the next patches.
Let's see.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      Teamlead Storage & Networking
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer