On Monday January 14, neilb@xxxxxxx wrote: > > Thanks. I'll see what I can some up with. How about this, against current -mm On both the read and write path for an rdev attribute, we call mddev_lock, first checking that mddev is not NULL. Once we get the lock, we check again. If rdev->mddev is not NULL, we know it will stay that way as it only gets cleared under the same lock. While in the rdev show/store routines, we know that the mddev cannot get freed, do to the kobject relationships. rdev_size_store is awkward because it has to drop the lock. So we take a copy of rdev->mddev before the drop, and we are safe... Comments? NeilBrown Signed-off-by: Neil Brown <neilb@xxxxxxx> ### Diffstat output ./drivers/md/md.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff .prev/drivers/md/md.c ./drivers/md/md.c --- .prev/drivers/md/md.c 2008-01-14 12:26:15.000000000 +1100 +++ ./drivers/md/md.c 2008-01-14 17:05:53.000000000 +1100 @@ -1998,9 +1998,11 @@ rdev_size_store(mdk_rdev_t *rdev, const char *e; unsigned long long size = simple_strtoull(buf, &e, 10); unsigned long long oldsize = rdev->size; + mddev_t *my_mddev = rdev->mddev; + if (e==buf || (*e && *e != '\n')) return -EINVAL; - if (rdev->mddev->pers) + if (my_mddev->pers) return -EBUSY; rdev->size = size; if (size > oldsize && rdev->mddev->external) { @@ -2013,7 +2015,7 @@ rdev_size_store(mdk_rdev_t *rdev, const int overlap = 0; struct list_head *tmp, *tmp2; - mddev_unlock(rdev->mddev); + mddev_unlock(my_mddev); for_each_mddev(mddev, tmp) { mdk_rdev_t *rdev2; @@ -2033,7 +2035,7 @@ rdev_size_store(mdk_rdev_t *rdev, const break; } } - mddev_lock(rdev->mddev); + mddev_lock(my_mddev); if (overlap) { /* Someone else could have slipped in a size * change here, but doing so is just silly. @@ -2045,8 +2047,8 @@ rdev_size_store(mdk_rdev_t *rdev, const return -EBUSY; } } - if (size < rdev->mddev->size || rdev->mddev->size == 0) - rdev->mddev->size = size; + if (size < my_mddev->size || my_mddev->size == 0) + my_mddev->size = size; return len; } @@ -2067,10 +2069,21 @@ rdev_attr_show(struct kobject *kobj, str { struct rdev_sysfs_entry *entry = container_of(attr, struct rdev_sysfs_entry, attr); mdk_rdev_t *rdev = container_of(kobj, mdk_rdev_t, kobj); + mddev_t *mddev = rdev->mddev; + ssize_t rv; if (!entry->show) return -EIO; - return entry->show(rdev, page); + + rv = mddev ? mddev_lock(mddev) : -EBUSY; + if (!rv) { + if (rdev->mddev == NULL) + rv = -EBUSY; + else + rv = entry->show(rdev, page); + mddev_unlock(mddev); + } + return rv; } static ssize_t @@ -2079,15 +2092,19 @@ rdev_attr_store(struct kobject *kobj, st { struct rdev_sysfs_entry *entry = container_of(attr, struct rdev_sysfs_entry, attr); mdk_rdev_t *rdev = container_of(kobj, mdk_rdev_t, kobj); - int rv; + ssize_t rv; + mddev_t *mddev = rdev->mddev; if (!entry->store) return -EIO; if (!capable(CAP_SYS_ADMIN)) return -EACCES; - rv = mddev_lock(rdev->mddev); + rv = mddev ? mddev_lock(mddev): -EBUSY; if (!rv) { - rv = entry->store(rdev, page, length); + if (rdev->mddev == NULL) + rv = -EBUSY; + else + rv = entry->store(rdev, page, length); mddev_unlock(rdev->mddev); } return rv; - To unsubscribe from this list: send the line "unsubscribe linux-raid" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html