From ae87221d3ce49d9de1e43756da834fd0bf05a2ad Mon Sep 17 00:00:00 2001 From: Andrew Morton Date: Fri, 24 Aug 2007 16:11:54 -0700 Subject: sysfs: crash debugging Print the name of the last-accessed sysfs file when we oops, to help track down oopses which occur in sysfs store/read handlers. Because these oopses tend to not leave any trace of the offending code in the stack traces. Cc: Kay Sievers Cc: Mathieu Desnoyers Signed-off-by: Andrew Morton Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/file.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'fs') diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index c9e4e5091da1..ce8339c70a4b 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -19,10 +19,18 @@ #include #include #include +#include #include #include "sysfs.h" +/* used in crash dumps to help with debugging */ +static char last_sysfs_file[PATH_MAX]; +void sysfs_printk_last_file(void) +{ + printk(KERN_EMERG "last sysfs file: %s\n", last_sysfs_file); +} + /* * There's one sysfs_buffer for each open file and one * sysfs_open_dirent for each sysfs_dirent with one or more open @@ -328,6 +336,11 @@ static int sysfs_open_file(struct inode *inode, struct file *file) struct sysfs_buffer *buffer; struct sysfs_ops *ops; int error = -EACCES; + char *p; + + p = d_path(&file->f_path, last_sysfs_file, sizeof(last_sysfs_file)); + if (p) + memmove(last_sysfs_file, p, strlen(p) + 1); /* need attr_sd for attr and ops, its parent for kobj */ if (!sysfs_get_active_two(attr_sd)) -- cgit v1.2.3 From a9b12619f7b6f19c871437ec24a088787a04b1de Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Mon, 21 Jul 2008 20:03:34 -0700 Subject: device create: misc: convert device_create_drvdata to device_create Now that device_create() has been audited, rename things back to the original call to be sane. Signed-off-by: Greg Kroah-Hartman --- arch/mips/kernel/rtlx.c | 4 ++-- arch/mips/sibyte/common/sb_tbprof.c | 3 +-- arch/x86/kernel/cpuid.c | 4 ++-- arch/x86/kernel/msr.c | 4 ++-- drivers/dca/dca-sysfs.c | 8 +++----- drivers/hid/hidraw.c | 5 ++--- drivers/hwmon/hwmon.c | 4 ++-- drivers/i2c/i2c-dev.c | 6 +++--- drivers/isdn/capi/capi.c | 3 +-- drivers/leds/led-class.c | 4 ++-- drivers/macintosh/adb.c | 3 +-- drivers/media/dvb/dvb-core/dvbdev.c | 2 +- drivers/misc/phantom.c | 6 +++--- drivers/mtd/mtdchar.c | 10 ++++------ drivers/power/power_supply_core.c | 4 ++-- drivers/spi/spidev.c | 7 +++---- drivers/uio/uio.c | 6 +++--- fs/coda/psdev.c | 5 ++--- 18 files changed, 39 insertions(+), 49 deletions(-) (limited to 'fs') diff --git a/arch/mips/kernel/rtlx.c b/arch/mips/kernel/rtlx.c index dfd868b68364..4ce93aa7b372 100644 --- a/arch/mips/kernel/rtlx.c +++ b/arch/mips/kernel/rtlx.c @@ -522,8 +522,8 @@ static int __init rtlx_module_init(void) atomic_set(&channel_wqs[i].in_open, 0); mutex_init(&channel_wqs[i].mutex); - dev = device_create_drvdata(mt_class, NULL, MKDEV(major, i), - NULL, "%s%d", module_name, i); + dev = device_create(mt_class, NULL, MKDEV(major, i), NULL, + "%s%d", module_name, i); if (IS_ERR(dev)) { err = PTR_ERR(dev); goto out_chrdev; diff --git a/arch/mips/sibyte/common/sb_tbprof.c b/arch/mips/sibyte/common/sb_tbprof.c index 66e3e3fb311f..637a194e5cd5 100644 --- a/arch/mips/sibyte/common/sb_tbprof.c +++ b/arch/mips/sibyte/common/sb_tbprof.c @@ -576,8 +576,7 @@ static int __init sbprof_tb_init(void) tb_class = tbc; - dev = device_create_drvdata(tbc, NULL, MKDEV(SBPROF_TB_MAJOR, 0), - NULL, "tb"); + dev = device_create(tbc, NULL, MKDEV(SBPROF_TB_MAJOR, 0), NULL, "tb"); if (IS_ERR(dev)) { err = PTR_ERR(dev); goto out_class; diff --git a/arch/x86/kernel/cpuid.c b/arch/x86/kernel/cpuid.c index 6a44d6465991..72cefd1e649b 100644 --- a/arch/x86/kernel/cpuid.c +++ b/arch/x86/kernel/cpuid.c @@ -147,8 +147,8 @@ static __cpuinit int cpuid_device_create(int cpu) { struct device *dev; - dev = device_create_drvdata(cpuid_class, NULL, MKDEV(CPUID_MAJOR, cpu), - NULL, "cpu%d", cpu); + dev = device_create(cpuid_class, NULL, MKDEV(CPUID_MAJOR, cpu), NULL, + "cpu%d", cpu); return IS_ERR(dev) ? PTR_ERR(dev) : 0; } diff --git a/arch/x86/kernel/msr.c b/arch/x86/kernel/msr.c index 2e2af5d18191..82a7c7ed6d45 100644 --- a/arch/x86/kernel/msr.c +++ b/arch/x86/kernel/msr.c @@ -163,8 +163,8 @@ static int __cpuinit msr_device_create(int cpu) { struct device *dev; - dev = device_create_drvdata(msr_class, NULL, MKDEV(MSR_MAJOR, cpu), - NULL, "msr%d", cpu); + dev = device_create(msr_class, NULL, MKDEV(MSR_MAJOR, cpu), NULL, + "msr%d", cpu); return IS_ERR(dev) ? PTR_ERR(dev) : 0; } diff --git a/drivers/dca/dca-sysfs.c b/drivers/dca/dca-sysfs.c index 7af4b403bd2d..bb538b9690e0 100644 --- a/drivers/dca/dca-sysfs.c +++ b/drivers/dca/dca-sysfs.c @@ -15,9 +15,8 @@ int dca_sysfs_add_req(struct dca_provider *dca, struct device *dev, int slot) struct device *cd; static int req_count; - cd = device_create_drvdata(dca_class, dca->cd, - MKDEV(0, slot + 1), NULL, - "requester%d", req_count++); + cd = device_create(dca_class, dca->cd, MKDEV(0, slot + 1), NULL, + "requester%d", req_count++); if (IS_ERR(cd)) return PTR_ERR(cd); return 0; @@ -48,8 +47,7 @@ idr_try_again: return err; } - cd = device_create_drvdata(dca_class, dev, MKDEV(0, 0), NULL, - "dca%d", dca->id); + cd = device_create(dca_class, dev, MKDEV(0, 0), NULL, "dca%d", dca->id); if (IS_ERR(cd)) { spin_lock(&dca_idr_lock); idr_remove(&dca_idr, dca->id); diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c index 497e0d1dd3c3..af3edb98df43 100644 --- a/drivers/hid/hidraw.c +++ b/drivers/hid/hidraw.c @@ -326,9 +326,8 @@ int hidraw_connect(struct hid_device *hid) goto out; } - dev->dev = device_create_drvdata(hidraw_class, NULL, - MKDEV(hidraw_major, minor), NULL, - "%s%d", "hidraw", minor); + dev->dev = device_create(hidraw_class, NULL, MKDEV(hidraw_major, minor), + NULL, "%s%d", "hidraw", minor); if (IS_ERR(dev->dev)) { spin_lock(&minors_lock); diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c index 7321a88a5112..076a59cdabe9 100644 --- a/drivers/hwmon/hwmon.c +++ b/drivers/hwmon/hwmon.c @@ -55,8 +55,8 @@ again: return ERR_PTR(err); id = id & MAX_ID_MASK; - hwdev = device_create_drvdata(hwmon_class, dev, MKDEV(0, 0), NULL, - HWMON_ID_FORMAT, id); + hwdev = device_create(hwmon_class, dev, MKDEV(0, 0), NULL, + HWMON_ID_FORMAT, id); if (IS_ERR(hwdev)) { spin_lock(&idr_lock); diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c index 307d976c9b69..c171988a9f51 100644 --- a/drivers/i2c/i2c-dev.c +++ b/drivers/i2c/i2c-dev.c @@ -521,9 +521,9 @@ static int i2cdev_attach_adapter(struct i2c_adapter *adap) return PTR_ERR(i2c_dev); /* register this i2c device with the driver core */ - i2c_dev->dev = device_create_drvdata(i2c_dev_class, &adap->dev, - MKDEV(I2C_MAJOR, adap->nr), - NULL, "i2c-%d", adap->nr); + i2c_dev->dev = device_create(i2c_dev_class, &adap->dev, + MKDEV(I2C_MAJOR, adap->nr), NULL, + "i2c-%d", adap->nr); if (IS_ERR(i2c_dev->dev)) { res = PTR_ERR(i2c_dev->dev); goto error; diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c index 798d7f3e42ef..1b5bf87c4cf4 100644 --- a/drivers/isdn/capi/capi.c +++ b/drivers/isdn/capi/capi.c @@ -1553,8 +1553,7 @@ static int __init capi_init(void) return PTR_ERR(capi_class); } - device_create_drvdata(capi_class, NULL, MKDEV(capi_major, 0), NULL, - "capi"); + device_create(capi_class, NULL, MKDEV(capi_major, 0), NULL, "capi"); #ifdef CONFIG_ISDN_CAPI_MIDDLEWARE if (capinc_tty_init() < 0) { diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c index 559a40861c39..ee74ee7b2acc 100644 --- a/drivers/leds/led-class.c +++ b/drivers/leds/led-class.c @@ -103,8 +103,8 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev) { int rc; - led_cdev->dev = device_create_drvdata(leds_class, parent, 0, led_cdev, - "%s", led_cdev->name); + led_cdev->dev = device_create(leds_class, parent, 0, led_cdev, + "%s", led_cdev->name); if (IS_ERR(led_cdev->dev)) return PTR_ERR(led_cdev->dev); diff --git a/drivers/macintosh/adb.c b/drivers/macintosh/adb.c index cae52485208a..23741cec45e3 100644 --- a/drivers/macintosh/adb.c +++ b/drivers/macintosh/adb.c @@ -862,8 +862,7 @@ adbdev_init(void) adb_dev_class = class_create(THIS_MODULE, "adb"); if (IS_ERR(adb_dev_class)) return; - device_create_drvdata(adb_dev_class, NULL, MKDEV(ADB_MAJOR, 0), NULL, - "adb"); + device_create(adb_dev_class, NULL, MKDEV(ADB_MAJOR, 0), NULL, "adb"); platform_device_register(&adb_pfdev); platform_driver_probe(&adb_pfdrv, adb_dummy_probe); diff --git a/drivers/media/dvb/dvb-core/dvbdev.c b/drivers/media/dvb/dvb-core/dvbdev.c index e7132770a3bf..665776d72a48 100644 --- a/drivers/media/dvb/dvb-core/dvbdev.c +++ b/drivers/media/dvb/dvb-core/dvbdev.c @@ -233,7 +233,7 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev, mutex_unlock(&dvbdev_register_lock); - clsdev = device_create_drvdata(dvb_class, adap->device, + clsdev = device_create(dvb_class, adap->device, MKDEV(DVB_MAJOR, nums2minor(adap->num, type, id)), NULL, "dvb%d.%s%d", adap->num, dnames[type], id); if (IS_ERR(clsdev)) { diff --git a/drivers/misc/phantom.c b/drivers/misc/phantom.c index daf585689ce3..abdebe347383 100644 --- a/drivers/misc/phantom.c +++ b/drivers/misc/phantom.c @@ -399,9 +399,9 @@ static int __devinit phantom_probe(struct pci_dev *pdev, goto err_irq; } - if (IS_ERR(device_create_drvdata(phantom_class, &pdev->dev, - MKDEV(phantom_major, minor), - NULL, "phantom%u", minor))) + if (IS_ERR(device_create(phantom_class, &pdev->dev, + MKDEV(phantom_major, minor), NULL, + "phantom%u", minor))) dev_err(&pdev->dev, "can't create device\n"); pci_set_drvdata(pdev, pht); diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c index e00d424e6575..1c74762dec89 100644 --- a/drivers/mtd/mtdchar.c +++ b/drivers/mtd/mtdchar.c @@ -26,13 +26,11 @@ static void mtd_notify_add(struct mtd_info* mtd) if (!mtd) return; - device_create_drvdata(mtd_class, NULL, - MKDEV(MTD_CHAR_MAJOR, mtd->index*2), - NULL, "mtd%d", mtd->index); + device_create(mtd_class, NULL, MKDEV(MTD_CHAR_MAJOR, mtd->index*2), + NULL, "mtd%d", mtd->index); - device_create_drvdata(mtd_class, NULL, - MKDEV(MTD_CHAR_MAJOR, mtd->index*2+1), - NULL, "mtd%dro", mtd->index); + device_create(mtd_class, NULL, MKDEV(MTD_CHAR_MAJOR, mtd->index*2+1), + NULL, "mtd%dro", mtd->index); } static void mtd_notify_remove(struct mtd_info* mtd) diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c index cb1ccb472921..3007695f90c8 100644 --- a/drivers/power/power_supply_core.c +++ b/drivers/power/power_supply_core.c @@ -91,8 +91,8 @@ int power_supply_register(struct device *parent, struct power_supply *psy) { int rc = 0; - psy->dev = device_create_drvdata(power_supply_class, parent, 0, - psy, "%s", psy->name); + psy->dev = device_create(power_supply_class, parent, 0, psy, + "%s", psy->name); if (IS_ERR(psy->dev)) { rc = PTR_ERR(psy->dev); goto dev_create_failed; diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c index e5e0cfed5e3b..89a43755a453 100644 --- a/drivers/spi/spidev.c +++ b/drivers/spi/spidev.c @@ -583,10 +583,9 @@ static int spidev_probe(struct spi_device *spi) struct device *dev; spidev->devt = MKDEV(SPIDEV_MAJOR, minor); - dev = device_create_drvdata(spidev_class, &spi->dev, - spidev->devt, spidev, - "spidev%d.%d", - spi->master->bus_num, spi->chip_select); + dev = device_create(spidev_class, &spi->dev, spidev->devt, + spidev, "spidev%d.%d", + spi->master->bus_num, spi->chip_select); status = IS_ERR(dev) ? PTR_ERR(dev) : 0; } else { dev_dbg(&spi->dev, "no minor number available!\n"); diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c index 3a6934bf7131..9ac22c7c3854 100644 --- a/drivers/uio/uio.c +++ b/drivers/uio/uio.c @@ -682,9 +682,9 @@ int __uio_register_device(struct module *owner, if (ret) goto err_get_minor; - idev->dev = device_create_drvdata(uio_class->class, parent, - MKDEV(uio_major, idev->minor), idev, - "uio%d", idev->minor); + idev->dev = device_create(uio_class->class, parent, + MKDEV(uio_major, idev->minor), idev, + "uio%d", idev->minor); if (IS_ERR(idev->dev)) { printk(KERN_ERR "UIO: device register failed\n"); ret = PTR_ERR(idev->dev); diff --git a/fs/coda/psdev.c b/fs/coda/psdev.c index 0d9b80ec689c..cfd29da714d1 100644 --- a/fs/coda/psdev.c +++ b/fs/coda/psdev.c @@ -362,9 +362,8 @@ static int init_coda_psdev(void) goto out_chrdev; } for (i = 0; i < MAX_CODADEVS; i++) - device_create_drvdata(coda_psdev_class, NULL, - MKDEV(CODA_PSDEV_MAJOR, i), - NULL, "cfs%d", i); + device_create(coda_psdev_class, NULL, + MKDEV(CODA_PSDEV_MAJOR, i), NULL, "cfs%d", i); coda_sysctl_init(); goto out; -- cgit v1.2.3 From f1282c844e86db5a041afa41335b5f9eea6cec0c Mon Sep 17 00:00:00 2001 From: Neil Brown Date: Wed, 16 Jul 2008 08:58:04 +1000 Subject: sysfs: Support sysfs_notify from atomic context with new sysfs_notify_dirent Support sysfs_notify from atomic context with new sysfs_notify_dirent sysfs_notify currently takes sysfs_mutex. This means that it cannot be called in atomic context. sysfs_mutex is sometimes held over a malloc (sysfs_rename_dir) so it can block on low memory. In md I want to be able to notify on a sysfs attribute from atomic context, and I don't want to block on low memory because I could be in the writeout path for freeing memory. So: - export the "sysfs_dirent" structure along with sysfs_get, sysfs_put and sysfs_get_dirent so I can get the sysfs_dirent that I want to notify on and hold it in an md structure. - split sysfs_notify_dirent out of sysfs_notify so the sysfs_dirent can be notified on with no blocking (just a spinlock). Signed-off-by: Neil Brown Acked-by: Tejun Heo Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/dir.c | 1 + fs/sysfs/file.c | 31 ++++++++++++++++++------------- fs/sysfs/mount.c | 15 +++++++++++++++ fs/sysfs/sysfs.h | 6 ++++-- include/linux/sysfs.h | 27 ++++++++++++++++++++++++--- 5 files changed, 62 insertions(+), 18 deletions(-) (limited to 'fs') diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index aedaeba82ae5..53bc7fc31af3 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -636,6 +636,7 @@ struct sysfs_dirent *sysfs_get_dirent(struct sysfs_dirent *parent_sd, return sd; } +EXPORT_SYMBOL_GPL(sysfs_get_dirent); static int create_dir(struct kobject *kobj, struct sysfs_dirent *parent_sd, const char *name, struct sysfs_dirent **p_sd) diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index ce8339c70a4b..d0d79e6b6d11 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -453,6 +453,22 @@ static unsigned int sysfs_poll(struct file *filp, poll_table *wait) return POLLERR|POLLPRI; } +void sysfs_notify_dirent(struct sysfs_dirent *sd) +{ + struct sysfs_open_dirent *od; + + spin_lock(&sysfs_open_dirent_lock); + + od = sd->s_attr.open; + if (od) { + atomic_inc(&od->event); + wake_up_interruptible(&od->poll); + } + + spin_unlock(&sysfs_open_dirent_lock); +} +EXPORT_SYMBOL_GPL(sysfs_notify_dirent); + void sysfs_notify(struct kobject *k, char *dir, char *attr) { struct sysfs_dirent *sd = k->sd; @@ -463,19 +479,8 @@ void sysfs_notify(struct kobject *k, char *dir, char *attr) sd = sysfs_find_dirent(sd, dir); if (sd && attr) sd = sysfs_find_dirent(sd, attr); - if (sd) { - struct sysfs_open_dirent *od; - - spin_lock(&sysfs_open_dirent_lock); - - od = sd->s_attr.open; - if (od) { - atomic_inc(&od->event); - wake_up_interruptible(&od->poll); - } - - spin_unlock(&sysfs_open_dirent_lock); - } + if (sd) + sysfs_notify_dirent(sd); mutex_unlock(&sysfs_mutex); } diff --git a/fs/sysfs/mount.c b/fs/sysfs/mount.c index 14f0023984d7..ab343e371d64 100644 --- a/fs/sysfs/mount.c +++ b/fs/sysfs/mount.c @@ -16,6 +16,7 @@ #include #include #include +#include #include "sysfs.h" @@ -115,3 +116,17 @@ out_err: sysfs_dir_cachep = NULL; goto out; } + +#undef sysfs_get +struct sysfs_dirent *sysfs_get(struct sysfs_dirent *sd) +{ + return __sysfs_get(sd); +} +EXPORT_SYMBOL_GPL(sysfs_get); + +#undef sysfs_put +void sysfs_put(struct sysfs_dirent *sd) +{ + __sysfs_put(sd); +} +EXPORT_SYMBOL_GPL(sysfs_put); diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h index a5db496f71c7..93c6d6b27c4d 100644 --- a/fs/sysfs/sysfs.h +++ b/fs/sysfs/sysfs.h @@ -124,7 +124,7 @@ int sysfs_create_subdir(struct kobject *kobj, const char *name, struct sysfs_dirent **p_sd); void sysfs_remove_subdir(struct sysfs_dirent *sd); -static inline struct sysfs_dirent *sysfs_get(struct sysfs_dirent *sd) +static inline struct sysfs_dirent *__sysfs_get(struct sysfs_dirent *sd) { if (sd) { WARN_ON(!atomic_read(&sd->s_count)); @@ -132,12 +132,14 @@ static inline struct sysfs_dirent *sysfs_get(struct sysfs_dirent *sd) } return sd; } +#define sysfs_get(sd) __sysfs_get(sd) -static inline void sysfs_put(struct sysfs_dirent *sd) +static inline void __sysfs_put(struct sysfs_dirent *sd) { if (sd && atomic_dec_and_test(&sd->s_count)) release_sysfs_dirent(sd); } +#define sysfs_put(sd) __sysfs_put(sd) /* * inode.c diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h index 8ec406afb3eb..d8e0230f1e68 100644 --- a/include/linux/sysfs.h +++ b/include/linux/sysfs.h @@ -78,6 +78,8 @@ struct sysfs_ops { ssize_t (*store)(struct kobject *,struct attribute *,const char *, size_t); }; +struct sysfs_dirent; + #ifdef CONFIG_SYSFS int sysfs_schedule_callback(struct kobject *kobj, void (*func)(void *), @@ -118,10 +120,13 @@ void sysfs_remove_file_from_group(struct kobject *kobj, const struct attribute *attr, const char *group); void sysfs_notify(struct kobject *kobj, char *dir, char *attr); - +void sysfs_notify_dirent(struct sysfs_dirent *sd); +struct sysfs_dirent *sysfs_get_dirent(struct sysfs_dirent *parent_sd, + const unsigned char *name); +struct sysfs_dirent *sysfs_get(struct sysfs_dirent *sd); +void sysfs_put(struct sysfs_dirent *sd); void sysfs_printk_last_file(void); - -extern int __must_check sysfs_init(void); +int __must_check sysfs_init(void); #else /* CONFIG_SYSFS */ @@ -227,6 +232,22 @@ static inline void sysfs_remove_file_from_group(struct kobject *kobj, static inline void sysfs_notify(struct kobject *kobj, char *dir, char *attr) { } +static inline void sysfs_notify_dirent(struct sysfs_dirent *sd) +{ +} +static inline +struct sysfs_dirent *sysfs_get_dirent(struct sysfs_dirent *parent_sd, + const unsigned char *name) +{ + return NULL; +} +static inline struct sysfs_dirent *sysfs_get(struct sysfs_dirent *sd) +{ + return NULL; +} +static inline void sysfs_put(struct sysfs_dirent *sd) +{ +} static inline int __must_check sysfs_init(void) { -- cgit v1.2.3 From b31ca3f5dfc89c3f1f654b30f0760d0e2fb15e45 Mon Sep 17 00:00:00 2001 From: Nick Piggin Date: Fri, 12 Sep 2008 11:24:11 +0200 Subject: sysfs: fix deadlock On Thu, Sep 11, 2008 at 10:27:10AM +0200, Ingo Molnar wrote: > and it's working fine on most boxes. One testbox found this new locking > scenario: > > PM: Adding info for No Bus:vcsa7 > EDAC DEBUG: MC0: i82860_check() > > ======================================================= > [ INFO: possible circular locking dependency detected ] > 2.6.27-rc6-tip #1 > ------------------------------------------------------- > X/4873 is trying to acquire lock: > (&bb->mutex){--..}, at: [] mmap+0x40/0xa0 > > but task is already holding lock: > (&mm->mmap_sem){----}, at: [] sys_mmap2+0x8e/0xc0 > > which lock already depends on the new lock. > > > the existing dependency chain (in reverse order) is: > > -> #1 (&mm->mmap_sem){----}: > [] validate_chain+0xa96/0xf50 > [] __lock_acquire+0x2cb/0x5b0 > [] lock_acquire+0x89/0xc0 > [] might_fault+0x6b/0x90 > [] copy_to_user+0x38/0x60 > [] read+0xfb/0x170 > [] vfs_read+0x95/0x110 > [] sys_pread64+0x63/0x80 > [] sysenter_do_call+0x12/0x43 > [] 0xffffffff > > -> #0 (&bb->mutex){--..}: > [] validate_chain+0x6b7/0xf50 > [] __lock_acquire+0x2cb/0x5b0 > [] lock_acquire+0x89/0xc0 > [] __mutex_lock_common+0xab/0x3c0 > [] mutex_lock_nested+0x38/0x50 > [] mmap+0x40/0xa0 > [] mmap_region+0x14e/0x450 > [] do_mmap_pgoff+0x2ef/0x310 > [] sys_mmap2+0xad/0xc0 > [] sysenter_do_call+0x12/0x43 > [] 0xffffffff > > other info that might help us debug this: > > 1 lock held by X/4873: > #0: (&mm->mmap_sem){----}, at: [] sys_mmap2+0x8e/0xc0 > > stack backtrace: > Pid: 4873, comm: X Not tainted 2.6.27-rc6-tip #1 > [] print_circular_bug_tail+0x79/0xc0 > [] validate_chain+0x6b7/0xf50 > [] ? trace_hardirqs_off_caller+0x15/0xb0 > [] __lock_acquire+0x2cb/0x5b0 > [] lock_acquire+0x89/0xc0 > [] ? mmap+0x40/0xa0 > [] __mutex_lock_common+0xab/0x3c0 > [] ? mmap+0x40/0xa0 > [] mutex_lock_nested+0x38/0x50 > [] ? mmap+0x40/0xa0 > [] mmap+0x40/0xa0 > [] mmap_region+0x14e/0x450 > [] ? arch_get_unmapped_area_topdown+0xf8/0x160 > [] do_mmap_pgoff+0x2ef/0x310 > [] sys_mmap2+0xad/0xc0 > [] sysenter_do_call+0x12/0x43 > [] ? __switch_to+0x130/0x220 > ======================= > evbug.c: Event. Dev: input3, Type: 20, Code: 0, Value: 500 > warning: `sudo' uses deprecated v2 capabilities in a way that may be insecure. > > i've attached the config. > > at first sight it looks like a genuine bug in fs/sysfs/bin.c? Yes, it is a real bug by the looks. bin.c takes bb->mutex under mmap_sem when it is mmapped, and then does its copy_*_user under bb->mutex too. Here is a basic fix for the sysfs lor. From: Nick Piggin Signed-off-by: Ingo Molnar Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/bin.c | 42 +++++++++++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 11 deletions(-) (limited to 'fs') diff --git a/fs/sysfs/bin.c b/fs/sysfs/bin.c index 006fc64227dd..66f6e58a7e4b 100644 --- a/fs/sysfs/bin.c +++ b/fs/sysfs/bin.c @@ -61,6 +61,7 @@ read(struct file *file, char __user *userbuf, size_t bytes, loff_t *off) int size = dentry->d_inode->i_size; loff_t offs = *off; int count = min_t(size_t, bytes, PAGE_SIZE); + char *temp; if (size) { if (offs > size) @@ -69,23 +70,33 @@ read(struct file *file, char __user *userbuf, size_t bytes, loff_t *off) count = size - offs; } + temp = kmalloc(count, GFP_KERNEL); + if (!temp) + return -ENOMEM; + mutex_lock(&bb->mutex); count = fill_read(dentry, bb->buffer, offs, count); - if (count < 0) - goto out_unlock; + if (count < 0) { + mutex_unlock(&bb->mutex); + goto out_free; + } - if (copy_to_user(userbuf, bb->buffer, count)) { + memcpy(temp, bb->buffer, count); + + mutex_unlock(&bb->mutex); + + if (copy_to_user(userbuf, temp, count)) { count = -EFAULT; - goto out_unlock; + goto out_free; } pr_debug("offs = %lld, *off = %lld, count = %d\n", offs, *off, count); *off = offs + count; - out_unlock: - mutex_unlock(&bb->mutex); + out_free: + kfree(temp); return count; } @@ -118,6 +129,7 @@ static ssize_t write(struct file *file, const char __user *userbuf, int size = dentry->d_inode->i_size; loff_t offs = *off; int count = min_t(size_t, bytes, PAGE_SIZE); + char *temp; if (size) { if (offs > size) @@ -126,19 +138,27 @@ static ssize_t write(struct file *file, const char __user *userbuf, count = size - offs; } - mutex_lock(&bb->mutex); + temp = kmalloc(count, GFP_KERNEL); + if (!temp) + return -ENOMEM; - if (copy_from_user(bb->buffer, userbuf, count)) { + if (copy_from_user(temp, userbuf, count)) { count = -EFAULT; - goto out_unlock; + goto out_free; } + mutex_lock(&bb->mutex); + + memcpy(bb->buffer, temp, count); + count = flush_write(dentry, bb->buffer, offs, count); + mutex_unlock(&bb->mutex); + if (count > 0) *off = offs + count; - out_unlock: - mutex_unlock(&bb->mutex); +out_free: + kfree(temp); return count; } -- cgit v1.2.3 From 45c076c5d71e6e644e2eae64f80922d162c900ac Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 28 Sep 2008 07:48:08 +0900 Subject: sysfs: use ilookup5() instead of ilookup5_nowait() As inode creation is protected by sysfs_mutex, ilookup5_nowait() always either fails to find at all or finds one which is fully initialized, so using ilookup5_nowait() or ilookup5() doesn't make any difference. Switch to ilookup5() as it's planned to be removed. This change also makes lookup return value handling a bit simpler. This change was suggested by Al Viro. Signed-off-by: Tejun Heo Cc: Al Viro Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/dir.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) (limited to 'fs') diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index 53bc7fc31af3..c18342641cec 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -370,17 +370,17 @@ void sysfs_addrm_start(struct sysfs_addrm_cxt *acxt, memset(acxt, 0, sizeof(*acxt)); acxt->parent_sd = parent_sd; - /* Lookup parent inode. inode initialization and I_NEW - * clearing are protected by sysfs_mutex. By grabbing it and - * looking up with _nowait variant, inode state can be - * determined reliably. + /* Lookup parent inode. inode initialization is protected by + * sysfs_mutex, so inode existence can be determined by + * looking up inode while holding sysfs_mutex. */ mutex_lock(&sysfs_mutex); - inode = ilookup5_nowait(sysfs_sb, parent_sd->s_ino, sysfs_ilookup_test, - parent_sd); + inode = ilookup5(sysfs_sb, parent_sd->s_ino, sysfs_ilookup_test, + parent_sd); + if (inode) { + WARN_ON(inode->i_state & I_NEW); - if (inode && !(inode->i_state & I_NEW)) { /* parent inode available */ acxt->parent_inode = inode; @@ -393,8 +393,7 @@ void sysfs_addrm_start(struct sysfs_addrm_cxt *acxt, mutex_lock(&inode->i_mutex); mutex_lock(&sysfs_mutex); } - } else - iput(inode); + } } /** -- cgit v1.2.3 From 8c0e3998f5b71e68fe6b6e489a92e052715e563c Mon Sep 17 00:00:00 2001 From: Trent Piepho Date: Thu, 25 Sep 2008 16:45:13 -0700 Subject: sysfs: Make dir and name args to sysfs_notify() const Because they can be, and because code like this produces a warning if they're not: struct device_attribute dev_attr; sysfs_notify(&kobj, NULL, dev_attr.attr.name); Signed-off-by: Trent Piepho CC: Neil Brown Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/file.c | 2 +- include/linux/sysfs.h | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) (limited to 'fs') diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index d0d79e6b6d11..1f4a3f877262 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -469,7 +469,7 @@ void sysfs_notify_dirent(struct sysfs_dirent *sd) } EXPORT_SYMBOL_GPL(sysfs_notify_dirent); -void sysfs_notify(struct kobject *k, char *dir, char *attr) +void sysfs_notify(struct kobject *k, const char *dir, const char *attr) { struct sysfs_dirent *sd = k->sd; diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h index d8e0230f1e68..b330e289d71f 100644 --- a/include/linux/sysfs.h +++ b/include/linux/sysfs.h @@ -119,7 +119,7 @@ int sysfs_add_file_to_group(struct kobject *kobj, void sysfs_remove_file_from_group(struct kobject *kobj, const struct attribute *attr, const char *group); -void sysfs_notify(struct kobject *kobj, char *dir, char *attr); +void sysfs_notify(struct kobject *kobj, const char *dir, const char *attr); void sysfs_notify_dirent(struct sysfs_dirent *sd); struct sysfs_dirent *sysfs_get_dirent(struct sysfs_dirent *parent_sd, const unsigned char *name); @@ -229,7 +229,8 @@ static inline void sysfs_remove_file_from_group(struct kobject *kobj, { } -static inline void sysfs_notify(struct kobject *kobj, char *dir, char *attr) +static inline void sysfs_notify(struct kobject *kobj, const char *dir, + const char *attr) { } static inline void sysfs_notify_dirent(struct sysfs_dirent *sd) -- cgit v1.2.3 From 0b4a4fea253e1296222603ccc55430ed7cd9413a Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Thu, 3 Jul 2008 18:05:28 -0700 Subject: kobject: Cleanup kobject_rename and !CONFIG_SYSFS It finally dawned on me what the clean fix to sysfs_rename_dir calling kobject_set_name is. Move the work into kobject_rename where it belongs. The callers serialize us anyway so this is safe. Signed-off-by: Eric W. Biederman Acked-by: Tejun Heo Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/dir.c | 6 +----- include/linux/sysfs.h | 4 +--- lib/kobject.c | 17 +++++++++++++++-- 3 files changed, 17 insertions(+), 10 deletions(-) (limited to 'fs') diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index c18342641cec..3a05a596e3b4 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -829,16 +829,12 @@ int sysfs_rename_dir(struct kobject * kobj, const char *new_name) if (!new_dentry) goto out_unlock; - /* rename kobject and sysfs_dirent */ + /* rename sysfs_dirent */ error = -ENOMEM; new_name = dup_name = kstrdup(new_name, GFP_KERNEL); if (!new_name) goto out_unlock; - error = kobject_set_name(kobj, "%s", new_name); - if (error) - goto out_unlock; - dup_name = sd->s_name; sd->s_name = new_name; diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h index 39924a962207..b330e289d71f 100644 --- a/include/linux/sysfs.h +++ b/include/linux/sysfs.h @@ -20,8 +20,6 @@ struct kobject; struct module; -extern int kobject_set_name(struct kobject *kobj, const char *name, ...) - __attribute__((format(printf, 2, 3))); /* FIXME * The *owner field is no longer used, but leave around * until the tree gets cleaned up fully. @@ -149,7 +147,7 @@ static inline void sysfs_remove_dir(struct kobject *kobj) static inline int sysfs_rename_dir(struct kobject *kobj, const char *new_name) { - return kobject_set_name(kobj, "%s", new_name); + return 0; } static inline int sysfs_move_dir(struct kobject *kobj, diff --git a/lib/kobject.c b/lib/kobject.c index ae6bb900bfb6..0487d1f64806 100644 --- a/lib/kobject.c +++ b/lib/kobject.c @@ -397,6 +397,7 @@ int kobject_rename(struct kobject *kobj, const char *new_name) { int error = 0; const char *devpath = NULL; + const char *dup_name = NULL, *name; char *devpath_string = NULL; char *envp[2]; @@ -420,15 +421,27 @@ int kobject_rename(struct kobject *kobj, const char *new_name) envp[0] = devpath_string; envp[1] = NULL; + name = dup_name = kstrdup(new_name, GFP_KERNEL); + if (!name) { + error = -ENOMEM; + goto out; + } + error = sysfs_rename_dir(kobj, new_name); + if (error) + goto out; + + /* Install the new kobject name */ + dup_name = kobj->name; + kobj->name = name; /* This function is mostly/only used for network interface. * Some hotplug package track interfaces by their name and * therefore want to know when the name is changed by the user. */ - if (!error) - kobject_uevent_env(kobj, KOBJ_MOVE, envp); + kobject_uevent_env(kobj, KOBJ_MOVE, envp); out: + kfree(dup_name); kfree(devpath_string); kfree(devpath); kobject_put(kobj); -- cgit v1.2.3