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
--

[ message continues ]

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 = ...
[ message continues ]

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

--

[ message continues ]

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 ...
[ message continues ]

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(" ...
[ message continues ]

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 ...
[ message continues ]

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
--

[ message continues ]

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

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
--

[ message continues ]

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

--

[ message continues ]

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
--

[ message continues ]

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

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
--

[ message continues ]

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

-- 
tejun
--

[ message continues ]