All of lore.kernel.org
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.de>
To: Andrew Morton <akpm@osdl.org>
Cc: Greg KH <greg@kroah.com>, linux-raid@vger.kernel.org
Subject: [PATCH md 004 of 10] Fix some locking and module refcounting issues with md's use of sysfs.
Date: Wed, 2 Nov 2005 21:15:10 +1100	[thread overview]
Message-ID: <1051102101510.23887@suse.de> (raw)
In-Reply-To: 20051102205640.22689.patches@notabene


1/ I really should be using the __ATTR macros for defining attributes, so
 that the .owner field get set properly, otherwise modules can be removed
 while sysfs files are open.
 This also involves some name changes of _show routines.

2/ Always lock the mddev (against reconfiguration) for all sysfs
 attribute access.  This easily avoid certain races and is completely
 consistant with other interfaces (ioctl and /proc/mdstat both 
 always lock against reconfiguration).

3/ raid5 attributes must check that the 'conf' structure actually 
  exists (the array could have been stopped while an attribute file
  was open).

4/ A missing 'kfree' from when the raid5_conf_t was converted to have
  a kobject embedded, and then converted back again.



Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./drivers/md/md.c    |   62 +++++++++++++++++++++++----------------------------
 ./drivers/md/raid5.c |   30 ++++++++++++++----------
 2 files changed, 46 insertions(+), 46 deletions(-)

diff ./drivers/md/md.c~current~ ./drivers/md/md.c
--- ./drivers/md/md.c~current~	2005-11-02 17:32:41.000000000 +1100
+++ ./drivers/md/md.c	2005-11-02 17:33:16.000000000 +1100
@@ -1504,7 +1504,7 @@ struct rdev_sysfs_entry {
 };
 
 static ssize_t
-rdev_show_state(mdk_rdev_t *rdev, char *page)
+state_show(mdk_rdev_t *rdev, char *page)
 {
 	char *sep = "";
 	int len=0;
@@ -1525,13 +1525,11 @@ rdev_show_state(mdk_rdev_t *rdev, char *
 	return len+sprintf(page+len, "\n");
 }
 
-static struct rdev_sysfs_entry rdev_state = {
-	.attr = {.name = "state", .mode = S_IRUGO },
-	.show = rdev_show_state,
-};
+static struct rdev_sysfs_entry
+rdev_state = __ATTR_RO(state);
 
 static ssize_t
-rdev_show_super(mdk_rdev_t *rdev, char *page)
+super_show(mdk_rdev_t *rdev, char *page)
 {
 	if (rdev->sb_loaded && rdev->sb_size) {
 		memcpy(page, page_address(rdev->sb_page), rdev->sb_size);
@@ -1539,10 +1537,8 @@ rdev_show_super(mdk_rdev_t *rdev, char *
 	} else
 		return 0;
 }
-static struct rdev_sysfs_entry rdev_super = {
-	.attr = {.name = "super", .mode = S_IRUGO },
-	.show = rdev_show_super,
-};
+static struct rdev_sysfs_entry rdev_super = __ATTR_RO(super);
+
 static struct attribute *rdev_default_attrs[] = {
 	&rdev_state.attr,
 	&rdev_super.attr,
@@ -1728,7 +1724,7 @@ static void analyze_sbs(mddev_t * mddev)
 }
 
 static ssize_t
-md_show_level(mddev_t *mddev, char *page)
+level_show(mddev_t *mddev, char *page)
 {
 	mdk_personality_t *p = mddev->pers;
 	if (p == NULL)
@@ -1739,21 +1735,15 @@ md_show_level(mddev_t *mddev, char *page
 		return sprintf(page, "%s\n", p->name);
 }
 
-static struct md_sysfs_entry md_level = {
-	.attr = {.name = "level", .mode = S_IRUGO },
-	.show = md_show_level,
-};
+static struct md_sysfs_entry md_level = __ATTR_RO(level);
 
 static ssize_t
-md_show_rdisks(mddev_t *mddev, char *page)
+raid_disks_show(mddev_t *mddev, char *page)
 {
 	return sprintf(page, "%d\n", mddev->raid_disks);
 }
 
-static struct md_sysfs_entry md_raid_disks = {
-	.attr = {.name = "raid_disks", .mode = S_IRUGO },
-	.show = md_show_rdisks,
-};
+static struct md_sysfs_entry md_raid_disks = __ATTR_RO(raid_disks);
 
 static ssize_t
 md_show_scan(mddev_t *mddev, char *page)
@@ -1782,10 +1772,10 @@ md_store_scan(mddev_t *mddev, const char
 	if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) ||
 	    test_bit(MD_RECOVERY_NEEDED, &mddev->recovery))
 		return -EBUSY;
-	down(&mddev->reconfig_sem);
+
 	if (mddev->pers && mddev->pers->sync_request)
 		canscan=1;
-	up(&mddev->reconfig_sem);
+
 	if (!canscan)
 		return -EINVAL;
 
@@ -1801,22 +1791,18 @@ md_store_scan(mddev_t *mddev, const char
 }
 
 static ssize_t
-md_show_mismatch(mddev_t *mddev, char *page)
+mismatch_cnt_show(mddev_t *mddev, char *page)
 {
 	return sprintf(page, "%llu\n",
 		       (unsigned long long) mddev->resync_mismatches);
 }
 
-static struct md_sysfs_entry md_scan_mode = {
-	.attr = {.name = "scan_mode", .mode = S_IRUGO|S_IWUSR },
-	.show = md_show_scan,
-	.store = md_store_scan,
-};
+static struct md_sysfs_entry
+md_scan_mode = __ATTR(scan_mode, S_IRUGO|S_IWUSR, md_show_scan, md_store_scan);
 
-static struct md_sysfs_entry md_mismatches = {
-	.attr = {.name = "mismatch_cnt", .mode = S_IRUGO },
-	.show = md_show_mismatch,
-};
+
+static struct md_sysfs_entry
+md_mismatches = __ATTR_RO(mismatch_cnt);
 
 static struct attribute *md_default_attrs[] = {
 	&md_level.attr,
@@ -1831,10 +1817,14 @@ md_attr_show(struct kobject *kobj, struc
 {
 	struct md_sysfs_entry *entry = container_of(attr, struct md_sysfs_entry, attr);
 	mddev_t *mddev = container_of(kobj, struct mddev_s, kobj);
+	ssize_t rv;
 
 	if (!entry->show)
 		return -EIO;
-	return entry->show(mddev, page);
+	mddev_lock(mddev);
+	rv = entry->show(mddev, page);
+	mddev_unlock(mddev);
+	return rv;
 }
 
 static ssize_t
@@ -1843,10 +1833,14 @@ md_attr_store(struct kobject *kobj, stru
 {
 	struct md_sysfs_entry *entry = container_of(attr, struct md_sysfs_entry, attr);
 	mddev_t *mddev = container_of(kobj, struct mddev_s, kobj);
+	ssize_t rv;
 
 	if (!entry->store)
 		return -EIO;
-	return entry->store(mddev, page, length);
+	mddev_lock(mddev);
+	rv = entry->store(mddev, page, length);
+	mddev_unlock(mddev);
+	return rv;
 }
 
 static void md_free(struct kobject *ko)

diff ./drivers/md/raid5.c~current~ ./drivers/md/raid5.c
--- ./drivers/md/raid5.c~current~	2005-11-02 17:32:57.000000000 +1100
+++ ./drivers/md/raid5.c	2005-11-02 17:33:17.000000000 +1100
@@ -1750,7 +1750,10 @@ static ssize_t
 raid5_show_stripe_cache_size(mddev_t *mddev, char *page)
 {
 	raid5_conf_t *conf = mddev_to_conf(mddev);
-	return sprintf(page, "%d\n", conf->max_nr_stripes);
+	if (conf)
+		return sprintf(page, "%d\n", conf->max_nr_stripes);
+	else
+		return 0;
 }
 
 static ssize_t
@@ -1761,6 +1764,8 @@ raid5_store_stripe_cache_size(mddev_t *m
 	int new;
 	if (len >= PAGE_SIZE)
 		return -EINVAL;
+	if (!conf)
+		return -ENODEV;
 
 	new = simple_strtoul(page, &end, 10);
 	if (!*page || (*end && *end != '\n') )
@@ -1781,23 +1786,23 @@ raid5_store_stripe_cache_size(mddev_t *m
 	return len;
 }
 
-static struct md_sysfs_entry raid5_stripecache_size = {
-	.attr = {.name = "stripe_cache_size", .mode = S_IRUGO | S_IWUSR },
-	.show = raid5_show_stripe_cache_size,
-	.store = raid5_store_stripe_cache_size,
-};
+static struct md_sysfs_entry
+raid5_stripecache_size = __ATTR(stripe_cache_size, S_IRUGO | S_IWUSR,
+				raid5_show_stripe_cache_size,
+				raid5_store_stripe_cache_size);
 
 static ssize_t
-raid5_show_stripe_cache_active(mddev_t *mddev, char *page)
+stripe_cache_active_show(mddev_t *mddev, char *page)
 {
 	raid5_conf_t *conf = mddev_to_conf(mddev);
-	return sprintf(page, "%d\n", atomic_read(&conf->active_stripes));
+	if (conf)
+		return sprintf(page, "%d\n", atomic_read(&conf->active_stripes));
+	else
+		return 0;
 }
 
-static struct md_sysfs_entry raid5_stripecache_active = {
-	.attr = {.name = "stripe_cache_active", .mode = S_IRUGO},
-	.show = raid5_show_stripe_cache_active,
-};
+static struct md_sysfs_entry
+raid5_stripecache_active = __ATTR_RO(stripe_cache_active);
 
 static struct attribute *raid5_attrs[] =  {
 	&raid5_stripecache_size.attr,
@@ -1985,6 +1990,7 @@ static int stop(mddev_t *mddev)
 	free_pages((unsigned long) conf->stripe_hashtbl, HASH_PAGES_ORDER);
 	blk_sync_queue(mddev->queue); /* the unplug fn references 'conf'*/
 	sysfs_remove_group(&mddev->kobj, &raid5_attrs_group);
+	kfree(conf);
 	mddev->private = NULL;
 	return 0;
 }

  parent reply	other threads:[~2005-11-02 10:15 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-11-02 10:14 [PATCH md 000 of 10] Introduction NeilBrown
2005-11-02 10:14 ` [PATCH md 001 of 10] Make sure /block link in /sys/.../md/ goes to correct devices NeilBrown
2005-11-02 21:46   ` Greg KH
2005-11-02 10:14 ` [PATCH md 002 of 10] Make manual repair work for raid1 NeilBrown
2005-11-02 10:15 ` [PATCH md 003 of 10] Make sure a user-request sync of raid5 ignores intent bitmap NeilBrown
2005-11-02 10:15 ` NeilBrown [this message]
2005-11-02 21:47   ` [PATCH md 004 of 10] Fix some locking and module refcounting issues with md's use of sysfs Greg KH
2005-11-02 10:15 ` [PATCH md 005 of 10] Split off some md attributes in sysfs to a separate group NeilBrown
2005-11-02 21:48   ` Greg KH
2005-11-02 10:15 ` [PATCH md 006 of 10] Only try to print recovery/resync status for personalities that support recovery NeilBrown
2005-11-02 10:15 ` [PATCH md 007 of 10] Ignore auto-readonly flag for arrays where it isn't meaningful NeilBrown
2005-11-02 10:15 ` [PATCH md 008 of 10] Complete conversion of md to use kthreads NeilBrown
2005-11-02 10:15 ` [PATCH md 009 of 10] Improve 'scan_mode' and rename it to 'sync_action' NeilBrown
2005-11-02 21:49   ` Greg KH
2005-11-02 10:16 ` [PATCH md 010 of 10] Document sysfs usage of md, and make a couple of small refinements NeilBrown
2005-11-02 21:50   ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1051102101510.23887@suse.de \
    --to=neilb@suse.de \
    --cc=akpm@osdl.org \
    --cc=greg@kroah.com \
    --cc=linux-raid@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.