Re: [PATCH 2/2] crypto: qat - Enforce valid numa configuration.

On 10/08/2014 10:57 AM, Prarit Bhargava wrote:

quoted

 	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.

But then how do I know which node I'm physically connected to?

quoted

quoted

+	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?

As the comment say there is no point continuing if the configuration is
wrong. Defaulting to 0 will cause the same panic you pointed out in your
first patch if node 0 has no memory.

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.