On 10/08/2014 01:38 PM, Tadeusz Struk wrote:
> In a system with NUMA configuration we want to enforce that the accelerator is
> connected to a node with memory to avoid cross QPI memory transaction.
> Otherwise there is no point in using the accelerator as the encryption in
> software will be faster.
>
> Signed-off-by: Tadeusz Struk <[email protected]>
> ---
> drivers/crypto/qat/qat_common/adf_accel_devices.h | 3 +--
> drivers/crypto/qat/qat_common/adf_transport.c | 12 +++++++-----
> drivers/crypto/qat/qat_common/qat_algs.c | 5 +++--
> drivers/crypto/qat/qat_common/qat_crypto.c | 8 +++++---
> drivers/crypto/qat/qat_dh895xcc/adf_admin.c | 2 +-
> drivers/crypto/qat/qat_dh895xcc/adf_drv.c | 9 ++++++++-
> drivers/crypto/qat/qat_dh895xcc/adf_isr.c | 2 +-
> 7 files changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/crypto/qat/qat_common/adf_accel_devices.h
> b/drivers/crypto/qat/qat_common/adf_accel_devices.h
> index 3cfe195..96e0b06 100644
> --- a/drivers/crypto/qat/qat_common/adf_accel_devices.h
> +++ b/drivers/crypto/qat/qat_common/adf_accel_devices.h
> @@ -203,8 +203,7 @@ struct adf_accel_dev {
> struct dentry *debugfs_dir;
> struct list_head list;
> struct module *owner;
> - uint8_t accel_id;
> - uint8_t numa_node;
> struct adf_accel_pci accel_pci_dev;
> + uint8_t accel_id;
> } __packed;
> #endif
> diff --git a/drivers/crypto/qat/qat_common/adf_transport.c
> b/drivers/crypto/qat/qat_common/adf_transport.c
> index 5f3fa45..9dd2cb7 100644
> --- a/drivers/crypto/qat/qat_common/adf_transport.c
> +++ b/drivers/crypto/qat/qat_common/adf_transport.c
> @@ -419,9 +419,10 @@ static int adf_init_bank(struct adf_accel_dev *accel_dev,
> WRITE_CSR_RING_BASE(csr_addr, bank_num, i, 0);
> ring = &bank->rings[i];
> if (hw_data->tx_rings_mask & (1 << i)) {
> - ring->inflights = kzalloc_node(sizeof(atomic_t),
> - GFP_KERNEL,
> - accel_dev->numa_node);
> + ring->inflights =
> + kzalloc_node(sizeof(atomic_t),
> + GFP_KERNEL,
> + dev_to_node(&GET_DEV(accel_dev)));
> if (!ring->inflights)
> goto err;
> } else {
> @@ -469,13 +470,14 @@ int adf_init_etr_data(struct adf_accel_dev *accel_dev)
> int i, ret;
>
> etr_data = kzalloc_node(sizeof(*etr_data), GFP_KERNEL,
> - accel_dev->numa_node);
> + dev_to_node(&GET_DEV(accel_dev)));
> if (!etr_data)
> return -ENOMEM;
>
> num_banks = GET_MAX_BANKS(accel_dev);
> size = num_banks * sizeof(struct adf_etr_bank_data);
> - etr_data->banks = kzalloc_node(size, GFP_KERNEL, accel_dev->numa_node);
> + etr_data->banks = kzalloc_node(size, GFP_KERNEL,
> + dev_to_node(&GET_DEV(accel_dev)));
> if (!etr_data->banks) {
> ret = -ENOMEM;
> goto err_bank;
> diff --git a/drivers/crypto/qat/qat_common/qat_algs.c
> b/drivers/crypto/qat/qat_common/qat_algs.c
> index bffa8bf..0897a1c 100644
> --- a/drivers/crypto/qat/qat_common/qat_algs.c
> +++ b/drivers/crypto/qat/qat_common/qat_algs.c
> @@ -598,7 +598,8 @@ static int qat_alg_sgl_to_bufl(struct qat_crypto_instance
> *inst,
> if (unlikely(!n))
> return -EINVAL;
>
> - bufl = kmalloc_node(sz, GFP_ATOMIC, inst->accel_dev->numa_node);
> + bufl = kmalloc_node(sz, GFP_ATOMIC,
> + dev_to_node(&GET_DEV(inst->accel_dev)));
> if (unlikely(!bufl))
> return -ENOMEM;
>
> @@ -644,7 +645,7 @@ static int qat_alg_sgl_to_bufl(struct qat_crypto_instance
> *inst,
> struct qat_alg_buf *bufers;
>
> buflout = kmalloc_node(sz, GFP_ATOMIC,
> - inst->accel_dev->numa_node);
> + dev_to_node(&GET_DEV(inst->accel_dev)));
> if (unlikely(!buflout))
> goto err;
> bloutp = dma_map_single(dev, buflout, sz, DMA_TO_DEVICE);
> diff --git a/drivers/crypto/qat/qat_common/qat_crypto.c
> b/drivers/crypto/qat/qat_common/qat_crypto.c
> index 060dc0a..c1eefc4 100644
> --- a/drivers/crypto/qat/qat_common/qat_crypto.c
> +++ b/drivers/crypto/qat/qat_common/qat_crypto.c
> @@ -109,12 +109,14 @@ struct qat_crypto_instance
> *qat_crypto_get_instance_node(int node)
>
> list_for_each(itr, adf_devmgr_get_head()) {
> accel_dev = list_entry(itr, struct adf_accel_dev, list);
> - if (accel_dev->numa_node == node && adf_dev_started(accel_dev))
> + if ((node == dev_to_node(&GET_DEV(accel_dev)) ||
> + dev_to_node(&GET_DEV(accel_dev)) < 0)
> + && adf_dev_started(accel_dev))
> break;
> accel_dev = NULL;
> }
> if (!accel_dev) {
> - pr_err("QAT: Could not find a device on the given node\n");
> + pr_err("QAT: Could not find a device on node %d\n", node);
> accel_dev = adf_devmgr_get_first();
> }
> if (!accel_dev || !adf_dev_started(accel_dev))
> @@ -164,7 +166,7 @@ static int qat_crypto_create_instances(struct
> adf_accel_dev *accel_dev)
>
> for (i = 0; i < num_inst; i++) {
> inst = kzalloc_node(sizeof(*inst), GFP_KERNEL,
> - accel_dev->numa_node);
> + dev_to_node(&GET_DEV(accel_dev)));
> if (!inst)
> goto err;
>
> diff --git a/drivers/crypto/qat/qat_dh895xcc/adf_admin.c
> b/drivers/crypto/qat/qat_dh895xcc/adf_admin.c
> index 978d6c5..53c491b 100644
> --- a/drivers/crypto/qat/qat_dh895xcc/adf_admin.c
> +++ b/drivers/crypto/qat/qat_dh895xcc/adf_admin.c
> @@ -108,7 +108,7 @@ int adf_init_admin_comms(struct adf_accel_dev *accel_dev)
> uint64_t reg_val;
>
> admin = kzalloc_node(sizeof(*accel_dev->admin), GFP_KERNEL,
> - accel_dev->numa_node);
> + dev_to_node(&GET_DEV(accel_dev)));
> if (!admin)
> return -ENOMEM;
> admin->virt_addr = dma_zalloc_coherent(&GET_DEV(accel_dev), PAGE_SIZE,
> diff --git a/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
> b/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
> index 84edf17..40202cc 100644
> --- a/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
> +++ b/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
> @@ -244,11 +244,18 @@ static int adf_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> }
>
> node = adf_get_dev_node_id(pdev);
^^^ I don't think you should ever make this call. IMO it is wrong to do it that
way. Just stick with
node = dev_to_node(&pdev->dev)
as the line below forces a default to that anyway.
> + if (node != dev_to_node(&pdev->dev) && dev_to_node(&pdev->dev) > 0) {
> + /* If the accelerator is connected to a node with no memory
> + * there is no point in using the accelerator since the remote
> + * memory transaction will be very slow. */
> + dev_err(&pdev->dev, "Invalid NUMA configuration.\n");
> + return -EINVAL;
Hmm ... I wonder if it would be safe to do
/* force allocations to node 0 */
node = 0;
dev_err(&pdev->dev, "Invalid NUMA configuration detected, node id = %d .
Defaulting node to 0. \n",
node);
and then continue?
And maybe even a FW_WARN of some sort here might be appropriate to indicate that
something is wrong with the mapping? In any case a better error message is a
always good idea IMO.
P.
> + }
> +
> accel_dev = kzalloc_node(sizeof(*accel_dev), GFP_KERNEL, node);
> if (!accel_dev)
> return -ENOMEM;
>
> - accel_dev->numa_node = node;
> INIT_LIST_HEAD(&accel_dev->crypto_list);
>
> /* Add accel device to accel table.
> diff --git a/drivers/crypto/qat/qat_dh895xcc/adf_isr.c
> b/drivers/crypto/qat/qat_dh895xcc/adf_isr.c
> index c9212b9..fe8f896 100644
> --- a/drivers/crypto/qat/qat_dh895xcc/adf_isr.c
> +++ b/drivers/crypto/qat/qat_dh895xcc/adf_isr.c
> @@ -168,7 +168,7 @@ static int adf_isr_alloc_msix_entry_table(struct
> adf_accel_dev *accel_dev)
> uint32_t msix_num_entries = hw_data->num_banks + 1;
>
> entries = kzalloc_node(msix_num_entries * sizeof(*entries),
> - GFP_KERNEL, accel_dev->numa_node);
> + GFP_KERNEL, dev_to_node(&GET_DEV(accel_dev)));
> if (!entries)
> return -ENOMEM;
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
