Add partition resize function to BLKPG ioctl

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Add a new operation code ( BLKPG_RES_PARTITION ) to the BLKPG ioctl that allows altering the size of an existing partition, even if it is currently in use.

- ---

 block/ioctl.c         |   51 +++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/blkpg.h |    1 +
 2 files changed, 50 insertions(+), 2 deletions(-)

index ca939fc..f97e6a4 100644

- --- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -36,8 +36,8 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
case BLKPG_ADD_PARTITION:
start = p.start >> 9;
length = p.length >> 9;
- - /* check for fit in a hd_struct */
- - if (sizeof(sector_t) == sizeof(long) &&
+ /* check for fit in a hd_struct */
+ if (sizeof(sector_t) == sizeof(long) &&
sizeof(long long) > sizeof(long)) {
long pstart = start, plength = length;
if (pstart != start || plength != length
@@ -92,6 +92,53 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
bdput(bdevp);

return 0;
+ case BLKPG_RES_PARTITION:
+ start = p.start >> 9;
+ length = p.length >> 9;
+ /* check for fit in a hd_struct */
+ if (sizeof(sector_t) == sizeof(long) &&
+ sizeof(long long) > sizeof(long)) {
+ long pstart = start, plength = length;
+ if (pstart != start || plength != length
+ || pstart < 0 || plength < 0)
+ return -EINVAL;
+ }
+
+ mutex_lock(&bdev->bd_mutex);
+
+ /* overlap? */
+ disk_part_iter_init(&piter, disk,
+ DISK_PITER_INCL_EMPTY);
+ while ((part = disk_part_iter_next(&piter))) {
+ if (part->partno != partno && !(start + length <= part->start_sect ||
+ start >= part->start_sect + part->nr_sects)) {
+ disk_part_iter_exit(&piter);
+ mutex_unlock(&bdev->bd_mutex);
+ return -EBUSY;
+ }
+ }
+ disk_part_iter_exit(&piter);
+ part = disk_get_part(disk, partno);
+ if (!part)
+ {
+ mutex_unlock(&bdev->bd_mutex);
+ return -ENXIO;
+ }
+ if (start != part->start_sect)
+ {
+ mutex_unlock(&bdev->bd_mutex);
+ disk_put_part(part);
+ return -EINVAL;
+ }
+ part->nr_sects = length;
+ bdevp = bdget(part_devt(part));
+ mutex_lock(&bdevp->bd_mutex);
+ i_size_write(bdevp->bd_inode, p.length);
+ mutex_unlock(&bdevp->bd_mutex);
+ bdput(bdevp);
+ disk_put_part(part);
+ mutex_unlock(&bdev->bd_mutex);
+ return 0;
default:
return -EINVAL;
}

index faf8a45..103da38 100644

- --- a/include/linux/blkpg.h
+++ b/include/linux/blkpg.h
@@ -40,6 +40,7 @@ struct blkpg_ioctl_arg {
/* The subfunctions (for the op field) */
#define BLKPG_ADD_PARTITION 1
#define BLKPG_DEL_PARTITION 2
+#define BLKPG_RES_PARTITION 3

/* Sizes of name fields. Unused at present. */
#define BLKPG_DEVNAMELTH 64
- --
1.7.5.4

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk7W8yAACgkQJ4UciIs+XuKpiwCfd8YC2241ScOcruAoGvBnTyVZ p6UAoI5JT/UX4gkilpjmTKbuBLzeCTv4
=K/d3
-----END PGP SIGNATURE-----

It seems very close to Vivek's BLKPG_EXTEND_PARTITION idea, see:

    https://lkml.org/lkml/2011/9/1/351

It's evident that we (userspace) need something better than BLKPG_{ADD,DEL}_* and BLKRRPART, but no response from kernel side :-(

Karel

 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com

Yes, this looks very similar to mine, except that I also wanted to allow shrinking the partition, not just extending, since btrfs can handle this ( tested ).

Is it safe to alter the partition size in arbitrary way if the partition is used by any process?

Vivek's BLKPG_EXTEND_PARTITIONV seems more safety, because it extends a partition size only, so all offsets in all running stuff are still valid.

Maybe you need to check bdevp->bd_openers and returns -EBUSY if you want to alter the begin of the partition.

Karel

 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com

I disallowed altering the start of the partition ( that would just be crazy ), but altering the end has been supported on lvm for years now. Ext4 can not perform an online shrink, but btrfs can, and I was able to successfully have btrfs shrink the fs and then use BLKPG_RES_PARTITION to shrink the partition.

So, if there is an IO in flight while partition shrinking is happening, then IO can end up happening outside the partition?

part->nr_sects can be 64 bits on 32bit machines and update will be non-atomic. I had used sequence counter to make sure read is able to get to intermediate value. May be it is a good idea to address this concenrn.

Thanks
Vivek

Yes. Of course if there was an in flight IO at the time you truncate the partition, then you did something wrong ( didn't shrink the fs first ).

Isn't that what the mutex is for?

Ok, so one needs to shrink filesystem. So if partition is part of a volume group one is supposed to first reduce the size using pvresize and then shrink actual partition?

I thought I have found couple of places where we don't take mutex. For example.

drive_stat_acct()
disk_map_sector_rcu()
sector_in_part()

    --> read part->nr_sects without mutex.

printk_all_partitions()
---> read part->nr_sects without mutex.

show_partition()
---> read part->nr_sects without mutex.

Thanks
Vivek

Yes.

I merged your seq changes into my patch. How does this look?

From 8dbca9e824dd18eeb44079717234db0e853f8478 Mon Sep 17 00:00:00 2001
From: Phillip Susi <[email protected]>
Date: Wed, 30 Nov 2011 22:05:24 -0500
Subject: [PATCH 1/3] Add partition resize function to BLKPG ioctl

Add a new operation code ( BLKPG_RES_PARTITION ) to the BLKPG ioctl that allows altering the size of an existing partition, even if it is currently in use.

---
 block/genhd.c         |   10 ++++----
 block/ioctl.c         |   51 +++++++++++++++++++++++++++++++++++++-
 fs/partitions/check.c |   65 ++++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/blkpg.h |    1 +
 include/linux/genhd.h |    7 +++++
 5 files changed, 126 insertions(+), 8 deletions(-)

index 02e9fca..b34fb90 100644
@@ -154,7 +154,7 @@ struct hd_struct *disk_part_iter_next(struct disk_part_iter *piter)
 		part = rcu_dereference(ptbl->part[piter->idx]);
 		if (!part)
 			continue;
-		if (!part->nr_sects &&
+		if (!part_nr_sects_read(part) &&
 		    !(piter->flags & DISK_PITER_INCL_EMPTY) &&
 		    !(piter->flags & DISK_PITER_INCL_EMPTY_PART0 &&
 		      piter->idx == 0))

@@ -191,7 +191,7 @@ EXPORT_SYMBOL_GPL(disk_part_iter_exit);
 static inline int sector_in_part(struct hd_struct *part, sector_t sector)
 {
 	return part->start_sect <= sector &&
-		sector < part->start_sect + part->nr_sects;
+		sector < part->start_sect + part_nr_sects_read(part);
 }
 
 /**

@@ -766,8 +766,8 @@ void __init printk_all_partitions(void)
 
 			printk("%s%s %10llu %s %s", is_part0 ? "" : "  ",
 			       bdevt_str(part_devt(part), devt_buf),
-			       (unsigned long long)part->nr_sects >> 1,
-			       disk_name(disk, part->partno, name_buf), uuid);
+			       (unsigned long long)part_nr_sects_read(part) >> 1
+			       , disk_name(disk, part->partno, name_buf), uuid);
 			if (is_part0) {
 				if (disk->driverfs_dev != NULL &&
 				    disk->driverfs_dev->driver != NULL)

@@ -858,7 +858,7 @@ static int show_partition(struct seq_file *seqf, void *v)
 	while ((part = disk_part_iter_next(&piter)))
 		seq_printf(seqf, "%4d  %7d %10llu %s\n",
 			   MAJOR(part_devt(part)), MINOR(part_devt(part)),
-			   (unsigned long long)part->nr_sects >> 1,
+			   (unsigned long long)part_nr_sects_read(part) >> 1,
 			   disk_name(sgp, part->partno, buf));
 	disk_part_iter_exit(&piter);
 

index faf8a45..103da38 100644
@@ -40,6 +40,7 @@ struct blkpg_ioctl_arg {
 /* The subfunctions (for the op field) */
 #define BLKPG_ADD_PARTITION	1
 #define BLKPG_DEL_PARTITION	2
+#define BLKPG_RES_PARTITION	3
 
 /* Sizes of name fields. Unused at present. */
 #define BLKPG_DEVNAMELTH	64

index 6d18f35..a55ce2c 100644
@@ -98,7 +98,13 @@ struct partition_meta_info {
 
 struct hd_struct {
 	sector_t start_sect;
+	/*
+	 * nr_sects is protected by sequence counter. One might extend a
+	 * partition while IO is happening to it and update of nr_sects
+	 * can be non-atomic on 32bit machines with 64bit sector_t.
+	 */
 	sector_t nr_sects;
+	seqcount_t seq;
 	sector_t alignment_offset;
 	unsigned int discard_alignment;
 	struct device __dev;

@@ -604,6 +610,7 @@ extern struct hd_struct * __must_check add_partition(struct gendisk *disk,
 extern void __delete_partition(struct hd_struct *);
 extern void delete_partition(struct gendisk *, int);
 extern void printk_all_partitions(void);
+extern sector_t part_nr_sects_read(struct hd_struct *part);
 
 extern struct gendisk *alloc_disk_node(int minors, int node_id);
 extern struct gendisk *alloc_disk(int minors);

1.7.5.4

Can you please inline the patch so that reviewing becomes easier. Right now it is an attachment.

Couple of quick comments.

- I think during copy/paste tabs have been replaced by spaces at places.
- You have retained function extend_partition() from my patch. I think you are not using it.
- Can we rename BLKPG_RES_PARTITION to BLKPG_RESIZE_PARTITION. It is easier to read.

Thanks
Vivek

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hrm... it shows up inline for me. I guess I'll copy/paste this time.

Woops, fixed.

BLKPG_DELETE_PARTITION is also easier to read, but the existing code went with DEL. I prefer to stay consistent with the existing code and also use a 3 letter name.

- From de3a303233515b89c4d18869daca1bd5e6775895 Mon Sep 17 00:00:00 2001
From: Phillip Susi <[email protected]>
Date: Mon, 12 Dec 2011 12:33:36 -0500
Subject: [PATCH] Add partition resize function to BLKPG ioctl

Add a new operation code ( BLKPG_RES_PARTITION ) to the BLKPG ioctl that allows altering the size of an existing partition, even if it is currently in use.

Thanks to Vivek Goyal for the seq counter idea.
- ---

 block/genhd.c         |   10 ++++----
 block/ioctl.c         |   51 +++++++++++++++++++++++++++++++++++++++++++++++-
 fs/partitions/check.c |   39 ++++++++++++++++++++++++++++++++++++-
 include/linux/blkpg.h |    1 +
 include/linux/genhd.h |    7 ++++++
 5 files changed, 100 insertions(+), 8 deletions(-)

index 02e9fca..b34fb90 100644

- --- a/block/genhd.c
+++ b/block/genhd.c
@@ -154,7 +154,7 @@ struct hd_struct *disk_part_iter_next(struct disk_part_iter *piter)
part = rcu_dereference(ptbl->part[piter->idx]);
if (!part)
continue;
- - if (!part->nr_sects &&
+ if (!part_nr_sects_read(part) &&
!(piter->flags & DISK_PITER_INCL_EMPTY) && !(piter->flags & DISK_PITER_INCL_EMPTY_PART0 &&
piter->idx == 0))
@@ -191,7 +191,7 @@ EXPORT_SYMBOL_GPL(disk_part_iter_exit);
static inline int sector_in_part(struct hd_struct *part, sector_t sector) {
return part->start_sect <= sector &&
- - sector < part->start_sect + part->nr_sects;
+ sector < part->start_sect + part_nr_sects_read(part);
}

/**
@@ -766,8 +766,8 @@ void __init printk_all_partitions(void)

printk("%s%s %10llu %s %s", is_part0 ? "" : " ",
bdevt_str(part_devt(part), devt_buf),
- - (unsigned long long)part->nr_sects >> 1,
- - disk_name(disk, part->partno, name_buf), uuid);
+ (unsigned long long)part_nr_sects_read(part) >> 1
+ , disk_name(disk, part->partno, name_buf), uuid);
if (is_part0) {
if (disk->driverfs_dev != NULL &&
disk->driverfs_dev->driver != NULL)
@@ -858,7 +858,7 @@ static int show_partition(struct seq_file *seqf, void *v)
while ((part = disk_part_iter_next(&piter)))
seq_printf(seqf, "%4d %7d %10llu %s\n",
MAJOR(part_devt(part)), MINOR(part_devt(part)),
- - (unsigned long long)part->nr_sects >> 1,
+ (unsigned long long)part_nr_sects_read(part) >> 1,
disk_name(sgp, part->partno, buf));
disk_part_iter_exit(&piter);

index ca939fc..f97e6a4 100644

- --- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -36,8 +36,8 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
case BLKPG_ADD_PARTITION:
start = p.start >> 9;
length = p.length >> 9;
- - /* check for fit in a hd_struct */
- - if (sizeof(sector_t) == sizeof(long) &&
+ /* check for fit in a hd_struct */
+ if (sizeof(sector_t) == sizeof(long) &&
sizeof(long long) > sizeof(long)) {
long pstart = start, plength = length;
if (pstart != start || plength != length
@@ -92,6 +92,53 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
bdput(bdevp);

return 0;
+ case BLKPG_RES_PARTITION:
+ start = p.start >> 9;
+ length = p.length >> 9;
+ /* check for fit in a hd_struct */
+ if (sizeof(sector_t) == sizeof(long) &&
+ sizeof(long long) > sizeof(long)) {
+ long pstart = start, plength = length;
+ if (pstart != start || plength != length
+ || pstart < 0 || plength < 0)
+ return -EINVAL;
+ }
+
+ mutex_lock(&bdev->bd_mutex);
+
+ /* overlap? */
+ disk_part_iter_init(&piter, disk,
+ DISK_PITER_INCL_EMPTY);
+ while ((part = disk_part_iter_next(&piter))) {
+ if (part->partno != partno && !(start + length <= part->start_sect ||
+ start >= part->start_sect + part->nr_sects)) {
+ disk_part_iter_exit(&piter);
+ mutex_unlock(&bdev->bd_mutex);
+ return -EBUSY;
+ }
+ }
+ disk_part_iter_exit(&piter);
+ part = disk_get_part(disk, partno);
+ if (!part)
+ {
+ mutex_unlock(&bdev->bd_mutex);
+ return -ENXIO;
+ }
+ if (start != part->start_sect)
+ {
+ mutex_unlock(&bdev->bd_mutex);
+ disk_put_part(part);
+ return -EINVAL;
+ }
+ part->nr_sects = length;
+ bdevp = bdget(part_devt(part));
+ mutex_lock(&bdevp->bd_mutex);
+ i_size_write(bdevp->bd_inode, p.length);
+ mutex_unlock(&bdevp->bd_mutex);
+ bdput(bdevp);
+ disk_put_part(part);
+ mutex_unlock(&bdev->bd_mutex);
+ return 0;
default:
return -EINVAL;
}

index e3c63d1..089e4bc 100644

- --- a/fs/partitions/check.c
+++ b/fs/partitions/check.c
@@ -234,7 +234,7 @@ ssize_t part_size_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct hd_struct *p = dev_to_part(dev);
- - return sprintf(buf, "%llu\n",(unsigned long long)p->nr_sects);
+ return sprintf(buf, "%llu\n",(unsigned long long)part_nr_sects_read(p));
}

static ssize_t part_ro_show(struct device *dev,
@@ -408,6 +408,43 @@ void delete_partition(struct gendisk *disk, int partno)
hd_struct_put(part);
}

+#if BITS_PER_LONG == 32 && defined(CONFIG_LBDAF)
+static inline void part_nr_sects_write_begin(struct seqcount_t *seq) +{
+ write_seqcount_begin(&seq);
+}
+
+static inline void part_nr_sects_write_end(struct seqcount_t *seq) +{
+ write_seqcount_end(&seq);
+}
+
+/*
+ * Any access of part->nr_sects which is not protected by partition
+ * bd_mutex or gendisk bdev bd_mutex, hould be done using this accessor
+ * function.
+ */
+sector_t part_nr_sects_read(struct hd_struct *part)
+{
+ sector_t nr_sects;
+ unsigned seq;
+
+ do {
+ seq = read_seqcount_begin(&part->seq);
+ nr_sects = part->nr_sects;
+ } while (read_seqcount_retry(&part->seq, seq));
+
+ return nr_sects;
+}
+#else
+static inline void part_nr_sects_write_begin(seqcount_t *seq) {} +static inline void part_nr_sects_write_end(seqcount_t *seq) {} +sector_t part_nr_sects_read(struct hd_struct *part)
+{
+ return part->nr_sects;
+}
+#endif
+
static ssize_t whole_disk_show(struct device *dev,
struct device_attribute *attr, char *buf)
{

index faf8a45..103da38 100644

- --- a/include/linux/blkpg.h
+++ b/include/linux/blkpg.h
@@ -40,6 +40,7 @@ struct blkpg_ioctl_arg {
/* The subfunctions (for the op field) */
#define BLKPG_ADD_PARTITION 1
#define BLKPG_DEL_PARTITION 2
+#define BLKPG_RES_PARTITION 3

/* Sizes of name fields. Unused at present. */
#define BLKPG_DEVNAMELTH 64

index 6d18f35..a55ce2c 100644

- --- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -98,7 +98,13 @@ struct partition_meta_info {

struct hd_struct {
sector_t start_sect;
+ /*
+ * nr_sects is protected by sequence counter. One might extend a
+ * partition while IO is happening to it and update of nr_sects
+ * can be non-atomic on 32bit machines with 64bit sector_t.
+ */
sector_t nr_sects;
+ seqcount_t seq;
sector_t alignment_offset;
unsigned int discard_alignment;
struct device __dev;
@@ -604,6 +610,7 @@ extern struct hd_struct * __must_check add_partition(struct gendisk *disk,
extern void __delete_partition(struct hd_struct *);
extern void delete_partition(struct gendisk *, int);
extern void printk_all_partitions(void);
+extern sector_t part_nr_sects_read(struct hd_struct *part);

extern struct gendisk *alloc_disk_node(int minors, int node_id); extern struct gendisk *alloc_disk(int minors);
- --
1.7.5.4

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk7mPT4ACgkQJ4UciIs+XuJs4QCfS4zI3KeVsI1trJ/135mZMz6F 6J8AnRy1hH++V32jUgait02EYeHfuI+c
=6guc
-----END PGP SIGNATURE-----

[]

RES is most commonly used for RESOURCE.

If it really needs to be 3 letters, and I don't think it does, perhaps BLKPG_RSZ_PARTITION

Agreed. RES does not occur to me as resize but resources. ADD and DEL are very clear.

Thanks
Vivek

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ok, I suppose RESIZE is fine. Here come both patches, hopefully final. -----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk7mmQoACgkQJ4UciIs+XuIDkwCdEvqpBaxWw5jmgj2PBs/0pStB IyQAoJHW2U4GeBRy7f/WdKWlN0Vk6pgC
=x4de
-----END PGP SIGNATURE-----

Add a new operation code ( BLKPG_RESIZE_PARTITION ) to the BLKPG ioctl that allows altering the size of an existing partition, even if it is currently in use.

Thanks to Vivek Goyal for the seq counter idea.

---
 block/genhd.c         |   10 ++++----
 block/ioctl.c         |   51 +++++++++++++++++++++++++++++++++++++++++++++++-
 fs/partitions/check.c |   39 ++++++++++++++++++++++++++++++++++++-
 include/linux/blkpg.h |    1 +
 include/linux/genhd.h |    7 ++++++
 5 files changed, 100 insertions(+), 8 deletions(-)

index 02e9fca..b34fb90 100644
@@ -154,7 +154,7 @@ struct hd_struct *disk_part_iter_next(struct disk_part_iter *piter)
 		part = rcu_dereference(ptbl->part[piter->idx]);
 		if (!part)
 			continue;
-		if (!part->nr_sects &&
+		if (!part_nr_sects_read(part) &&
 		    !(piter->flags & DISK_PITER_INCL_EMPTY) &&
 		    !(piter->flags & DISK_PITER_INCL_EMPTY_PART0 &&
 		      piter->idx == 0))

@@ -191,7 +191,7 @@ EXPORT_SYMBOL_GPL(disk_part_iter_exit);
 static inline int sector_in_part(struct hd_struct *part, sector_t sector)
 {
 	return part->start_sect <= sector &&
-		sector < part->start_sect + part->nr_sects;
+		sector < part->start_sect + part_nr_sects_read(part);
 }
 
 /**

@@ -766,8 +766,8 @@ void __init printk_all_partitions(void)
 
 			printk("%s%s %10llu %s %s", is_part0 ? "" : "  ",
 			       bdevt_str(part_devt(part), devt_buf),
-			       (unsigned long long)part->nr_sects >> 1,
-			       disk_name(disk, part->partno, name_buf), uuid);
+			       (unsigned long long)part_nr_sects_read(part) >> 1
+			       , disk_name(disk, part->partno, name_buf), uuid);
 			if (is_part0) {
 				if (disk->driverfs_dev != NULL &&
 				    disk->driverfs_dev->driver != NULL)

@@ -858,7 +858,7 @@ static int show_partition(struct seq_file *seqf, void *v)
 	while ((part = disk_part_iter_next(&piter)))
 		seq_printf(seqf, "%4d  %7d %10llu %s\n",
 			   MAJOR(part_devt(part)), MINOR(part_devt(part)),
-			   (unsigned long long)part->nr_sects >> 1,
+			   (unsigned long long)part_nr_sects_read(part) >> 1,
 			   disk_name(sgp, part->partno, buf));
 	disk_part_iter_exit(&piter);
 

index e3c63d1..089e4bc 100644
@@ -234,7 +234,7 @@ ssize_t part_size_show(struct device *dev,
 		       struct device_attribute *attr, char *buf)
 {
 	struct hd_struct *p = dev_to_part(dev);
-	return sprintf(buf, "%llu\n",(unsigned long long)p->nr_sects);
+	return sprintf(buf, "%llu\n",(unsigned long long)part_nr_sects_read(p));
 }
 
 static ssize_t part_ro_show(struct device *dev,

@@ -408,6 +408,43 @@ void delete_partition(struct gendisk *disk, int partno)
 	hd_struct_put(part);
 }
 
+#if BITS_PER_LONG == 32 && defined(CONFIG_LBDAF)
+static inline void part_nr_sects_write_begin(struct seqcount_t *seq)
+{
+	write_seqcount_begin(&seq);
+}
+
+static inline void part_nr_sects_write_end(struct seqcount_t *seq)
+{
+	write_seqcount_end(&seq);
+}
+
+/*
+ * Any access of part->nr_sects which is not protected by partition
+ * bd_mutex or gendisk bdev bd_mutex, hould be done using this accessor
+ * function.
+ */
+sector_t part_nr_sects_read(struct hd_struct *part)
+{
+	sector_t nr_sects;
+	unsigned seq;
+
+	do {
+		seq = read_seqcount_begin(&part->seq);
+		nr_sects = part->nr_sects;
+	} while (read_seqcount_retry(&part->seq, seq));
+
+	return nr_sects;
+}
+#else
+static inline void part_nr_sects_write_begin(seqcount_t *seq) {}
+static inline void part_nr_sects_write_end(seqcount_t *seq) {}
+sector_t part_nr_sects_read(struct hd_struct *part)
+{
+	return part->nr_sects;
+}
+#endif
+
 static ssize_t whole_disk_show(struct device *dev,
 			       struct device_attribute *attr, char *buf)
 {

index faf8a45..a851944 100644
@@ -40,6 +40,7 @@ struct blkpg_ioctl_arg {
 /* The subfunctions (for the op field) */
 #define BLKPG_ADD_PARTITION	1
 #define BLKPG_DEL_PARTITION	2
+#define BLKPG_RESIZE_PARTITION	3
 
 /* Sizes of name fields. Unused at present. */
 #define BLKPG_DEVNAMELTH	64

index 6d18f35..a55ce2c 100644
@@ -98,7 +98,13 @@ struct partition_meta_info {
 
 struct hd_struct {
 	sector_t start_sect;
+	/*
+	 * nr_sects is protected by sequence counter. One might extend a
+	 * partition while IO is happening to it and update of nr_sects
+	 * can be non-atomic on 32bit machines with 64bit sector_t.
+	 */
 	sector_t nr_sects;
+	seqcount_t seq;
 	sector_t alignment_offset;
 	unsigned int discard_alignment;
 	struct device __dev;

@@ -604,6 +610,7 @@ extern struct hd_struct * __must_check add_partition(struct gendisk *disk,
 extern void __delete_partition(struct hd_struct *);
 extern void delete_partition(struct gendisk *, int);
 extern void printk_all_partitions(void);
+extern sector_t part_nr_sects_read(struct hd_struct *part);
 
 extern struct gendisk *alloc_disk_node(int minors, int node_id);
 extern struct gendisk *alloc_disk(int minors);

1.7.5.4

Add a new operation to the BLKPG ioctl to read the partition. This allows user space to find the current start and length of a partition without having to open the partition and use the long depreciated HDIO_GETGEO ioctl.

---
 block/ioctl.c         |   15 +++++++++++++++
 include/linux/blkpg.h |    1 +
 2 files changed, 16 insertions(+), 0 deletions(-)

index e4e9198..c84a949 100644
@@ -139,6 +139,21 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
 			disk_put_part(part);
 			mutex_unlock(&bdev->bd_mutex);
 			return 0;
+		case BLKPG_GET_PARTITION:
+			mutex_lock(&bdev->bd_mutex);
+			part = disk_get_part(disk, partno);
+			if (!part)
+			{
+				mutex_unlock(&bdev->bd_mutex);
+				return -ENXIO;
+			}
+			p.start = part->start_sect << 9;
+			p.length = part->nr_sects << 9;
+			disk_put_part(part);
+			mutex_unlock(&bdev->bd_mutex);
+			if (copy_to_user(a.data, &p, sizeof(struct blkpg_partition)))
+				return -EFAULT;
+			return 0;
 		default:
 			return -EINVAL;
 	}

index a851944..72bf5e2 100644
@@ -41,6 +41,7 @@ struct blkpg_ioctl_arg {
 #define BLKPG_ADD_PARTITION	1
 #define BLKPG_DEL_PARTITION	2
 #define BLKPG_RESIZE_PARTITION	3
+#define BLKPG_GET_PARTITION	4
 
 /* Sizes of name fields. Unused at present. */
 #define BLKPG_DEVNAMELTH	64

1.7.5.4

On Mon, Dec 12, 2011 at 07:16:21PM -0500, Phillip Susi wrote:

[..]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Good point, yes, it seems you need to take the partition first. Now that I think about it though, I don't think we need the disk mutex at all. I thought you needed to hold it in order to iterate over the partitions, but it looks like you don't since that uses an RCU, right?

I guess we'll go for another try.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJO8TwsAAoJEJrBOlT6nu75PIwIAJiPhnaRKJokVxzgzg3UQLb+ jPLgzl3a/lD9PcDdg50ol8rYSwqTXUgWjncFwV752nIEsTV/3kGRGfvgkmO25PsS vjkPjL3rubGmYk2n3QPCU5SNIm1iH7CtyTxHcU2e/qUdfccF5jwo38mPd3xYw/IY GEngNnXwmDDoVttHFK+WDuHpxltZb60quIeVtBhdJQxff6npY78G3dNdKnbvwAb6 v97ZNPDMt5vXOCqA7vWYMLZbJMW5FzDCEr3ehTUOaegT3HgXyRa3tpWDnih3/nyT WI1ngR9qou2qpu13FxS4h8GI4zWyPxgd4EOVqD7JlAE5Z6hTne5Z0fo2VJkVK3A= =ZC+O
-----END PGP SIGNATURE-----

Add a new operation code ( BLKPG_RESIZE_PARTITION ) to the BLKPG ioctl that allows altering the size of an existing partition, even if it is currently in use.

Thanks to Vivek Goyal for the seq counter idea.

---
 block/genhd.c         |   10 +++++-----
 block/ioctl.c         |   41 +++++++++++++++++++++++++++++++++++++++--
 fs/partitions/check.c |   39 ++++++++++++++++++++++++++++++++++++++-
 include/linux/blkpg.h |    1 +
 include/linux/genhd.h |    7 +++++++
 5 files changed, 90 insertions(+), 8 deletions(-)

index 02e9fca..b34fb90 100644
@@ -154,7 +154,7 @@ struct hd_struct *disk_part_iter_next(struct disk_part_iter *piter)
 		part = rcu_dereference(ptbl->part[piter->idx]);
 		if (!part)
 			continue;
-		if (!part->nr_sects &&
+		if (!part_nr_sects_read(part) &&
 		    !(piter->flags & DISK_PITER_INCL_EMPTY) &&
 		    !(piter->flags & DISK_PITER_INCL_EMPTY_PART0 &&
 		      piter->idx == 0))

@@ -191,7 +191,7 @@ EXPORT_SYMBOL_GPL(disk_part_iter_exit);
 static inline int sector_in_part(struct hd_struct *part, sector_t sector)
 {
 	return part->start_sect <= sector &&
-		sector < part->start_sect + part->nr_sects;
+		sector < part->start_sect + part_nr_sects_read(part);
 }
 
 /**

@@ -766,8 +766,8 @@ void __init printk_all_partitions(void)
 
 			printk("%s%s %10llu %s %s", is_part0 ? "" : "  ",
 			       bdevt_str(part_devt(part), devt_buf),
-			       (unsigned long long)part->nr_sects >> 1,
-			       disk_name(disk, part->partno, name_buf), uuid);
+			       (unsigned long long)part_nr_sects_read(part) >> 1
+			       , disk_name(disk, part->partno, name_buf), uuid);
 			if (is_part0) {
 				if (disk->driverfs_dev != NULL &&
 				    disk->driverfs_dev->driver != NULL)

@@ -858,7 +858,7 @@ static int show_partition(struct seq_file *seqf, void *v)
 	while ((part = disk_part_iter_next(&piter)))
 		seq_printf(seqf, "%4d  %7d %10llu %s\n",
 			   MAJOR(part_devt(part)), MINOR(part_devt(part)),
-			   (unsigned long long)part->nr_sects >> 1,
+			   (unsigned long long)part_nr_sects_read(part) >> 1,
 			   disk_name(sgp, part->partno, buf));
 	disk_part_iter_exit(&piter);
 

index e3c63d1..089e4bc 100644
@@ -234,7 +234,7 @@ ssize_t part_size_show(struct device *dev,
 		       struct device_attribute *attr, char *buf)
 {
 	struct hd_struct *p = dev_to_part(dev);
-	return sprintf(buf, "%llu\n",(unsigned long long)p->nr_sects);
+	return sprintf(buf, "%llu\n",(unsigned long long)part_nr_sects_read(p));
 }
 
 static ssize_t part_ro_show(struct device *dev,

@@ -408,6 +408,43 @@ void delete_partition(struct gendisk *disk, int partno)
 	hd_struct_put(part);
 }
 
+#if BITS_PER_LONG == 32 && defined(CONFIG_LBDAF)
+static inline void part_nr_sects_write_begin(struct seqcount_t *seq)
+{
+	write_seqcount_begin(&seq);
+}
+
+static inline void part_nr_sects_write_end(struct seqcount_t *seq)
+{
+	write_seqcount_end(&seq);
+}
+
+/*
+ * Any access of part->nr_sects which is not protected by partition
+ * bd_mutex or gendisk bdev bd_mutex, hould be done using this accessor
+ * function.
+ */
+sector_t part_nr_sects_read(struct hd_struct *part)
+{
+	sector_t nr_sects;
+	unsigned seq;
+
+	do {
+		seq = read_seqcount_begin(&part->seq);
+		nr_sects = part->nr_sects;
+	} while (read_seqcount_retry(&part->seq, seq));
+
+	return nr_sects;
+}
+#else
+static inline void part_nr_sects_write_begin(seqcount_t *seq) {}
+static inline void part_nr_sects_write_end(seqcount_t *seq) {}
+sector_t part_nr_sects_read(struct hd_struct *part)
+{
+	return part->nr_sects;
+}
+#endif
+
 static ssize_t whole_disk_show(struct device *dev,
 			       struct device_attribute *attr, char *buf)
 {

index faf8a45..a851944 100644
@@ -40,6 +40,7 @@ struct blkpg_ioctl_arg {
 /* The subfunctions (for the op field) */
 #define BLKPG_ADD_PARTITION	1
 #define BLKPG_DEL_PARTITION	2
+#define BLKPG_RESIZE_PARTITION	3
 
 /* Sizes of name fields. Unused at present. */
 #define BLKPG_DEVNAMELTH	64

index 6d18f35..a55ce2c 100644
@@ -98,7 +98,13 @@ struct partition_meta_info {
 
 struct hd_struct {
 	sector_t start_sect;
+	/*
+	 * nr_sects is protected by sequence counter. One might extend a
+	 * partition while IO is happening to it and update of nr_sects
+	 * can be non-atomic on 32bit machines with 64bit sector_t.
+	 */
 	sector_t nr_sects;
+	seqcount_t seq;
 	sector_t alignment_offset;
 	unsigned int discard_alignment;
 	struct device __dev;

@@ -604,6 +610,7 @@ extern struct hd_struct * __must_check add_partition(struct gendisk *disk,
 extern void __delete_partition(struct hd_struct *);
 extern void delete_partition(struct gendisk *, int);
 extern void printk_all_partitions(void);
+extern sector_t part_nr_sects_read(struct hd_struct *part);
 
 extern struct gendisk *alloc_disk_node(int minors, int node_id);
 extern struct gendisk *alloc_disk(int minors);

1.7.5.4

Add a new operation to the BLKPG ioctl to read the partition. This allows user space to find the current start and length of a partition without having to open the partition and use the long depreciated HDIO_GETGEO ioctl.

---
 block/ioctl.c         |   15 +++++++++++++++
 include/linux/blkpg.h |    1 +
 2 files changed, 16 insertions(+), 0 deletions(-)

index 4a097db..245024a 100644
@@ -129,6 +129,21 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
 			bdput(bdevp);
 			disk_put_part(part);
 			return 0;
+		case BLKPG_GET_PARTITION:
+			mutex_lock(&bdev->bd_mutex);
+			part = disk_get_part(disk, partno);
+			if (!part)
+			{
+				mutex_unlock(&bdev->bd_mutex);
+				return -ENXIO;
+			}
+			p.start = part->start_sect << 9;
+			p.length = part->nr_sects << 9;
+			disk_put_part(part);
+			mutex_unlock(&bdev->bd_mutex);
+			if (copy_to_user(a.data, &p, sizeof(struct blkpg_partition)))
+				return -EFAULT;
+			return 0;
 		default:
 			return -EINVAL;
 	}

index a851944..72bf5e2 100644
@@ -41,6 +41,7 @@ struct blkpg_ioctl_arg {
 #define BLKPG_ADD_PARTITION	1
 #define BLKPG_DEL_PARTITION	2
 #define BLKPG_RESIZE_PARTITION	3
+#define BLKPG_GET_PARTITION	4
 
 /* Sizes of name fields. Unused at present. */
 #define BLKPG_DEVNAMELTH	64

1.7.5.4

I guess taking disk mutex will help us providing mutual exclusion between multiple ioctls or multiple instances of same ioctl. Like, while you are extending a partition and checking for overlap, you don't want another call making progress on next partition which might overlap with this partition.

Thanks
Vivek

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Good point; I will put it back and grab it in the correct order.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJO9kXcAAoJEJrBOlT6nu75NcMH/jC8YXfpB6jnCjKtPxP2Hw8G vRfCULFi3h8WQbRmhR4AvayMJfwTYNkA/QB7TxZ0bYEm8SawesJuVyAKWLe1qutB AFUEFL6d8+lt8vMcpsK1LWFBACEPd2TIN+oMttbaLTm2J+SOotnCdkKk691cIX8g PtxEIeAB3NBBxfJ8hJUee3CQb4E0yg7zefdBEctLUC5Y3bKzAHtD5OvboyWf6Ycj 4+VONmaBwHS10hvXnk1CjLJXgjVs5lG+gKniW29SqdnKFbZ3zGl7Ye+QyUyjH9FU xltg/a+qV5Zx2e3EAi3uTkqaSUgsgo4dCKdCIAmR5+FyhbooJV8xV4d4rdMJvFU= =taVc
-----END PGP SIGNATURE-----

Add a new operation code ( BLKPG_RESIZE_PARTITION ) to the BLKPG ioctl that allows altering the size of an existing partition, even if it is currently in use.

Thanks to Vivek Goyal for the seq counter idea.

---
 block/genhd.c         |   10 ++++----
 block/ioctl.c         |   53 +++++++++++++++++++++++++++++++++++++++++++++++-
 fs/partitions/check.c |   39 +++++++++++++++++++++++++++++++++++-
 include/linux/blkpg.h |    1 +
 include/linux/genhd.h |    7 ++++++
 5 files changed, 102 insertions(+), 8 deletions(-)

index 02e9fca..b34fb90 100644
@@ -154,7 +154,7 @@ struct hd_struct *disk_part_iter_next(struct disk_part_iter *piter)
 		part = rcu_dereference(ptbl->part[piter->idx]);
 		if (!part)
 			continue;
-		if (!part->nr_sects &&
+		if (!part_nr_sects_read(part) &&
 		    !(piter->flags & DISK_PITER_INCL_EMPTY) &&
 		    !(piter->flags & DISK_PITER_INCL_EMPTY_PART0 &&
 		      piter->idx == 0))

@@ -191,7 +191,7 @@ EXPORT_SYMBOL_GPL(disk_part_iter_exit);
 static inline int sector_in_part(struct hd_struct *part, sector_t sector)
 {
 	return part->start_sect <= sector &&
-		sector < part->start_sect + part->nr_sects;
+		sector < part->start_sect + part_nr_sects_read(part);
 }
 
 /**

@@ -766,8 +766,8 @@ void __init printk_all_partitions(void)
 
 			printk("%s%s %10llu %s %s", is_part0 ? "" : "  ",
 			       bdevt_str(part_devt(part), devt_buf),
-			       (unsigned long long)part->nr_sects >> 1,
-			       disk_name(disk, part->partno, name_buf), uuid);
+			       (unsigned long long)part_nr_sects_read(part) >> 1
+			       , disk_name(disk, part->partno, name_buf), uuid);
 			if (is_part0) {
 				if (disk->driverfs_dev != NULL &&
 				    disk->driverfs_dev->driver != NULL)

@@ -858,7 +858,7 @@ static int show_partition(struct seq_file *seqf, void *v)
 	while ((part = disk_part_iter_next(&piter)))
 		seq_printf(seqf, "%4d  %7d %10llu %s\n",
 			   MAJOR(part_devt(part)), MINOR(part_devt(part)),
-			   (unsigned long long)part->nr_sects >> 1,
+			   (unsigned long long)part_nr_sects_read(part) >> 1,
 			   disk_name(sgp, part->partno, buf));
 	disk_part_iter_exit(&piter);
 

index e3c63d1..089e4bc 100644
@@ -234,7 +234,7 @@ ssize_t part_size_show(struct device *dev,
 		       struct device_attribute *attr, char *buf)
 {
 	struct hd_struct *p = dev_to_part(dev);
-	return sprintf(buf, "%llu\n",(unsigned long long)p->nr_sects);
+	return sprintf(buf, "%llu\n",(unsigned long long)part_nr_sects_read(p));
 }
 
 static ssize_t part_ro_show(struct device *dev,

@@ -408,6 +408,43 @@ void delete_partition(struct gendisk *disk, int partno)
 	hd_struct_put(part);
 }
 
+#if BITS_PER_LONG == 32 && defined(CONFIG_LBDAF)
+static inline void part_nr_sects_write_begin(struct seqcount_t *seq)
+{
+	write_seqcount_begin(&seq);
+}
+
+static inline void part_nr_sects_write_end(struct seqcount_t *seq)
+{
+	write_seqcount_end(&seq);
+}
+
+/*
+ * Any access of part->nr_sects which is not protected by partition
+ * bd_mutex or gendisk bdev bd_mutex, hould be done using this accessor
+ * function.
+ */
+sector_t part_nr_sects_read(struct hd_struct *part)
+{
+	sector_t nr_sects;
+	unsigned seq;
+
+	do {
+		seq = read_seqcount_begin(&part->seq);
+		nr_sects = part->nr_sects;
+	} while (read_seqcount_retry(&part->seq, seq));
+
+	return nr_sects;
+}
+#else
+static inline void part_nr_sects_write_begin(seqcount_t *seq) {}
+static inline void part_nr_sects_write_end(seqcount_t *seq) {}
+sector_t part_nr_sects_read(struct hd_struct *part)
+{
+	return part->nr_sects;
+}
+#endif
+
 static ssize_t whole_disk_show(struct device *dev,
 			       struct device_attribute *attr, char *buf)
 {

index faf8a45..a851944 100644
@@ -40,6 +40,7 @@ struct blkpg_ioctl_arg {
 /* The subfunctions (for the op field) */
 #define BLKPG_ADD_PARTITION	1
 #define BLKPG_DEL_PARTITION	2
+#define BLKPG_RESIZE_PARTITION	3
 
 /* Sizes of name fields. Unused at present. */
 #define BLKPG_DEVNAMELTH	64

index 6d18f35..a55ce2c 100644
@@ -98,7 +98,13 @@ struct partition_meta_info {
 
 struct hd_struct {
 	sector_t start_sect;
+	/*
+	 * nr_sects is protected by sequence counter. One might extend a
+	 * partition while IO is happening to it and update of nr_sects
+	 * can be non-atomic on 32bit machines with 64bit sector_t.
+	 */
 	sector_t nr_sects;
+	seqcount_t seq;
 	sector_t alignment_offset;
 	unsigned int discard_alignment;
 	struct device __dev;

@@ -604,6 +610,7 @@ extern struct hd_struct * __must_check add_partition(struct gendisk *disk,
 extern void __delete_partition(struct hd_struct *);
 extern void delete_partition(struct gendisk *, int);
 extern void printk_all_partitions(void);
+extern sector_t part_nr_sects_read(struct hd_struct *part);
 
 extern struct gendisk *alloc_disk_node(int minors, int node_id);
 extern struct gendisk *alloc_disk(int minors);

1.7.5.4

Add a new operation to the BLKPG ioctl to read the partition. This allows user space to find the current start and length of a partition without having to open the partition and use the long depreciated HDIO_GETGEO ioctl.

---
 block/ioctl.c         |   15 +++++++++++++++
 include/linux/blkpg.h |    1 +
 2 files changed, 16 insertions(+), 0 deletions(-)

index 08b2bd6..fad9064 100644
@@ -141,6 +141,21 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
 			bdput(bdevp);
 			disk_put_part(part);
 			return 0;
+		case BLKPG_GET_PARTITION:
+			mutex_lock(&bdev->bd_mutex);
+			part = disk_get_part(disk, partno);
+			if (!part)
+			{
+				mutex_unlock(&bdev->bd_mutex);
+				return -ENXIO;
+			}
+			p.start = part->start_sect << 9;
+			p.length = part->nr_sects << 9;
+			disk_put_part(part);
+			mutex_unlock(&bdev->bd_mutex);
+			if (copy_to_user(a.data, &p, sizeof(struct blkpg_partition)))
+				return -EFAULT;
+			return 0;
 		default:
 			return -EINVAL;
 	}

index a851944..72bf5e2 100644
@@ -41,6 +41,7 @@ struct blkpg_ioctl_arg {
 #define BLKPG_ADD_PARTITION	1
 #define BLKPG_DEL_PARTITION	2
 #define BLKPG_RESIZE_PARTITION	3
+#define BLKPG_GET_PARTITION	4
 
 /* Sizes of name fields. Unused at present. */
 #define BLKPG_DEVNAMELTH	64

1.7.5.4

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Add a new operation to the BLKPG ioctl to read the partition. This allows user space to find the current start and length of a partition without having to open the partition and use the long depreciated HDIO_GETGEO ioctl.

- ---

 block/ioctl.c         |   15 +++++++++++++++
 include/linux/blkpg.h |    1 +
 2 files changed, 16 insertions(+), 0 deletions(-)

index f97e6a4..3e92f0f 100644

- --- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -139,6 +139,21 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
disk_put_part(part);
mutex_unlock(&bdev->bd_mutex);
return 0;
+ case BLKPG_GET_PARTITION:
+ mutex_lock(&bdev->bd_mutex);
+ part = disk_get_part(disk, partno);
+ if (!part)
+ {
+ mutex_unlock(&bdev->bd_mutex);
+ return -ENXIO;
+ }
+ p.start = part->start_sect << 9;
+ p.length = part->nr_sects << 9;
+ disk_put_part(part);
+ mutex_unlock(&bdev->bd_mutex);
+ if (copy_to_user(a.data, &p, sizeof(struct blkpg_partition)))
+ return -EFAULT;
+ return 0;
default:
return -EINVAL;
}

index 103da38..26b1aef 100644

- --- a/include/linux/blkpg.h
+++ b/include/linux/blkpg.h
@@ -41,6 +41,7 @@ struct blkpg_ioctl_arg {
#define BLKPG_ADD_PARTITION 1
#define BLKPG_DEL_PARTITION 2
#define BLKPG_RES_PARTITION 3
+#define BLKPG_GET_PARTITION 4

/* Sizes of name fields. Unused at present. */
#define BLKPG_DEVNAMELTH 64
- --
1.7.5.4

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk7W80AACgkQJ4UciIs+XuJHtQCgjsMlhoVgn2ZDOv2nJ2pJNM3j 0psAoIDeUgJKw+eCAeleFNAGuuHy+HBO
=mBqW
-----END PGP SIGNATURE-----

 $ cat /sys/block/sda/sda1/{size,start}
 4096000
 2048

...is probably the best way if you want to avoid problems with permissions and deprecated (or all) ioctls :-)

Karel

 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com

Yes, I patched parted to grab the values from sysfs and only use HDIO_GETGEO if that fails, but I thought an ioctl would be preferable ( no need to muck about with appending partition numbers to paths and parsing text and such ), and it seemed an orthogonal operation to the other BLKPG ops.