Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.

From: Rafael J. Wysocki
Date: Fri Jul 19 2013 - 10:39:35 EST


On Friday, July 19, 2013 03:03:36 PM Rafael J. Wysocki wrote:
> On Thursday, July 18, 2013 04:08:14 PM Tim Chen wrote:
> > On Fri, 2013-07-19 at 00:17 +0200, Rafael J. Wysocki wrote:
> > > On 7/18/2013 11:00 PM, Tim Chen wrote:
> > > > On Thu, 2013-07-18 at 12:47 +0900, Tetsuo Handa wrote:
> > > >> Tim Chen wrote:
> > > >>>>> Your approach is quite complicated. I think something simpler like the
> > > >>>>> following will work:
> > > >>>> We cannot benefit from PCLMULQDQ. Is it acceptable for you?
> > > >>>
> > > >>> The following code in crct10dif-pclmul_glue.c
> > > >>>
> > > >>> static const struct x86_cpu_id crct10dif_cpu_id[] = {
> > > >>> X86_FEATURE_MATCH(X86_FEATURE_PCLMULQDQ),
> > > >>> {}
> > > >>> };
> > > >>> MODULE_DEVICE_TABLE(x86cpu, crct10dif_cpu_id);
> > > >>>
> > > >>> will put the module in the device table and get the module
> > > >>> loaded, as long as the cpu support PCLMULQDQ. So we should be able
> > > >>> to benefit.
> > > >> Excuse me, how can crct10dif-pclmul.ko get loaded automatically?
> > > >> Did you test CONFIG_CRYPTO_CRCT10DIF_PCLMUL=m with below debug message?
> > > > The code:
> > > >
> > > > static const struct x86_cpu_id crct10dif_cpu_id[] = {
> > > > X86_FEATURE_MATCH(X86_FEATURE_PCLMULQDQ),
> > > > {}
> > > > };
> > > > MODULE_DEVICE_TABLE(x86cpu, crct10dif_cpu_id);
> > > >
> > > > causes the following line to be added to modules.alias file:
> > > >
> > > > alias x86cpu:vendor:*:family:*:model:*:feature:*0081* crct10dif_pclmul
> > > >
> > > > This should cause udev to load the crct10dif_pclml module when cpu
> > > > support the PCLMULQDQ (feature code 0081). I did my testing during
> > > > development on 3.10 and the module was indeed loaded.
> > > >
> > > > However, I found that the following commit under 3.11-rc1 broke
> > > > the mechanism after some bisection.
> > > >
> > > > commit ac212b6980d8d5eda705864fc5a8ecddc6d6eacc
> > > > Author: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > > > Date: Fri May 3 00:26:22 2013 +0200
> > > >
> > > > ACPI / processor: Use common hotplug infrastructure
> > > >
> > > > Split the ACPI processor driver into two parts, one that is
> > > > non-modular, resides in the ACPI core and handles the enumeration
> > > > and hotplug of processors and one that implements the rest of the
> > > > existing processor driver functionality.
> > > >
> > > > Rafael, can you check and see if this can be fixed so those optimized
> > > > crypto modules for Intel cpu that support them can be loaded?
> > >
> > > I think this is an ordering issue between udev startup and the time when
> > > devices are registered.
> >
> > Something that can be fixed?
>
> I'm sure it can be fixes, but I'm not sure whether it's a udev bug or real
> breakage in the kernel yet. I need to figure out that part.

OK

I wonder if the appended patch makes the issue go away for you?

Rafael


---
drivers/acpi/acpi_platform.c | 24 +++++++++++++-----------
drivers/acpi/acpi_processor.c | 14 ++++----------
drivers/acpi/processor_driver.c | 26 ++++++++++++++------------
3 files changed, 31 insertions(+), 33 deletions(-)

Index: linux-pm/drivers/acpi/acpi_processor.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_processor.c
+++ linux-pm/drivers/acpi/acpi_processor.c
@@ -397,20 +397,14 @@ static int __cpuinit acpi_processor_add(
result = -ENODEV;
goto err;
}
-
- result = acpi_bind_one(dev, pr->handle);
- if (result)
- goto err;
-
pr->dev = dev;
dev->offline = pr->flags.need_hotplug_init;

- /* Trigger the processor driver's .probe() if present. */
- if (device_attach(dev) >= 0)
- return 1;
+ result = acpi_create_platform_device(device, id);
+ if (result > 0)
+ return result;

- dev_err(dev, "Processor driver could not be attached\n");
- acpi_unbind_one(dev);
+ dev_err(dev, "Unable to create processor platform device\n");

err:
free_cpumask_var(pr->throttling.shared_cpu_map);
Index: linux-pm/drivers/acpi/acpi_platform.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_platform.c
+++ linux-pm/drivers/acpi/acpi_platform.c
@@ -52,7 +52,7 @@ int acpi_create_platform_device(struct a
struct platform_device_info pdevinfo;
struct resource_list_entry *rentry;
struct list_head resource_list;
- struct resource *resources;
+ struct resource *resources = NULL;
int count;

/* If the ACPI node already has a physical device attached, skip it. */
@@ -61,20 +61,22 @@ int acpi_create_platform_device(struct a

INIT_LIST_HEAD(&resource_list);
count = acpi_dev_get_resources(adev, &resource_list, NULL, NULL);
- if (count <= 0)
+ if (count < 0) {
return 0;
+ } else if (count > 0) {
+ resources = kmalloc(count * sizeof(struct resource),
+ GFP_KERNEL);
+ if (!resources) {
+ dev_err(&adev->dev, "No memory for resources\n");
+ acpi_dev_free_resource_list(&resource_list);
+ return -ENOMEM;
+ }
+ count = 0;
+ list_for_each_entry(rentry, &resource_list, node)
+ resources[count++] = rentry->res;

- resources = kmalloc(count * sizeof(struct resource), GFP_KERNEL);
- if (!resources) {
- dev_err(&adev->dev, "No memory for resources\n");
acpi_dev_free_resource_list(&resource_list);
- return -ENOMEM;
}
- count = 0;
- list_for_each_entry(rentry, &resource_list, node)
- resources[count++] = rentry->res;
-
- acpi_dev_free_resource_list(&resource_list);

memset(&pdevinfo, 0, sizeof(pdevinfo));
/*
Index: linux-pm/drivers/acpi/processor_driver.c
===================================================================
--- linux-pm.orig/drivers/acpi/processor_driver.c
+++ linux-pm/drivers/acpi/processor_driver.c
@@ -36,6 +36,7 @@
#include <linux/cpuidle.h>
#include <linux/slab.h>
#include <linux/acpi.h>
+#include <linux/platform_device.h>

#include <acpi/processor.h>

@@ -54,8 +55,8 @@ MODULE_AUTHOR("Paul Diefenbaugh");
MODULE_DESCRIPTION("ACPI Processor Driver");
MODULE_LICENSE("GPL");

-static int acpi_processor_start(struct device *dev);
-static int acpi_processor_stop(struct device *dev);
+static int acpi_processor_start(struct platform_device *pdev);
+static int acpi_processor_stop(struct platform_device *pdev);

static const struct acpi_device_id processor_device_ids[] = {
{ACPI_PROCESSOR_OBJECT_HID, 0},
@@ -64,10 +65,11 @@ static const struct acpi_device_id proce
};
MODULE_DEVICE_TABLE(acpi, processor_device_ids);

-static struct device_driver acpi_processor_driver = {
- .name = "processor",
- .bus = &cpu_subsys,
- .acpi_match_table = processor_device_ids,
+static struct platform_driver acpi_processor_driver = {
+ .driver = {
+ .name = "processor",
+ .acpi_match_table = processor_device_ids,
+ },
.probe = acpi_processor_start,
.remove = acpi_processor_stop,
};
@@ -222,22 +224,22 @@ static __cpuinit int __acpi_processor_st
return result;
}

-static int __cpuinit acpi_processor_start(struct device *dev)
+static int __cpuinit acpi_processor_start(struct platform_device *pdev)
{
struct acpi_device *device;

- if (acpi_bus_get_device(ACPI_HANDLE(dev), &device))
+ if (acpi_bus_get_device(ACPI_HANDLE(&pdev->dev), &device))
return -ENODEV;

return __acpi_processor_start(device);
}

-static int acpi_processor_stop(struct device *dev)
+static int acpi_processor_stop(struct platform_device *pdev)
{
struct acpi_device *device;
struct acpi_processor *pr;

- if (acpi_bus_get_device(ACPI_HANDLE(dev), &device))
+ if (acpi_bus_get_device(ACPI_HANDLE(&pdev->dev), &device))
return 0;

acpi_remove_notify_handler(device->handle, ACPI_DEVICE_NOTIFY,
@@ -271,7 +273,7 @@ static int __init acpi_processor_driver_
if (acpi_disabled)
return 0;

- result = driver_register(&acpi_processor_driver);
+ result = platform_driver_register(&acpi_processor_driver);
if (result < 0)
return result;

@@ -292,7 +294,7 @@ static void __exit acpi_processor_driver
acpi_thermal_cpufreq_exit();
unregister_hotcpu_notifier(&acpi_cpu_notifier);
acpi_processor_syscore_exit();
- driver_unregister(&acpi_processor_driver);
+ platform_driver_unregister(&acpi_processor_driver);
}

module_init(acpi_processor_driver_init);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/