On 10/08/2014 10:57 AM, Prarit Bhargava wrote:
>> 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?
>
>> > + 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
