[PATCHSET 1/3 blk-for-2.6.28] block: misc fixes and improvements

[PATCHSET 1/3 blk-for-2.6.28] block: misc fixes and improvements

[Tejun Heo]

Hello,

This patchset contains the following five patches containing misc
fixes and improvements.

  0001-block-fix-partition-info-printouts.patch
  0002-block-don-t-grab-block_class_lock-unnecessarily.patch
  0003-block-use-class_dev_iterator-instead-of-class_for_e.patch
  0004-block-allow-deleting-zero-length-partition.patch
  0005-block-update-add_partition-error-handling.patch

Recent block_class iteration updates 5c6f35c5..27f3025 introduced a
few bugs and trivial user visible behavior change.  0001-0003 update
those issues and simplify related code using new class iterator.

0004 fixes deletion of zero sized partitions.

d805dda4 tried to fix error handling path in add_partition() but had a
few problems.  0005 fixes those and moves busy partition check into
add_partition().

This patchset is against

  blk-for-2.6.28 (9abd7c437c02e7448fb1d2d3cfc0b9c1ab77cf2d)
+ [1] klist-dont-iterate-over-deleted-entries
+ [2] use-klist-for-class-device-list-and-implement-iterator

and available in the following git tree.

  http://git.kernel.org/?p=linux/kernel/git/tj/misc.git;a=shortlog;h=block-misc-fixes
  git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git block-misc-fixes

with the following combined diffstat

 block/genhd.c         |  246 +++++++++++++++++++-------------------------------
 block/ioctl.c         |    9 -
 fs/partitions/check.c |   42 ++++----
 3 files changed, 121 insertions(+), 176 deletions(-)

Thanks.

--
tejun

[1] http://article.gmane.org/gmane.linux.kernel/725706
[2] http://article.gmane.org/gmane.linux.kernel/725708

[PATCH] block: fix partition info printouts

[Tejun Heo]

Recent block_class iteration updates 5c6f35c5..27f3025 broke partition
info printouts.

* printk_all_partitions(): Partition print out stops when it meets a
  partition hole.  Partition printing inner loop should continue
  instead of exiting on empty partition slot.

* /proc/partitions and /proc/diskstats: If all information can't be
  read in single read(), the information is truncated.  This is
  because find_start() doesn't actually update the counter containing
  the initial seek.  It runs to the end and ends up always reporting
  EOF on the second read.

This patch fixes both problems.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
---
 block/genhd.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index decc8f1..3a43c1d 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -238,7 +238,7 @@ static int printk_partition(struct device *dev, void *data)
 	int n;
 
 	if (dev->type != &disk_type)
-		goto exit;
+		return 0;
 
 	sgp = dev_to_disk(dev);
 	/*
@@ -246,7 +246,7 @@ static int printk_partition(struct device *dev, void *data)
 	 */
 	if (get_capacity(sgp) == 0 ||
 	    (sgp->flags & GENHD_FL_SUPPRESS_PARTITION_INFO))
-		goto exit;
+		return 0;
 
 	/*
 	 * Note, unlike /proc/partitions, I am showing the numbers in
@@ -266,15 +266,15 @@ static int printk_partition(struct device *dev, void *data)
 	/* now show the partitions */
 	for (n = 0; n < sgp->minors - 1; ++n) {
 		if (sgp->part[n] == NULL)
-			goto exit;
+			continue;
 		if (sgp->part[n]->nr_sects == 0)
-			goto exit;
+			continue;
 		printk("  %02x%02x %10llu %s\n",
 			sgp->major, n + 1 + sgp->first_minor,
 			(unsigned long long)sgp->part[n]->nr_sects >> 1,
 			disk_name(sgp, n + 1, buf));
 	}
-exit:
+
 	return 0;
 }
 
@@ -294,11 +294,11 @@ void __init printk_all_partitions(void)
 /* iterator */
 static int find_start(struct device *dev, void *data)
 {
-	loff_t k = *(loff_t *)data;
+	loff_t *k = (loff_t *)data;
 
 	if (dev->type != &disk_type)
 		return 0;
-	if (!k--)
+	if (!(*k)--)
 		return 1;
 	return 0;
 }
@@ -312,7 +312,7 @@ static void *part_start(struct seq_file *part, loff_t *pos)
 		seq_puts(part, "major minor  #blocks  name\n\n");
 
 	mutex_lock(&block_class_lock);
-	dev = class_find_device(&block_class, NULL, (void *)pos, find_start);
+	dev = class_find_device(&block_class, NULL, &n, find_start);
 	if (dev)
 		return dev_to_disk(dev);
 	return NULL;
@@ -569,9 +569,10 @@ static struct device_type disk_type = {
 static void *diskstats_start(struct seq_file *part, loff_t *pos)
 {
 	struct device *dev;
+	loff_t n = *pos;
 
 	mutex_lock(&block_class_lock);
-	dev = class_find_device(&block_class, NULL, (void *)pos, find_start);
+	dev = class_find_device(&block_class, NULL, &n, find_start);
 	if (dev)
 		return dev_to_disk(dev);
 	return NULL;
-- 
1.5.4.5

[PATCH] block: don't grab block_class_lock unnecessarily

[Tejun Heo]

block_class_lock protects major_names array and bdev_map and doesn't
have anything to do with block class devices.  Don't grab them while
iterating over block class devices.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 block/genhd.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 3a43c1d..5a1511e 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -285,9 +285,7 @@ static int printk_partition(struct device *dev, void *data)
  */
 void __init printk_all_partitions(void)
 {
-	mutex_lock(&block_class_lock);
 	class_for_each_device(&block_class, NULL, NULL, printk_partition);
-	mutex_unlock(&block_class_lock);
 }
 
 #ifdef CONFIG_PROC_FS
@@ -311,7 +309,6 @@ static void *part_start(struct seq_file *part, loff_t *pos)
 	if (!n)
 		seq_puts(part, "major minor  #blocks  name\n\n");
 
-	mutex_lock(&block_class_lock);
 	dev = class_find_device(&block_class, NULL, &n, find_start);
 	if (dev)
 		return dev_to_disk(dev);
@@ -338,7 +335,6 @@ static void *part_next(struct seq_file *part, void *v, loff_t *pos)
 
 static void part_stop(struct seq_file *part, void *v)
 {
-	mutex_unlock(&block_class_lock);
 }
 
 static int show_partition(struct seq_file *part, void *v)
@@ -571,7 +567,6 @@ static void *diskstats_start(struct seq_file *part, loff_t *pos)
 	struct device *dev;
 	loff_t n = *pos;
 
-	mutex_lock(&block_class_lock);
 	dev = class_find_device(&block_class, NULL, &n, find_start);
 	if (dev)
 		return dev_to_disk(dev);
@@ -592,7 +587,6 @@ static void *diskstats_next(struct seq_file *part, void *v, loff_t *pos)
 
 static void diskstats_stop(struct seq_file *part, void *v)
 {
-	mutex_unlock(&block_class_lock);
 }
 
 static int diskstats_show(struct seq_file *s, void *v)
@@ -711,14 +705,12 @@ dev_t blk_lookup_devt(const char *name, int part)
 	dev_t devt = MKDEV(0, 0);
 	struct find_block find;
 
-	mutex_lock(&block_class_lock);
 	find.name = name;
 	find.part = part;
 	dev = class_find_device(&block_class, NULL, (void *)&find, match_id);
 	if (dev)
 		devt = MKDEV(MAJOR(dev->devt),
 			     MINOR(dev->devt) + part);
-	mutex_unlock(&block_class_lock);
 
 	return devt;
 }
-- 
1.5.4.5

[PATCH] block: use class_dev_iterator instead of class_for_each_device()

[Tejun Heo]

Recent block_class iteration updates 5c6f35c5..27f3025 converted all
class device iteration to class_for_each_device() and
class_find_device(), which are correct but pain in the ass to use.
This pach converts them to newly introduced class_dev_iterator so that
they can use more natural control structures instead of separate
callbacks and struct to pass parameters to them.

This results in smaller and easier code.

This patch also restores the original behavior of not printing header
in /proc/partitions if there's no partition to print.  This is trivial
but still user-visible behavior.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
---
 block/genhd.c |  239 +++++++++++++++++++++++---------------------------------
 1 files changed, 98 insertions(+), 141 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 5a1511e..e9d60fd 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -228,113 +228,113 @@ struct gendisk *get_gendisk(dev_t devt, int *part)
 }
 
 /*
- * print a partitions - intended for places where the root filesystem can't be
- * mounted and thus to give the victim some idea of what went wrong
- */
-static int printk_partition(struct device *dev, void *data)
-{
-	struct gendisk *sgp;
-	char buf[BDEVNAME_SIZE];
-	int n;
-
-	if (dev->type != &disk_type)
-		return 0;
-
-	sgp = dev_to_disk(dev);
-	/*
-	 * Don't show empty devices or things that have been surpressed
-	 */
-	if (get_capacity(sgp) == 0 ||
-	    (sgp->flags & GENHD_FL_SUPPRESS_PARTITION_INFO))
-		return 0;
-
-	/*
-	 * Note, unlike /proc/partitions, I am showing the numbers in
-	 * hex - the same format as the root= option takes.
-	 */
-	printk("%02x%02x %10llu %s",
-		sgp->major, sgp->first_minor,
-		(unsigned long long)get_capacity(sgp) >> 1,
-		disk_name(sgp, 0, buf));
-	if (sgp->driverfs_dev != NULL &&
-	    sgp->driverfs_dev->driver != NULL)
-		printk(" driver: %s\n",
-			sgp->driverfs_dev->driver->name);
-	else
-		printk(" (driver?)\n");
-
-	/* now show the partitions */
-	for (n = 0; n < sgp->minors - 1; ++n) {
-		if (sgp->part[n] == NULL)
-			continue;
-		if (sgp->part[n]->nr_sects == 0)
-			continue;
-		printk("  %02x%02x %10llu %s\n",
-			sgp->major, n + 1 + sgp->first_minor,
-			(unsigned long long)sgp->part[n]->nr_sects >> 1,
-			disk_name(sgp, n + 1, buf));
-	}
-
-	return 0;
-}
-
-/*
  * print a full list of all partitions - intended for places where the root
  * filesystem can't be mounted and thus to give the victim some idea of what
  * went wrong
  */
 void __init printk_all_partitions(void)
 {
-	class_for_each_device(&block_class, NULL, NULL, printk_partition);
+	struct class_dev_iter iter;
+	struct device *dev;
+
+	class_dev_iter_init(&iter, &block_class, NULL, &disk_type);
+	while ((dev = class_dev_iter_next(&iter))) {
+		struct gendisk *disk = dev_to_disk(dev);
+		char buf[BDEVNAME_SIZE];
+		int n;
+
+		/*
+		 * Don't show empty devices or things that have been
+		 * surpressed
+		 */
+		if (get_capacity(disk) == 0 ||
+		    (disk->flags & GENHD_FL_SUPPRESS_PARTITION_INFO))
+			continue;
+
+		/*
+		 * Note, unlike /proc/partitions, I am showing the
+		 * numbers in hex - the same format as the root=
+		 * option takes.
+		 */
+		printk("%02x%02x %10llu %s",
+		       disk->major, disk->first_minor,
+		       (unsigned long long)get_capacity(disk) >> 1,
+		       disk_name(disk, 0, buf));
+		if (disk->driverfs_dev != NULL &&
+		    disk->driverfs_dev->driver != NULL)
+			printk(" driver: %s\n",
+			       disk->driverfs_dev->driver->name);
+		else
+			printk(" (driver?)\n");
+
+		/* now show the partitions */
+		for (n = 0; n < disk->minors - 1; ++n) {
+			if (disk->part[n] == NULL)
+				continue;
+			if (disk->part[n]->nr_sects == 0)
+				continue;
+			printk("  %02x%02x %10llu %s\n",
+			       disk->major, n + 1 + disk->first_minor,
+			       (unsigned long long)disk->part[n]->nr_sects >> 1,
+			       disk_name(disk, n + 1, buf));
+		}
+	}
+	class_dev_iter_exit(&iter);
 }
 
 #ifdef CONFIG_PROC_FS
 /* iterator */
-static int find_start(struct device *dev, void *data)
+static void *disk_seqf_start(struct seq_file *seqf, loff_t *pos)
 {
-	loff_t *k = (loff_t *)data;
+	loff_t skip = *pos;
+	struct class_dev_iter *iter;
+	struct device *dev;
 
-	if (dev->type != &disk_type)
-		return 0;
-	if (!(*k)--)
-		return 1;
-	return 0;
+	iter = kmalloc(GFP_KERNEL, sizeof(*iter));
+	if (!iter)
+		return ERR_PTR(-ENOMEM);
+
+	seqf->private = iter;
+	class_dev_iter_init(iter, &block_class, NULL, &disk_type);
+	do {
+		dev = class_dev_iter_next(iter);
+		if (!dev)
+			return NULL;
+	} while (skip--);
+
+	return dev_to_disk(dev);
 }
 
-static void *part_start(struct seq_file *part, loff_t *pos)
+static void *disk_seqf_next(struct seq_file *seqf, void *v, loff_t *pos)
 {
 	struct device *dev;
-	loff_t n = *pos;
-
-	if (!n)
-		seq_puts(part, "major minor  #blocks  name\n\n");
 
-	dev = class_find_device(&block_class, NULL, &n, find_start);
+	(*pos)++;
+	dev = class_dev_iter_next(seqf->private);
 	if (dev)
 		return dev_to_disk(dev);
 	return NULL;
 }
 
-static int find_next(struct device *dev, void *data)
+static void disk_seqf_stop(struct seq_file *seqf, void *v)
 {
-	if (dev->type == &disk_type)
-		return 1;
-	return 0;
-}
+	struct class_dev_iter *iter = seqf->private;
 
-static void *part_next(struct seq_file *part, void *v, loff_t *pos)
-{
-	struct gendisk *gp = v;
-	struct device *dev;
-	++*pos;
-	dev = class_find_device(&block_class, &gp->dev, NULL, find_next);
-	if (dev)
-		return dev_to_disk(dev);
-	return NULL;
+	/* stop is called even after start failed :-( */
+	if (iter) {
+		class_dev_iter_exit(iter);
+		kfree(iter);
+	}
 }
 
-static void part_stop(struct seq_file *part, void *v)
+static void *show_partition_start(struct seq_file *seqf, loff_t *pos)
 {
+	static void *p;
+
+	p = disk_seqf_start(seqf, pos);
+	if (!IS_ERR(p) && p)
+		seq_puts(part, "major minor  #blocks  name\n\n");
+	return p;
 }
 
 static int show_partition(struct seq_file *part, void *v)
@@ -370,9 +370,9 @@ static int show_partition(struct seq_file *part, void *v)
 }
 
 const struct seq_operations partitions_op = {
-	.start	= part_start,
-	.next	= part_next,
-	.stop	= part_stop,
+	.start	= show_partition_start,
+	.next	= disk_seqf_next,
+	.stop	= disk_seqf_stop,
 	.show	= show_partition
 };
 #endif
@@ -561,34 +561,6 @@ static struct device_type disk_type = {
  * The output looks suspiciously like /proc/partitions with a bunch of
  * extra fields.
  */
-
-static void *diskstats_start(struct seq_file *part, loff_t *pos)
-{
-	struct device *dev;
-	loff_t n = *pos;
-
-	dev = class_find_device(&block_class, NULL, &n, find_start);
-	if (dev)
-		return dev_to_disk(dev);
-	return NULL;
-}
-
-static void *diskstats_next(struct seq_file *part, void *v, loff_t *pos)
-{
-	struct gendisk *gp = v;
-	struct device *dev;
-
-	++*pos;
-	dev = class_find_device(&block_class, &gp->dev, NULL, find_next);
-	if (dev)
-		return dev_to_disk(dev);
-	return NULL;
-}
-
-static void diskstats_stop(struct seq_file *part, void *v)
-{
-}
-
 static int diskstats_show(struct seq_file *s, void *v)
 {
 	struct gendisk *gp = v;
@@ -650,9 +622,9 @@ static int diskstats_show(struct seq_file *s, void *v)
 }
 
 const struct seq_operations diskstats_op = {
-	.start	= diskstats_start,
-	.next	= diskstats_next,
-	.stop	= diskstats_stop,
+	.start	= disk_seqf_start,
+	.next	= disk_seqf_next,
+	.stop	= disk_seqf_stop,
 	.show	= diskstats_show
 };
 #endif /* CONFIG_PROC_FS */
@@ -680,38 +652,23 @@ void genhd_media_change_notify(struct gendisk *disk)
 EXPORT_SYMBOL_GPL(genhd_media_change_notify);
 #endif  /*  0  */
 
-struct find_block {
-	const char *name;
-	int part;
-};
-
-static int match_id(struct device *dev, void *data)
-{
-	struct find_block *find = data;
-
-	if (dev->type != &disk_type)
-		return 0;
-	if (strcmp(dev->bus_id, find->name) == 0) {
-		struct gendisk *disk = dev_to_disk(dev);
-		if (find->part < disk->minors)
-			return 1;
-	}
-	return 0;
-}
-
 dev_t blk_lookup_devt(const char *name, int part)
 {
-	struct device *dev;
 	dev_t devt = MKDEV(0, 0);
-	struct find_block find;
+	struct class_dev_iter iter;
+	struct device *dev;
 
-	find.name = name;
-	find.part = part;
-	dev = class_find_device(&block_class, NULL, (void *)&find, match_id);
-	if (dev)
-		devt = MKDEV(MAJOR(dev->devt),
-			     MINOR(dev->devt) + part);
+	class_dev_iter_init(&iter, &block_class, NULL, &disk_type);
+	while ((dev = class_dev_iter_next(&iter))) {
+		struct gendisk *disk = dev_to_disk(dev);
 
+		if (!strcmp(dev->bus_id, name) && part < disk->minors) {
+			devt = MKDEV(MAJOR(dev->devt),
+				     MINOR(dev->devt) + part);
+			break;
+		}
+	}
+	class_dev_iter_exit(&iter);
 	return devt;
 }
 EXPORT_SYMBOL(blk_lookup_devt);
-- 
1.5.4.5

[PATCH] block: allow deleting zero length partition

[Tejun Heo]

delete_partition() was noop for zero length partition.  As the
addition code allows creating zero lenght partition and deletion is
assumed to always succeed, this causes memory leak for zero length
partitions.  Allow zero length partitions to end their meaningless
lives.

While at it, allow deleting zero lenght partition via
BLKPG_DEL_PARTITION ioctl too.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 block/ioctl.c         |    2 --
 fs/partitions/check.c |    2 --
 2 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/block/ioctl.c b/block/ioctl.c
index 375c579..c722de0 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -68,8 +68,6 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
 		case BLKPG_DEL_PARTITION:
 			if (!disk->part[part-1])
 				return -ENXIO;
-			if (disk->part[part - 1]->nr_sects == 0)
-				return -ENXIO;
 			bdevp = bdget_disk(disk, part);
 			if (!bdevp)
 				return -ENOMEM;
diff --git a/fs/partitions/check.c b/fs/partitions/check.c
index 7d6b34e..1b1808e 100644
--- a/fs/partitions/check.c
+++ b/fs/partitions/check.c
@@ -325,8 +325,6 @@ void delete_partition(struct gendisk *disk, int part)
 
 	if (!p)
 		return;
-	if (!p->nr_sects)
-		return;
 	disk->part[part-1] = NULL;
 	p->start_sect = 0;
 	p->nr_sects = 0;
-- 
1.5.4.5

[PATCH] block: update add_partition() error handling

[Tejun Heo]

d805dda4 tried to fix error case handling in add_partition() but had a
few problems.

* disk->part[] entry is set early and left dangling if operation
  fails.

* Once device initialized, the last put_device() is responsible for
  freeing all the resources.  The failure path freed part_stats and p
  regardless of put_device() causing double free.

* holders subdir holds reference to the disk device, so failure path
  should remove it to release resources properly which was missing.

This patch fixes the above problems and while at it move partition
slot busy check into add_partition() for completeness and inlines
holders subdirectory creation.  Using separate function for it just
obfuscates the code.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Abdel Benamrouche <draconux@gmail.com>
---
 block/ioctl.c         |    7 ++-----
 fs/partitions/check.c |   40 +++++++++++++++++++++-------------------
 2 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/block/ioctl.c b/block/ioctl.c
index c722de0..eb046ae 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -43,12 +43,9 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
 				    || pstart < 0 || plength < 0)
 					return -EINVAL;
 			}
-			/* partition number in use? */
+
 			mutex_lock(&bdev->bd_mutex);
-			if (disk->part[part - 1]) {
-				mutex_unlock(&bdev->bd_mutex);
-				return -EBUSY;
-			}
+
 			/* overlap? */
 			for (i = 0; i < disk->minors - 1; i++) {
 				struct hd_struct *s = disk->part[i];
diff --git a/fs/partitions/check.c b/fs/partitions/check.c
index 1b1808e..43dbfab 100644
--- a/fs/partitions/check.c
+++ b/fs/partitions/check.c
@@ -300,15 +300,6 @@ struct device_type part_type = {
 	.release	= part_release,
 };
 
-static inline void partition_sysfs_add_subdir(struct hd_struct *p)
-{
-	struct kobject *k;
-
-	k = kobject_get(&p->dev.kobj);
-	p->holder_dir = kobject_create_and_add("holders", k);
-	kobject_put(k);
-}
-
 static inline void disk_sysfs_add_subdirs(struct gendisk *disk)
 {
 	struct kobject *k;
@@ -347,13 +338,16 @@ int add_partition(struct gendisk *disk, int part, sector_t start, sector_t len,
 	struct hd_struct *p;
 	int err;
 
+	if (disk->part[part - 1])
+		return -EBUSY;
+
 	p = kzalloc(sizeof(*p), GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
 
 	if (!init_part_stats(p)) {
 		err = -ENOMEM;
-		goto out0;
+		goto out_free;
 	}
 	p->start_sect = start;
 	p->nr_sects = len;
@@ -372,34 +366,42 @@ int add_partition(struct gendisk *disk, int part, sector_t start, sector_t len,
 	p->dev.class = &block_class;
 	p->dev.type = &part_type;
 	p->dev.parent = &disk->dev;
-	disk->part[part-1] = p;
 
 	/* delay uevent until 'holders' subdir is created */
 	p->dev.uevent_suppress = 1;
 	err = device_add(&p->dev);
 	if (err)
-		goto out1;
-	partition_sysfs_add_subdir(p);
+		goto out_put;
+
+	err = -ENOMEM;
+	p->holder_dir = kobject_create_and_add("holders", &p->dev.kobj);
+	if (!p->holder_dir)
+		goto out_del;
+
 	p->dev.uevent_suppress = 0;
 	if (flags & ADDPART_FLAG_WHOLEDISK) {
 		err = device_create_file(&p->dev, &dev_attr_whole_disk);
 		if (err)
-			goto out2;
+			goto out_del;
 	}
 
+	/* everything is up and running, commence */
+	disk->part[part - 1] = p;
+
 	/* suppress uevent if the disk supresses it */
 	if (!disk->dev.uevent_suppress)
 		kobject_uevent(&p->dev.kobj, KOBJ_ADD);
 
 	return 0;
 
-out2:
+out_free:
+	kfree(p);
+	return err;
+out_del:
+	kobject_put(p->holder_dir);
 	device_del(&p->dev);
-out1:
+out_put:
 	put_device(&p->dev);
-	free_part_stats(p);
-out0:
-	kfree(p);
 	return err;
 }
 
-- 
1.5.4.5

Re: [PATCHSET 1/3 blk-for-2.6.28] block: misc fixes and improvements

[Tejun Heo]

Tejun Heo wrote:
> Hello,
> 
> This patchset contains the following five patches containing misc
> fixes and improvements.
> 
>   0001-block-fix-partition-info-printouts.patch
>   0002-block-don-t-grab-block_class_lock-unnecessarily.patch
>   0003-block-use-class_dev_iterator-instead-of-class_for_e.patch
>   0004-block-allow-deleting-zero-length-partition.patch
>   0005-block-update-add_partition-error-handling.patch

Oops, forgot to pass -n to git-format-patch.  The above order is correct
and the git tree has it all ordered, so I won't resend to the mailing
list.  Jens, I'll resend with proper numbers to you privately.

Thanks.

-- 
tejun

Re: [PATCHSET 1/3 blk-for-2.6.28] block: misc fixes and improvements

[Greg KH]

On Mon, Aug 25, 2008 at 06:15:35PM +0900, Tejun Heo wrote:
> Hello,
> 
> This patchset contains the following five patches containing misc
> fixes and improvements.
> 
>   0001-block-fix-partition-info-printouts.patch
>   0002-block-don-t-grab-block_class_lock-unnecessarily.patch
>   0003-block-use-class_dev_iterator-instead-of-class_for_e.patch
>   0004-block-allow-deleting-zero-length-partition.patch
>   0005-block-update-add_partition-error-handling.patch
> 
> Recent block_class iteration updates 5c6f35c5..27f3025 introduced a
> few bugs and trivial user visible behavior change.  0001-0003 update
> those issues and simplify related code using new class iterator.

The bugs should now be fixed in Linus's tree due to a fix from Kay,
right?

The user visable behavior change, I missed that, sorry, that is a
regression, do you think it's a big deal to fix in 2.6.27?

> 0004 fixes deletion of zero sized partitions.
> 
> d805dda4 tried to fix error handling path in add_partition() but had a
> few problems.  0005 fixes those and moves busy partition check into
> add_partition().
> 
> This patchset is against
> 
>   blk-for-2.6.28 (9abd7c437c02e7448fb1d2d3cfc0b9c1ab77cf2d)
> + [1] klist-dont-iterate-over-deleted-entries
> + [2] use-klist-for-class-device-list-and-implement-iterator
> 
> and available in the following git tree.
> 
>   http://git.kernel.org/?p=linux/kernel/git/tj/misc.git;a=shortlog;h=block-misc-fixes
>   git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git block-misc-fixes
> 
> with the following combined diffstat

I have no problem taking all of these patches as I did this original
work and don't want to burden Jens with this.  Also, I'd like to work
off of the klist changes and class changes as that can be used in other
work that I want to do in the driver core.

Jens any objection to me taking them?  Do you also have changes in this
area that this needs to be merged with?

thanks,

greg k-h

Re: [PATCHSET 1/3 blk-for-2.6.28] block: misc fixes and improvements

[Tejun Heo]

Greg KH wrote:
>> Recent block_class iteration updates 5c6f35c5..27f3025 introduced a
>> few bugs and trivial user visible behavior change.  0001-0003 update
>> those issues and simplify related code using new class iterator.
> 
> The bugs should now be fixed in Linus's tree due to a fix from Kay,
> right?

Ah.. okay, it's fixed in Linus's tree.  Anyways, what do you think about
the new iterator?

> The user visable behavior change, I missed that, sorry, that is a
> regression, do you think it's a big deal to fix in 2.6.27?

It's probably something which no one really cares but it's not difficult
to fix and having the behavior changed for 2.6.27 and reverted in 28
isn't too nice.  I'll send a patch.

Thanks.

-- 
tejun

Re: [PATCHSET 1/3 blk-for-2.6.28] block: misc fixes and improvements

[Tejun Heo]

Tejun Heo wrote:
> Greg KH wrote:
>>> Recent block_class iteration updates 5c6f35c5..27f3025 introduced a
>>> few bugs and trivial user visible behavior change.  0001-0003 update
>>> those issues and simplify related code using new class iterator.
>> The bugs should now be fixed in Linus's tree due to a fix from Kay,
>> right?
> 
> Ah.. okay, it's fixed in Linus's tree.  Anyways, what do you think about
> the new iterator?

Aiee.. never mind the last question.  I somehow missed the last
paragraph.  :-)

-- 
tejun

Re: [PATCHSET 1/3 blk-for-2.6.28] block: misc fixes and improvements

[Jens Axboe]

On Mon, Aug 25 2008, Greg KH wrote:
> On Mon, Aug 25, 2008 at 06:15:35PM +0900, Tejun Heo wrote:
> > Hello,
> > 
> > This patchset contains the following five patches containing misc
> > fixes and improvements.
> > 
> >   0001-block-fix-partition-info-printouts.patch
> >   0002-block-don-t-grab-block_class_lock-unnecessarily.patch
> >   0003-block-use-class_dev_iterator-instead-of-class_for_e.patch
> >   0004-block-allow-deleting-zero-length-partition.patch
> >   0005-block-update-add_partition-error-handling.patch
> > 
> > Recent block_class iteration updates 5c6f35c5..27f3025 introduced a
> > few bugs and trivial user visible behavior change.  0001-0003 update
> > those issues and simplify related code using new class iterator.
> 
> The bugs should now be fixed in Linus's tree due to a fix from Kay,
> right?
> 
> The user visable behavior change, I missed that, sorry, that is a
> regression, do you think it's a big deal to fix in 2.6.27?
> 
> > 0004 fixes deletion of zero sized partitions.
> > 
> > d805dda4 tried to fix error handling path in add_partition() but had a
> > few problems.  0005 fixes those and moves busy partition check into
> > add_partition().
> > 
> > This patchset is against
> > 
> >   blk-for-2.6.28 (9abd7c437c02e7448fb1d2d3cfc0b9c1ab77cf2d)
> > + [1] klist-dont-iterate-over-deleted-entries
> > + [2] use-klist-for-class-device-list-and-implement-iterator
> > 
> > and available in the following git tree.
> > 
> >   http://git.kernel.org/?p=linux/kernel/git/tj/misc.git;a=shortlog;h=block-misc-fixes
> >   git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git block-misc-fixes
> > 
> > with the following combined diffstat
> 
> I have no problem taking all of these patches as I did this original
> work and don't want to burden Jens with this.  Also, I'd like to work
> off of the klist changes and class changes as that can be used in other
> work that I want to do in the driver core.
> 
> Jens any objection to me taking them?  Do you also have changes in this
> area that this needs to be merged with?

It's actually easier for me to include them. Apart from a potential
clash in linux-next there, there shouldn't be any problem in that.

-- 
Jens Axboe

Re: [PATCHSET 1/3 blk-for-2.6.28] block: misc fixes and improvements

[Greg KH]

On Mon, Aug 25, 2008 at 04:58:41PM +0200, Jens Axboe wrote:
> On Mon, Aug 25 2008, Greg KH wrote:
> > On Mon, Aug 25, 2008 at 06:15:35PM +0900, Tejun Heo wrote:
> > > Hello,
> > > 
> > > This patchset contains the following five patches containing misc
> > > fixes and improvements.
> > > 
> > >   0001-block-fix-partition-info-printouts.patch
> > >   0002-block-don-t-grab-block_class_lock-unnecessarily.patch
> > >   0003-block-use-class_dev_iterator-instead-of-class_for_e.patch
> > >   0004-block-allow-deleting-zero-length-partition.patch
> > >   0005-block-update-add_partition-error-handling.patch
> > > 
> > > Recent block_class iteration updates 5c6f35c5..27f3025 introduced a
> > > few bugs and trivial user visible behavior change.  0001-0003 update
> > > those issues and simplify related code using new class iterator.
> > 
> > The bugs should now be fixed in Linus's tree due to a fix from Kay,
> > right?
> > 
> > The user visable behavior change, I missed that, sorry, that is a
> > regression, do you think it's a big deal to fix in 2.6.27?
> > 
> > > 0004 fixes deletion of zero sized partitions.
> > > 
> > > d805dda4 tried to fix error handling path in add_partition() but had a
> > > few problems.  0005 fixes those and moves busy partition check into
> > > add_partition().
> > > 
> > > This patchset is against
> > > 
> > >   blk-for-2.6.28 (9abd7c437c02e7448fb1d2d3cfc0b9c1ab77cf2d)
> > > + [1] klist-dont-iterate-over-deleted-entries
> > > + [2] use-klist-for-class-device-list-and-implement-iterator
> > > 
> > > and available in the following git tree.
> > > 
> > >   http://git.kernel.org/?p=linux/kernel/git/tj/misc.git;a=shortlog;h=block-misc-fixes
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git block-misc-fixes
> > > 
> > > with the following combined diffstat
> > 
> > I have no problem taking all of these patches as I did this original
> > work and don't want to burden Jens with this.  Also, I'd like to work
> > off of the klist changes and class changes as that can be used in other
> > work that I want to do in the driver core.
> > 
> > Jens any objection to me taking them?  Do you also have changes in this
> > area that this needs to be merged with?
> 
> It's actually easier for me to include them. Apart from a potential
> clash in linux-next there, there shouldn't be any problem in that.

Ok, that's fine.  Feel free to add my:
	Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
to them.

I'll grab the klist and class patch and also add them to my trees, that
should be easy to resolve in the linux-next merges.

thanks,

greg k-h