As reported in the thread [xen] double fault: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC on Linux kernel the drm has a crappy interaction model with the sysfs objects.
This is my preferred method of fixing it as I don't think the lifetimes need to be tied so closely, though this requires review my someone to make sure my unregistering etc is correct and in the right place.
Russell helped my understanding of the device model and supplied a patch doing this another way, but I think I like splitting the lifetimes more.
Patch 1 is just prepartory for i915, then patch 2 is the guts.
Dave.
From: Dave Airlie airlied@redhat.com
This will make the next patch to change how this works a lot cleaner.
Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/i915/i915_sysfs.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c index 44f4c1a..5caffb7 100644 --- a/drivers/gpu/drm/i915/i915_sysfs.c +++ b/drivers/gpu/drm/i915/i915_sysfs.c @@ -32,6 +32,8 @@ #include "intel_drv.h" #include "i915_drv.h"
+#define dev_to_drm_minor(d) container_of((d), struct drm_minor, kdev) + #ifdef CONFIG_PM static u32 calc_residency(struct drm_device *dev, const u32 reg) { @@ -48,14 +50,14 @@ static u32 calc_residency(struct drm_device *dev, const u32 reg) static ssize_t show_rc6_mask(struct device *kdev, struct device_attribute *attr, char *buf) { - struct drm_minor *dminor = container_of(kdev, struct drm_minor, kdev); + struct drm_minor *dminor = dev_to_drm_minor(kdev); return snprintf(buf, PAGE_SIZE, "%x\n", intel_enable_rc6(dminor->dev)); }
static ssize_t show_rc6_ms(struct device *kdev, struct device_attribute *attr, char *buf) { - struct drm_minor *dminor = container_of(kdev, struct drm_minor, kdev); + struct drm_minor *dminor = dev_to_drm_minor(kdev); u32 rc6_residency = calc_residency(dminor->dev, GEN6_GT_GFX_RC6); return snprintf(buf, PAGE_SIZE, "%u\n", rc6_residency); } @@ -63,7 +65,7 @@ show_rc6_ms(struct device *kdev, struct device_attribute *attr, char *buf) static ssize_t show_rc6p_ms(struct device *kdev, struct device_attribute *attr, char *buf) { - struct drm_minor *dminor = container_of(kdev, struct drm_minor, kdev); + struct drm_minor *dminor = dev_to_drm_minor(kdev); u32 rc6p_residency = calc_residency(dminor->dev, GEN6_GT_GFX_RC6p); if (IS_VALLEYVIEW(dminor->dev)) rc6p_residency = 0; @@ -73,7 +75,7 @@ show_rc6p_ms(struct device *kdev, struct device_attribute *attr, char *buf) static ssize_t show_rc6pp_ms(struct device *kdev, struct device_attribute *attr, char *buf) { - struct drm_minor *dminor = container_of(kdev, struct drm_minor, kdev); + struct drm_minor *dminor = dev_to_drm_minor(kdev); u32 rc6pp_residency = calc_residency(dminor->dev, GEN6_GT_GFX_RC6pp); if (IS_VALLEYVIEW(dminor->dev)) rc6pp_residency = 0; @@ -119,7 +121,7 @@ i915_l3_read(struct file *filp, struct kobject *kobj, loff_t offset, size_t count) { struct device *dev = container_of(kobj, struct device, kobj); - struct drm_minor *dminor = container_of(dev, struct drm_minor, kdev); + struct drm_minor *dminor = dev_to_drm_minor(dev); struct drm_device *drm_dev = dminor->dev; struct drm_i915_private *dev_priv = drm_dev->dev_private; int slice = (int)(uintptr_t)attr->private; @@ -155,7 +157,7 @@ i915_l3_write(struct file *filp, struct kobject *kobj, loff_t offset, size_t count) { struct device *dev = container_of(kobj, struct device, kobj); - struct drm_minor *dminor = container_of(dev, struct drm_minor, kdev); + struct drm_minor *dminor = dev_to_drm_minor(dev); struct drm_device *drm_dev = dminor->dev; struct drm_i915_private *dev_priv = drm_dev->dev_private; struct i915_hw_context *ctx; @@ -228,7 +230,7 @@ static struct bin_attribute dpf_attrs_1 = { static ssize_t gt_cur_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf) { - struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev); + struct drm_minor *minor = dev_to_drm_minor(kdev); struct drm_device *dev = minor->dev; struct drm_i915_private *dev_priv = dev->dev_private; int ret; @@ -249,7 +251,7 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev, static ssize_t vlv_rpe_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf) { - struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev); + struct drm_minor *minor = dev_to_drm_minor(kdev); struct drm_device *dev = minor->dev; struct drm_i915_private *dev_priv = dev->dev_private;
@@ -260,7 +262,7 @@ static ssize_t vlv_rpe_freq_mhz_show(struct device *kdev,
static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf) { - struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev); + struct drm_minor *minor = dev_to_drm_minor(kdev); struct drm_device *dev = minor->dev; struct drm_i915_private *dev_priv = dev->dev_private; int ret; @@ -279,7 +281,7 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev, struct device_attribute *attr, const char *buf, size_t count) { - struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev); + struct drm_minor *minor = dev_to_drm_minor(kdev); struct drm_device *dev = minor->dev; struct drm_i915_private *dev_priv = dev->dev_private; u32 val, rp_state_cap, hw_max, hw_min, non_oc_max; @@ -332,7 +334,7 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf) { - struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev); + struct drm_minor *minor = dev_to_drm_minor(kdev); struct drm_device *dev = minor->dev; struct drm_i915_private *dev_priv = dev->dev_private; int ret; @@ -351,7 +353,7 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev, struct device_attribute *attr, const char *buf, size_t count) { - struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev); + struct drm_minor *minor = dev_to_drm_minor(kdev); struct drm_device *dev = minor->dev; struct drm_i915_private *dev_priv = dev->dev_private; u32 val, rp_state_cap, hw_max, hw_min; @@ -410,7 +412,7 @@ static DEVICE_ATTR(gt_RPn_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL); /* For now we have a static number of RP states */ static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf) { - struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev); + struct drm_minor *minor = dev_to_drm_minor(kdev); struct drm_device *dev = minor->dev; struct drm_i915_private *dev_priv = dev->dev_private; u32 val, rp_state_cap; @@ -458,7 +460,7 @@ static ssize_t error_state_read(struct file *filp, struct kobject *kobj, {
struct device *kdev = container_of(kobj, struct device, kobj); - struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev); + struct drm_minor *minor = dev_to_drm_minor(kdev); struct drm_device *dev = minor->dev; struct i915_error_state_file_priv error_priv; struct drm_i915_error_state_buf error_str; @@ -493,7 +495,7 @@ static ssize_t error_state_write(struct file *file, struct kobject *kobj, loff_t off, size_t count) { struct device *kdev = container_of(kobj, struct device, kobj); - struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev); + struct drm_minor *minor = dev_to_drm_minor(kdev); struct drm_device *dev = minor->dev; int ret;
On Fri, Oct 11, 2013 at 03:05:56PM +1000, Dave Airlie wrote:
From: Dave Airlie airlied@redhat.com
This will make the next patch to change how this works a lot cleaner.
Signed-off-by: Dave Airlie airlied@redhat.com
drivers/gpu/drm/i915/i915_sysfs.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c index 44f4c1a..5caffb7 100644 --- a/drivers/gpu/drm/i915/i915_sysfs.c +++ b/drivers/gpu/drm/i915/i915_sysfs.c @@ -32,6 +32,8 @@ #include "intel_drv.h" #include "i915_drv.h"
+#define dev_to_drm_minor(d) container_of((d), struct drm_minor, kdev)
Maybe this should be in include/drm somewhere?
From: Dave Airlie airlied@redhat.com
So drm was abusing device lifetimes, by having embedded device structures in the minor and connector it meant that the lifetime of the internal drm objects (drm_minor and drm_connector) were tied to the lifetime of the device files in sysfs, so if something kept those files opened the current code would kfree the objects and things would go downhill from there.
Now in reality there is no need for these lifetimes to be so intertwined, especailly with hotplugging of devices where we wish to remove the sysfs and userspace facing pieces before we can unwind the internal objects due to open userspace files or mmaps, so split the objects out so the struct device is no longer embedded and do what fbdev does and just allocate and remove the sysfs inodes separately.
Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/drm_sysfs.c | 94 +++++++++---------------- drivers/gpu/drm/gma500/cdv_intel_dp.c | 2 +- drivers/gpu/drm/i915/i915_irq.c | 8 +-- drivers/gpu/drm/i915/i915_sysfs.c | 28 ++++---- drivers/gpu/drm/i915/intel_dp.c | 2 +- drivers/gpu/drm/nouveau/nouveau_backlight.c | 4 +- drivers/gpu/drm/radeon/atombios_encoders.c | 2 +- drivers/gpu/drm/radeon/radeon_legacy_encoders.c | 2 +- include/drm/drmP.h | 2 +- include/drm/drm_crtc.h | 2 +- 10 files changed, 60 insertions(+), 86 deletions(-)
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c index 2290b3b..dae42c7 100644 --- a/drivers/gpu/drm/drm_sysfs.c +++ b/drivers/gpu/drm/drm_sysfs.c @@ -22,8 +22,8 @@ #include <drm/drm_core.h> #include <drm/drmP.h>
-#define to_drm_minor(d) container_of(d, struct drm_minor, kdev) -#define to_drm_connector(d) container_of(d, struct drm_connector, kdev) +#define to_drm_minor(d) dev_get_drvdata(d) +#define to_drm_connector(d) dev_get_drvdata(d)
static struct device_type drm_sysfs_device_minor = { .name = "drm_minor" @@ -162,20 +162,6 @@ void drm_sysfs_destroy(void) drm_class = NULL; }
-/** - * drm_sysfs_device_release - do nothing - * @dev: Linux device - * - * Normally, this would free the DRM device associated with @dev, along - * with cleaning up any other stuff. But we do that in the DRM core, so - * this function can just return and hope that the core does its job. - */ -static void drm_sysfs_device_release(struct device *dev) -{ - memset(dev, 0, sizeof(struct device)); - return; -} - /* * Connector properties */ @@ -394,29 +380,26 @@ int drm_sysfs_connector_add(struct drm_connector *connector) int i; int ret;
- /* We shouldn't get called more than once for the same connector */ - BUG_ON(device_is_registered(&connector->kdev)); - - connector->kdev.parent = &dev->primary->kdev; - connector->kdev.class = drm_class; - connector->kdev.release = drm_sysfs_device_release; + if (connector->kdev) + return 0;
+ /* We shouldn't get called more than once for the same connector */ + connector->kdev = device_create(drm_class, dev->primary->kdev, + 0, connector, "card%d-%s", + dev->primary->index, drm_get_connector_name(connector)); DRM_DEBUG("adding "%s" to sysfs\n", drm_get_connector_name(connector));
- dev_set_name(&connector->kdev, "card%d-%s", - dev->primary->index, drm_get_connector_name(connector)); - ret = device_register(&connector->kdev); - - if (ret) { - DRM_ERROR("failed to register connector device: %d\n", ret); + if (IS_ERR(connector->kdev)) { + DRM_ERROR("failed to register connector device: %ld\n", PTR_ERR(connector->kdev)); + ret = PTR_ERR(connector->kdev); goto out; }
/* Standard attributes */
for (attr_cnt = 0; attr_cnt < ARRAY_SIZE(connector_attrs); attr_cnt++) { - ret = device_create_file(&connector->kdev, &connector_attrs[attr_cnt]); + ret = device_create_file(connector->kdev, &connector_attrs[attr_cnt]); if (ret) goto err_out_files; } @@ -433,7 +416,7 @@ int drm_sysfs_connector_add(struct drm_connector *connector) case DRM_MODE_CONNECTOR_Component: case DRM_MODE_CONNECTOR_TV: for (opt_cnt = 0; opt_cnt < ARRAY_SIZE(connector_attrs_opt1); opt_cnt++) { - ret = device_create_file(&connector->kdev, &connector_attrs_opt1[opt_cnt]); + ret = device_create_file(connector->kdev, &connector_attrs_opt1[opt_cnt]); if (ret) goto err_out_files; } @@ -442,7 +425,7 @@ int drm_sysfs_connector_add(struct drm_connector *connector) break; }
- ret = sysfs_create_bin_file(&connector->kdev.kobj, &edid_attr); + ret = sysfs_create_bin_file(&connector->kdev->kobj, &edid_attr); if (ret) goto err_out_files;
@@ -453,10 +436,11 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
err_out_files: for (i = 0; i < opt_cnt; i++) - device_remove_file(&connector->kdev, &connector_attrs_opt1[i]); + device_remove_file(connector->kdev, &connector_attrs_opt1[i]); for (i = 0; i < attr_cnt; i++) - device_remove_file(&connector->kdev, &connector_attrs[i]); - device_unregister(&connector->kdev); + device_remove_file(connector->kdev, &connector_attrs[i]); + put_device(connector->kdev); + device_unregister(connector->kdev);
out: return ret; @@ -480,16 +464,17 @@ void drm_sysfs_connector_remove(struct drm_connector *connector) { int i;
- if (!connector->kdev.parent) + if (!connector->kdev) return; DRM_DEBUG("removing "%s" from sysfs\n", drm_get_connector_name(connector));
for (i = 0; i < ARRAY_SIZE(connector_attrs); i++) - device_remove_file(&connector->kdev, &connector_attrs[i]); - sysfs_remove_bin_file(&connector->kdev.kobj, &edid_attr); - device_unregister(&connector->kdev); - connector->kdev.parent = NULL; + device_remove_file(connector->kdev, &connector_attrs[i]); + sysfs_remove_bin_file(&connector->kdev->kobj, &edid_attr); + put_device(connector->kdev); + device_unregister(connector->kdev); + connector->kdev = NULL; } EXPORT_SYMBOL(drm_sysfs_connector_remove);
@@ -508,7 +493,7 @@ void drm_sysfs_hotplug_event(struct drm_device *dev)
DRM_DEBUG("generating hotplug event\n");
- kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, envp); + kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp); } EXPORT_SYMBOL(drm_sysfs_hotplug_event);
@@ -523,15 +508,8 @@ EXPORT_SYMBOL(drm_sysfs_hotplug_event); */ int drm_sysfs_device_add(struct drm_minor *minor) { - int err; char *minor_str;
- minor->kdev.parent = minor->dev->dev; - - minor->kdev.class = drm_class; - minor->kdev.release = drm_sysfs_device_release; - minor->kdev.devt = minor->device; - minor->kdev.type = &drm_sysfs_device_minor; if (minor->type == DRM_MINOR_CONTROL) minor_str = "controlD%d"; else if (minor->type == DRM_MINOR_RENDER) @@ -539,18 +517,14 @@ int drm_sysfs_device_add(struct drm_minor *minor) else minor_str = "card%d";
- dev_set_name(&minor->kdev, minor_str, minor->index); - - err = device_register(&minor->kdev); - if (err) { - DRM_ERROR("device add failed: %d\n", err); - goto err_out; + minor->kdev = device_create(drm_class, minor->dev->dev, + MKDEV(DRM_MAJOR, minor->index), + minor, minor_str, minor->index); + if (IS_ERR(minor->kdev)) { + DRM_ERROR("device create failed %ld\n", PTR_ERR(minor->kdev)); + return PTR_ERR(minor->kdev); } - return 0; - -err_out: - return err; }
/** @@ -562,9 +536,9 @@ err_out: */ void drm_sysfs_device_remove(struct drm_minor *minor) { - if (minor->kdev.parent) - device_unregister(&minor->kdev); - minor->kdev.parent = NULL; + if (minor->kdev) + device_destroy(drm_class, MKDEV(DRM_MAJOR, minor->index)); + minor->kdev = NULL; }
diff --git a/drivers/gpu/drm/gma500/cdv_intel_dp.c b/drivers/gpu/drm/gma500/cdv_intel_dp.c index f4eb435..f88a181 100644 --- a/drivers/gpu/drm/gma500/cdv_intel_dp.c +++ b/drivers/gpu/drm/gma500/cdv_intel_dp.c @@ -666,7 +666,7 @@ cdv_intel_dp_i2c_init(struct gma_connector *connector, strncpy (intel_dp->adapter.name, name, sizeof(intel_dp->adapter.name) - 1); intel_dp->adapter.name[sizeof(intel_dp->adapter.name) - 1] = '\0'; intel_dp->adapter.algo_data = &intel_dp->algo; - intel_dp->adapter.dev.parent = &connector->base.kdev; + intel_dp->adapter.dev.parent = connector->base.kdev;
if (is_edp(encoder)) cdv_intel_edp_panel_vdd_on(encoder); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index b356dc1..8d133bf 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -933,7 +933,7 @@ static void ivybridge_parity_work(struct work_struct *work) parity_event[4] = kasprintf(GFP_KERNEL, "SLICE=%d", slice); parity_event[5] = NULL;
- kobject_uevent_env(&dev_priv->dev->primary->kdev.kobj, + kobject_uevent_env(&dev_priv->dev->primary->kdev->kobj, KOBJ_CHANGE, parity_event);
DRM_DEBUG("Parity error: Slice = %d, Row = %d, Bank = %d, Sub bank = %d.\n", @@ -1530,7 +1530,7 @@ static void i915_error_work_func(struct work_struct *work) char *reset_done_event[] = { I915_ERROR_UEVENT "=0", NULL }; int ret;
- kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, error_event); + kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, error_event);
/* * Note that there's only one work item which does gpu resets, so we @@ -1544,7 +1544,7 @@ static void i915_error_work_func(struct work_struct *work) */ if (i915_reset_in_progress(error) && !i915_terminally_wedged(error)) { DRM_DEBUG_DRIVER("resetting chip\n"); - kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, + kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, reset_event);
/* @@ -1571,7 +1571,7 @@ static void i915_error_work_func(struct work_struct *work) smp_mb__before_atomic_inc(); atomic_inc(&dev_priv->gpu_error.reset_counter);
- kobject_uevent_env(&dev->primary->kdev.kobj, + kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, reset_done_event); } else { atomic_set(&error->reset_counter, I915_WEDGED); diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c index 5caffb7..00173c3 100644 --- a/drivers/gpu/drm/i915/i915_sysfs.c +++ b/drivers/gpu/drm/i915/i915_sysfs.c @@ -32,7 +32,7 @@ #include "intel_drv.h" #include "i915_drv.h"
-#define dev_to_drm_minor(d) container_of((d), struct drm_minor, kdev) +#define dev_to_drm_minor(d) dev_get_drvdata((d))
#ifdef CONFIG_PM static u32 calc_residency(struct drm_device *dev, const u32 reg) @@ -57,7 +57,7 @@ show_rc6_mask(struct device *kdev, struct device_attribute *attr, char *buf) static ssize_t show_rc6_ms(struct device *kdev, struct device_attribute *attr, char *buf) { - struct drm_minor *dminor = dev_to_drm_minor(kdev); + struct drm_minor *dminor = dev_get_drvdata(kdev); u32 rc6_residency = calc_residency(dminor->dev, GEN6_GT_GFX_RC6); return snprintf(buf, PAGE_SIZE, "%u\n", rc6_residency); } @@ -525,19 +525,19 @@ void i915_setup_sysfs(struct drm_device *dev)
#ifdef CONFIG_PM if (INTEL_INFO(dev)->gen >= 6) { - ret = sysfs_merge_group(&dev->primary->kdev.kobj, + ret = sysfs_merge_group(&dev->primary->kdev->kobj, &rc6_attr_group); if (ret) DRM_ERROR("RC6 residency sysfs setup failed\n"); } #endif if (HAS_L3_DPF(dev)) { - ret = device_create_bin_file(&dev->primary->kdev, &dpf_attrs); + ret = device_create_bin_file(dev->primary->kdev, &dpf_attrs); if (ret) DRM_ERROR("l3 parity sysfs setup failed\n");
if (NUM_L3_SLICES(dev) > 1) { - ret = device_create_bin_file(&dev->primary->kdev, + ret = device_create_bin_file(dev->primary->kdev, &dpf_attrs_1); if (ret) DRM_ERROR("l3 parity slice 1 setup failed\n"); @@ -546,13 +546,13 @@ void i915_setup_sysfs(struct drm_device *dev)
ret = 0; if (IS_VALLEYVIEW(dev)) - ret = sysfs_create_files(&dev->primary->kdev.kobj, vlv_attrs); + ret = sysfs_create_files(&dev->primary->kdev->kobj, vlv_attrs); else if (INTEL_INFO(dev)->gen >= 6) - ret = sysfs_create_files(&dev->primary->kdev.kobj, gen6_attrs); + ret = sysfs_create_files(&dev->primary->kdev->kobj, gen6_attrs); if (ret) DRM_ERROR("RPS sysfs setup failed\n");
- ret = sysfs_create_bin_file(&dev->primary->kdev.kobj, + ret = sysfs_create_bin_file(&dev->primary->kdev->kobj, &error_state_attr); if (ret) DRM_ERROR("error_state sysfs setup failed\n"); @@ -560,14 +560,14 @@ void i915_setup_sysfs(struct drm_device *dev)
void i915_teardown_sysfs(struct drm_device *dev) { - sysfs_remove_bin_file(&dev->primary->kdev.kobj, &error_state_attr); + sysfs_remove_bin_file(&dev->primary->kdev->kobj, &error_state_attr); if (IS_VALLEYVIEW(dev)) - sysfs_remove_files(&dev->primary->kdev.kobj, vlv_attrs); + sysfs_remove_files(&dev->primary->kdev->kobj, vlv_attrs); else - sysfs_remove_files(&dev->primary->kdev.kobj, gen6_attrs); - device_remove_bin_file(&dev->primary->kdev, &dpf_attrs_1); - device_remove_bin_file(&dev->primary->kdev, &dpf_attrs); + sysfs_remove_files(&dev->primary->kdev->kobj, gen6_attrs); + device_remove_bin_file(dev->primary->kdev, &dpf_attrs_1); + device_remove_bin_file(dev->primary->kdev, &dpf_attrs); #ifdef CONFIG_PM - sysfs_unmerge_group(&dev->primary->kdev.kobj, &rc6_attr_group); + sysfs_unmerge_group(&dev->primary->kdev->kobj, &rc6_attr_group); #endif } diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 5f0c783..a19f0bd 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -732,7 +732,7 @@ intel_dp_i2c_init(struct intel_dp *intel_dp, strncpy(intel_dp->adapter.name, name, sizeof(intel_dp->adapter.name) - 1); intel_dp->adapter.name[sizeof(intel_dp->adapter.name) - 1] = '\0'; intel_dp->adapter.algo_data = &intel_dp->algo; - intel_dp->adapter.dev.parent = &intel_connector->base.kdev; + intel_dp->adapter.dev.parent = intel_connector->base.kdev;
ironlake_edp_panel_vdd_on(intel_dp); ret = i2c_dp_aux_add_bus(&intel_dp->adapter); diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c b/drivers/gpu/drm/nouveau/nouveau_backlight.c index 2ffad21..630f6e8 100644 --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c @@ -82,7 +82,7 @@ nv40_backlight_init(struct drm_connector *connector) memset(&props, 0, sizeof(struct backlight_properties)); props.type = BACKLIGHT_RAW; props.max_brightness = 31; - bd = backlight_device_register("nv_backlight", &connector->kdev, drm, + bd = backlight_device_register("nv_backlight", connector->kdev, drm, &nv40_bl_ops, &props); if (IS_ERR(bd)) return PTR_ERR(bd); @@ -204,7 +204,7 @@ nv50_backlight_init(struct drm_connector *connector) memset(&props, 0, sizeof(struct backlight_properties)); props.type = BACKLIGHT_RAW; props.max_brightness = 100; - bd = backlight_device_register("nv_backlight", &connector->kdev, + bd = backlight_device_register("nv_backlight", connector->kdev, nv_encoder, ops, &props); if (IS_ERR(bd)) return PTR_ERR(bd); diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c index 32923d2..28e2dc4 100644 --- a/drivers/gpu/drm/radeon/atombios_encoders.c +++ b/drivers/gpu/drm/radeon/atombios_encoders.c @@ -213,7 +213,7 @@ void radeon_atom_backlight_init(struct radeon_encoder *radeon_encoder, props.type = BACKLIGHT_RAW; snprintf(bl_name, sizeof(bl_name), "radeon_bl%d", dev->primary->index); - bd = backlight_device_register(bl_name, &drm_connector->kdev, + bd = backlight_device_register(bl_name, drm_connector->kdev, pdata, &radeon_atom_backlight_ops, &props); if (IS_ERR(bd)) { DRM_ERROR("Backlight registration failed\n"); diff --git a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c index 62cd512..c89971d 100644 --- a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c +++ b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c @@ -392,7 +392,7 @@ void radeon_legacy_backlight_init(struct radeon_encoder *radeon_encoder, props.type = BACKLIGHT_RAW; snprintf(bl_name, sizeof(bl_name), "radeon_bl%d", dev->primary->index); - bd = backlight_device_register(bl_name, &drm_connector->kdev, + bd = backlight_device_register(bl_name, drm_connector->kdev, pdata, &radeon_backlight_ops, &props); if (IS_ERR(bd)) { DRM_ERROR("Backlight registration failed\n"); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 9ba6a38..33eaba6 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1043,7 +1043,7 @@ struct drm_minor { int index; /**< Minor device number */ int type; /**< Control or render */ dev_t device; /**< Device number for mknod */ - struct device kdev; /**< Linux device */ + struct device *kdev; /**< Linux device */ struct drm_device *dev;
struct dentry *debugfs_root; diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 92e7820..c7ce4e2 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -587,7 +587,7 @@ enum drm_connector_force { */ struct drm_connector { struct drm_device *dev; - struct device kdev; + struct device *kdev; struct device_attribute *attr; struct list_head head;
On Thu, Oct 10, 2013 at 10:05 PM, Dave Airlie airlied@gmail.com wrote:
This is my preferred method of fixing it as I don't think the lifetimes need to be tied so closely, though this requires review my someone to make sure my unregistering etc is correct and in the right place.
Apparently this fixes the problem for Fengguang, and the code looks cleaner too. Thanks,
Linus
This is my preferred method of fixing it as I don't think the lifetimes need to be tied so closely, though this requires review my someone to make sure my unregistering etc is correct and in the right place.
Apparently this fixes the problem for Fengguang, and the code looks cleaner too. Thanks,
Leaves the fixes or next question, since I suppose its not technically a regression, -next is probably fine, let me know if you'd l like them earlier.
Dave.
On Sat, Oct 12, 2013 at 07:47:05AM +1000, Dave Airlie wrote:
This is my preferred method of fixing it as I don't think the lifetimes need to be tied so closely, though this requires review my someone to make sure my unregistering etc is correct and in the right place.
Apparently this fixes the problem for Fengguang, and the code looks cleaner too. Thanks,
Leaves the fixes or next question, since I suppose its not technically a regression, -next is probably fine, let me know if you'd l like them earlier.
Dave, I tested the two patches on top of drm-next and find that it does help eliminate the lots of oops messages in the below kernels.
However in the 2nd config, the patched kernel has one new oops type than its base kernels (6aba5b6 and v3.12-rc3). v3.12-rc4 is also tested for your reference. In the 3nd config, both patched and base kernels have that oops:
[ 96.969429] init: plymouth-upstart-bridge main process (309) terminated with status 1 * Asking all remaining processes to terminate... [ 97.260371] BUG: Bad page map in process killall5 pte:4f426de0 pmd:0f4f4067 [ 97.261114] addr:3fc00000 vm_flags:00100173 anon_vma:4f4066c0 mapping: (null) index:3ffe6 [ 97.261912] CPU: 0 PID: 334 Comm: killall5 Not tainted 3.12.0-rc3-00156-gdaeb5e3 #1 [ 97.262633] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 [ 97.263192] 3fc00000 4f4c1e14 4212e45c 4fbff9a0 4f4c1e4c 411a9c4b 4262ade0 3fc00000 [ 97.264051] 00100173 4f4066c0 00000000 0003ffe6 4f426de0 0003ffe6 00000000 4fbff9a0 [ 97.264906] 3fc00000 3fc00000 4f4c1e60 411ab50e 00000000 4f464000 00000000 4f4c1ed0 [ 97.265751] Call Trace: [ 97.266022] [<4212e45c>] dump_stack+0xbb/0x14b [ 97.266456] [<411a9c4b>] print_bad_pte+0x28b/0x2c0 [ 97.266931] [<411ab50e>] vm_normal_page+0xae/0xe0 [ 97.267388] [<411b37f3>] munlock_vma_pages_range+0x143/0x320 [ 97.267950] [<410d30fd>] ? sched_clock_cpu+0x20d/0x250 [ 97.268451] [<411bacee>] exit_mmap+0x7e/0x200 [ 97.268893] [<4131de9c>] ? __const_udelay+0x2c/0x40 [ 97.269369] [<410adf28>] ? __rcu_read_unlock+0x68/0x150 [ 97.269888] [<4123227b>] ? exit_aio+0x11b/0x280 [ 97.270412] [<4123217c>] ? exit_aio+0x1c/0x280 [ 97.270892] [<410829c7>] ? do_exit+0x697/0x1280 [ 97.271332] [<4107a1a1>] mmput+0x81/0x170 [ 97.271726] [<410829dc>] do_exit+0x6ac/0x1280 [ 97.272166] [<410bad75>] ? hrtimer_nanosleep+0x1f5/0x210 [ 97.272679] [<4215328a>] ? sysenter_exit+0xf/0x45 [ 97.273151] [<41083751>] do_group_exit+0x131/0x160 [ 97.273617] [<410837ad>] SyS_exit_group+0x2d/0x30 [ 97.274088] [<42153251>] sysenter_do_call+0x12/0x3c [ 97.274560] Disabling lock debugging due to kernel taint * All processes ended within 1 seconds....
That oops message looks very like the one I reported for this commit.
commit 7a8010cd36273ff5f6fea5201ef9232f30cebbd9 Author: Vlastimil Babka vbabka@suse.cz Date: Wed Sep 11 14:22:35 2013 -0700
mm: munlock: manual pte walk in fast path instead of follow_page_mask()
Vlastimil Babka, has this bug been fixed in -rc4?
Thanks, Fengguang ---
patch 2 daeb5e3 drm/sysfs: sort out minor and connector device object lifetimes. patch 1 b4dcbb3 drm/i915: abstract the conversion of device->minor out to a macro drm-next 6aba5b6 drm/i915/dp: get rid of intel_dp->link_configuration
/kernel/x86_64-randconfig-j3-1009/daeb5e37165e2ad745d1d2d5d7ab32ad9cb08f1a
+-----------------------------------------------------------------------+-----------+-----------+--------------+--------------+ | | v3.12-rc3 | v3.12-rc4 | 6aba5b6cf098 | daeb5e37165e | +-----------------------------------------------------------------------+-----------+-----------+--------------+--------------+ | has_kernel_error_warning | 161 | 60 | 109 | 3 | | WARNING:CPU:PID:at_fs/sysfs/dir.c:sysfs_add_one() | 161 | 60 | 109 | | | WARNING:CPU:PID:at_lib/kobject.c:kobject_add_internal() | 161 | 60 | 109 | | | WARNING:CPU:PID:at_drivers/base/driver.c:driver_unregister() | 161 | 60 | 109 | | | WARNING:CPU:PID:at_lib/debugobjects.c:debug_print_object() | 161 | 60 | 109 | | | WARNING:CPU:PID:at_arch/x86/kernel/irq_64.c:handle_irq() | 10 | 3 | 6 | | | BUG:unable_to_handle_kernel_NULL_pointer_dereference_at | 158 | 59 | 109 | | | Oops:DEBUG_PAGEALLOC | 92 | 28 | 64 | | | WARNING:CPU:PID:at_lib/list_debug.c:__list_del_entry() | 92 | 28 | 64 | | | BUG:scheduling_while_atomic | 99 | 30 | 64 | | | INFO:lockdep_is_turned_off | 99 | 30 | 64 | | | BUG:unable_to_handle_kernel_paging_request_at | 145 | 55 | 100 | | | kernel_BUG_at_kernel/smpboot.c | 45 | 13 | 23 | | | invalid_opcode:DEBUG_PAGEALLOC | 45 | 13 | 23 | | | general_protection_fault:DEBUG_PAGEALLOC | 10 | 2 | 3 | | | Kernel_panic-not_syncing:Fatal_exception_in_interrupt | 19 | 4 | 7 | | | BUG:kernel_boot_oops | 57 | 60 | 9 | | | WARNING:CPU:PID:at_kernel/workqueue.c:work_fixup_activate() | 75 | 24 | 57 | | | WARNING:CPU:PID:at_kernel/workqueue.c:__queue_work() | 29 | 10 | 16 | | | BUG:unable_to_handle_kernel_NULL_pointer_dereference_at(null) | 9 | 2 | 4 | | | BUG:unable_to_h | 1 | | | | | BUG:unable_to_handle_kernel_NULL_pointer_dereference | 5 | 4 | 2 | | | BUG:unable_to | 1 | | | | | BUG:unable_to_handle | 2 | | | | | BUG:unable_to_handle_kernel_NULL | 1 | | | | | Kernel_panic-not_syncing:stack-protector:Kernel_stack_is_corrupted_in | 1 | | | | | BUG:unable_to_handle_kernel_NULL_pointer_de | 1 | 1 | | | | BUG:unable_to_handle_kernel_NULL_po | 2 | | | | | BUG:unable_to_ha | 1 | | | | | BUG:u | 1 | | | | | BUG:unable_to_handle_kernel | 0 | 1 | 3 | | | BUG:unable_to_handle_kernel_NULL_point | 0 | 1 | | | | BUG:unable_to_handle_kernel_NULL_pointer_derefe | 0 | 1 | | | | BUG:un | 0 | 1 | 1 | | | BUG:unable_to_handle_kernel_NULL_pointer_dere | 0 | 0 | 1 | | | BUG | 0 | 0 | 1 | | | BUG:unable_to_handle_kernel_NULL_pointer | 0 | 0 | 1 | | | BUG:unable_to_hand | 0 | 0 | 1 | | | BUG:unable_to_handl | 0 | 0 | 1 | | | good_boots | 0 | 0 | 0 | 98 | | Out_of_memory:Kill_process | 0 | 0 | 0 | 3 | +-----------------------------------------------------------------------+-----------+-----------+--------------+--------------+
/kernel/i386-randconfig-c3-1009/daeb5e37165e2ad745d1d2d5d7ab32ad9cb08f1a
+---------------------------------------------------------------+-----------+-----------+--------------+--------------+--------------+ | | v3.12-rc3 | v3.12-rc4 | 6aba5b6cf098 | b4dcbb3b9284 | daeb5e37165e | +---------------------------------------------------------------+-----------+-----------+--------------+--------------+--------------+ | has_kernel_error_warning | 364 | 63 | 1109 | 78 | 71 | | BUG:kernel_early_hang_without_any_printk_output | 13 | 7 | 44 | 62 | 58 | | WARNING:CPU:PID:at_fs/sysfs/dir.c:sysfs_add_one() | 351 | 56 | 1065 | | | | WARNING:CPU:PID:at_lib/kobject.c:kobject_add_internal() | 351 | 56 | 1065 | | | | WARNING:CPU:PID:at_drivers/base/driver.c:driver_unregister() | 351 | 56 | 1065 | | | | WARNING:CPU:PID:at_lib/debugobjects.c:debug_print_object() | 351 | 56 | 1065 | | | | INFO:trying_to_register_non-static_key | 351 | 56 | 1065 | | | | BUG:unable_to_handle_kernel_NULL_pointer_dereference_at(null) | 351 | 56 | 1065 | | | | Oops:PREEMPT_SMP | 351 | 56 | 1065 | | | | Kernel_panic-not_syncing:Fatal_exception_in_interrupt | 351 | 56 | 1065 | | | | BUG:kernel_boot_oops | 58 | 56 | 9 | | | | good_boots | 0 | 0 | 0 | 922 | 1030 | | BUG:Bad_page_map_in_process_killall5_pte:pmd | 0 | 0 | 0 | 15 | 13 | | kernel_BUG_at_include/linux/mm.h | 0 | 0 | 0 | 1 | | | invalid_opcode:PREEMPT_SMP | 0 | 0 | 0 | 1 | | | Kernel_panic-not_syncing:Fatal_exception | 0 | 0 | 0 | 1 | | +---------------------------------------------------------------+-----------+-----------+--------------+--------------+--------------+
/kernel/i386-randconfig-i003-1011/daeb5e37165e2ad745d1d2d5d7ab32ad9cb08f1a
+-------------------------------------------------+-----------+-----------+--------------+--------------+ | | v3.12-rc3 | v3.12-rc4 | 6aba5b6cf098 | daeb5e37165e | +-------------------------------------------------+-----------+-----------+--------------+--------------+ | good_boots | 1088 | 1092 | 1070 | 1043 | | has_kernel_error_warning | 42 | 71 | 40 | 67 | | BUG:kernel_early_hang_without_any_printk_output | 37 | 71 | 36 | 59 | | BUG:Bad_page_map_in_process_killall5_pte:pmd | 5 | 0 | 4 | 8 | +-------------------------------------------------+-----------+-----------+--------------+--------------+ wfg@bee /kernel/i386-randconfig-r8-1011/daeb5e37165e2ad745d1d2d5d7ab32ad9cb08f1a%
On 12.10.2013 14:54, Fengguang Wu wrote:
On Sat, Oct 12, 2013 at 07:47:05AM +1000, Dave Airlie wrote:
This is my preferred method of fixing it as I don't think the lifetimes need to be tied so closely, though this requires review my someone to make sure my unregistering etc is correct and in the right place.
Apparently this fixes the problem for Fengguang, and the code looks cleaner too. Thanks,
Leaves the fixes or next question, since I suppose its not technically a regression, -next is probably fine, let me know if you'd l like them earlier.
Dave, I tested the two patches on top of drm-next and find that it does help eliminate the lots of oops messages in the below kernels.
However in the 2nd config, the patched kernel has one new oops type than its base kernels (6aba5b6 and v3.12-rc3). v3.12-rc4 is also tested for your reference. In the 3nd config, both patched and base kernels have that oops:
[ 96.969429] init: plymouth-upstart-bridge main process (309) terminated with status 1
- Asking all remaining processes to terminate...
[ 97.260371] BUG: Bad page map in process killall5 pte:4f426de0 pmd:0f4f4067 [ 97.261114] addr:3fc00000 vm_flags:00100173 anon_vma:4f4066c0 mapping: (null) index:3ffe6 [ 97.261912] CPU: 0 PID: 334 Comm: killall5 Not tainted 3.12.0-rc3-00156-gdaeb5e3 #1 [ 97.262633] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 [ 97.263192] 3fc00000 4f4c1e14 4212e45c 4fbff9a0 4f4c1e4c 411a9c4b 4262ade0 3fc00000 [ 97.264051] 00100173 4f4066c0 00000000 0003ffe6 4f426de0 0003ffe6 00000000 4fbff9a0 [ 97.264906] 3fc00000 3fc00000 4f4c1e60 411ab50e 00000000 4f464000 00000000 4f4c1ed0 [ 97.265751] Call Trace: [ 97.266022] [<4212e45c>] dump_stack+0xbb/0x14b [ 97.266456] [<411a9c4b>] print_bad_pte+0x28b/0x2c0 [ 97.266931] [<411ab50e>] vm_normal_page+0xae/0xe0 [ 97.267388] [<411b37f3>] munlock_vma_pages_range+0x143/0x320 [ 97.267950] [<410d30fd>] ? sched_clock_cpu+0x20d/0x250 [ 97.268451] [<411bacee>] exit_mmap+0x7e/0x200 [ 97.268893] [<4131de9c>] ? __const_udelay+0x2c/0x40 [ 97.269369] [<410adf28>] ? __rcu_read_unlock+0x68/0x150 [ 97.269888] [<4123227b>] ? exit_aio+0x11b/0x280 [ 97.270412] [<4123217c>] ? exit_aio+0x1c/0x280 [ 97.270892] [<410829c7>] ? do_exit+0x697/0x1280 [ 97.271332] [<4107a1a1>] mmput+0x81/0x170 [ 97.271726] [<410829dc>] do_exit+0x6ac/0x1280 [ 97.272166] [<410bad75>] ? hrtimer_nanosleep+0x1f5/0x210 [ 97.272679] [<4215328a>] ? sysenter_exit+0xf/0x45 [ 97.273151] [<41083751>] do_group_exit+0x131/0x160 [ 97.273617] [<410837ad>] SyS_exit_group+0x2d/0x30 [ 97.274088] [<42153251>] sysenter_do_call+0x12/0x3c [ 97.274560] Disabling lock debugging due to kernel taint
- All processes ended within 1 seconds....
That oops message looks very like the one I reported for this commit.
commit 7a8010cd36273ff5f6fea5201ef9232f30cebbd9 Author: Vlastimil Babkavbabka@suse.cz Date: Wed Sep 11 14:22:35 2013 -0700
mm: munlock: manual pte walk in fast path instead of follow_page_mask()Vlastimil Babka, has this bug been fixed in -rc4?
Yes, this should have been fixed by commit eadb41ae82f80210 "mm/mlock.c: prevent walking off the end of a pagetable in no-pmd configuration", merged between rc3 and rc4.
Vlastimil Babka
[ 97.260371] BUG: Bad page map in process killall5 pte:4f426de0 pmd:0f4f4067 [ 97.261114] addr:3fc00000 vm_flags:00100173 anon_vma:4f4066c0 mapping: (null) index:3ffe6 [ 97.261912] CPU: 0 PID: 334 Comm: killall5 Not tainted 3.12.0-rc3-00156-gdaeb5e3 #1 [ 97.262633] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 [ 97.263192] 3fc00000 4f4c1e14 4212e45c 4fbff9a0 4f4c1e4c 411a9c4b 4262ade0 3fc00000 [ 97.264051] 00100173 4f4066c0 00000000 0003ffe6 4f426de0 0003ffe6 00000000 4fbff9a0 [ 97.264906] 3fc00000 3fc00000 4f4c1e60 411ab50e 00000000 4f464000 00000000 4f4c1ed0 [ 97.265751] Call Trace: [ 97.266022] [<4212e45c>] dump_stack+0xbb/0x14b [ 97.266456] [<411a9c4b>] print_bad_pte+0x28b/0x2c0 [ 97.266931] [<411ab50e>] vm_normal_page+0xae/0xe0 [ 97.267388] [<411b37f3>] munlock_vma_pages_range+0x143/0x320 [ 97.267950] [<410d30fd>] ? sched_clock_cpu+0x20d/0x250 [ 97.268451] [<411bacee>] exit_mmap+0x7e/0x200
Yes, this should have been fixed by commit eadb41ae82f80210 "mm/mlock.c: prevent walking off the end of a pagetable in no-pmd configuration", merged between rc3 and rc4.
Great, thanks! This explains why only v3.12-rc4 is free from the bug in the below config.
/kernel/i386-randconfig-i003-1011/daeb5e37165e2ad745d1d2d5d7ab32ad9cb08f1a
+-------------------------------------------------+-----------+-----------+--------------+--------------+ | | v3.12-rc3 | v3.12-rc4 | 6aba5b6cf098 | daeb5e37165e | +-------------------------------------------------+-----------+-----------+--------------+--------------+ | good_boots | 1088 | 1092 | 1070 | 1043 | | has_kernel_error_warning | 42 | 71 | 40 | 67 | | BUG:kernel_early_hang_without_any_printk_output | 37 | 71 | 36 | 59 | | BUG:Bad_page_map_in_process_killall5_pte:pmd | 5 | 0 | 4 | 8 | +-------------------------------------------------+-----------+-----------+--------------+--------------+
Thanks, Fengguang
dri-devel@lists.freedesktop.org
-
Dave Airlie -
Fengguang Wu -
Linus Torvalds -
Russell King - ARM Linux -
Vlastimil Babka