2003-06-09 14:18:47

by Joe Thornber

[permalink] [raw]
Subject: device-mapper patchset against 2.5.70


Linus,

Here is a small set of patches for device mapper. Please apply.

- Joe


2003-06-09 14:25:43

by Joe Thornber

[permalink] [raw]
Subject: [PATCH 5/7] dm: Fix memory leak in dm_register_target()

Fix memory leak in dm_register_target. [Patrick Caulfield]
--- diff/drivers/md/dm-target.c 2003-06-09 15:05:02.000000000 +0100
+++ source/drivers/md/dm-target.c 2003-06-09 15:05:18.000000000 +0100
@@ -109,9 +109,10 @@
return -ENOMEM;

down_write(&_lock);
- if (__find_target_type(t->name))
+ if (__find_target_type(t->name)) {
+ kfree(ti);
rv = -EEXIST;
- else
+ } else
list_add(&ti->list, &_targets);

up_write(&_lock);

2003-06-09 14:25:44

by Joe Thornber

[permalink] [raw]
Subject: [PATCH 4/7] dm: Lift dm_div_up()

Pull dm_div_up() out of dm-table.c into dm.h
--- diff/drivers/md/dm-table.c 2003-06-09 15:05:08.000000000 +0100
+++ source/drivers/md/dm-table.c 2003-06-09 15:05:13.000000000 +0100
@@ -56,14 +56,6 @@
};

/*
- * Ceiling(n / size)
- */
-static inline unsigned long div_up(unsigned long n, unsigned long size)
-{
- return dm_round_up(n, size) / size;
-}
-
-/*
* Similar to ceiling(log_size(n))
*/
static unsigned int int_log(unsigned long n, unsigned long base)
@@ -71,7 +63,7 @@
int result = 0;

while (n > 1) {
- n = div_up(n, base);
+ n = dm_div_up(n, base);
result++;
}

@@ -664,7 +656,7 @@

/* allocate the space for *all* the indexes */
for (i = t->depth - 2; i >= 0; i--) {
- t->counts[i] = div_up(t->counts[i + 1], CHILDREN_PER_NODE);
+ t->counts[i] = dm_div_up(t->counts[i + 1], CHILDREN_PER_NODE);
total += t->counts[i];
}

@@ -691,7 +683,7 @@
unsigned int leaf_nodes;

/* how many indexes will the btree have ? */
- leaf_nodes = div_up(t->num_targets, KEYS_PER_NODE);
+ leaf_nodes = dm_div_up(t->num_targets, KEYS_PER_NODE);
t->depth = 1 + int_log(leaf_nodes, CHILDREN_PER_NODE);

/* leaf layer has already been set up */
--- diff/drivers/md/dm.h 2003-06-09 15:05:08.000000000 +0100
+++ source/drivers/md/dm.h 2003-06-09 15:05:13.000000000 +0100
@@ -135,6 +135,14 @@
}

/*
+ * Ceiling(n / size)
+ */
+static inline unsigned long dm_div_up(unsigned long n, unsigned long size)
+{
+ return dm_round_up(n, size) / size;
+}
+
+/*
* The device-mapper can be driven through one of two interfaces;
* ioctl or filesystem, depending which patch you have applied.
*/

2003-06-09 14:23:43

by Joe Thornber

[permalink] [raw]
Subject: [PATCH 2/7] dm: signed/unsigned audit

signed/unsigned audit.
--- diff/drivers/md/dm-ioctl.c 2003-06-09 14:18:18.000000000 +0100
+++ source/drivers/md/dm-ioctl.c 2003-06-09 15:05:02.000000000 +0100
@@ -351,7 +351,8 @@

static int populate_table(struct dm_table *table, struct dm_ioctl *args)
{
- int i = 0, r, first = 1;
+ int r, first = 1;
+ unsigned int i = 0;
struct dm_target_spec *spec;
char *params;
void *begin, *end;
@@ -380,7 +381,8 @@
}

r = dm_table_add_target(table, spec->target_type,
- spec->sector_start, spec->length,
+ (sector_t) spec->sector_start,
+ (sector_t) spec->length,
params);
if (r) {
DMWARN("internal error adding target to table");
@@ -558,7 +560,7 @@
int r;
struct dm_table *t;
struct mapped_device *md;
- int minor;
+ unsigned int minor = 0;

r = check_name(param->name);
if (r)
@@ -574,8 +576,8 @@
return r;
}

- minor = (param->flags & DM_PERSISTENT_DEV_FLAG) ?
- minor(to_kdev_t(param->dev)) : -1;
+ if (param->flags & DM_PERSISTENT_DEV_FLAG)
+ minor = minor(to_kdev_t(param->dev));

r = dm_create(minor, t, &md);
if (r) {
@@ -584,7 +586,7 @@
}
dm_table_put(t); /* md will have grabbed its own reference */

- set_disk_ro(dm_disk(md), (param->flags & DM_READONLY_FLAG));
+ set_disk_ro(dm_disk(md), (param->flags & DM_READONLY_FLAG) ? 1 : 0);
r = dm_hash_insert(param->name, *param->uuid ? param->uuid : NULL, md);
dm_put(md);

@@ -595,9 +597,9 @@
* Build up the status struct for each target
*/
static int __status(struct mapped_device *md, struct dm_ioctl *param,
- char *outbuf, int *len)
+ char *outbuf, size_t *len)
{
- int i, num_targets;
+ unsigned int i, num_targets;
struct dm_target_spec *spec;
char *outptr;
status_type_t type;
@@ -657,7 +659,7 @@
static int get_status(struct dm_ioctl *param, struct dm_ioctl *user)
{
struct mapped_device *md;
- int len = 0;
+ size_t len = 0;
int ret;
char *outbuf = NULL;

@@ -738,7 +740,8 @@
*/
static int dep(struct dm_ioctl *param, struct dm_ioctl *user)
{
- int count, r;
+ int r;
+ unsigned int count;
struct mapped_device *md;
struct list_head *tmp;
size_t len = 0;
@@ -889,7 +892,7 @@
}
dm_table_put(t); /* md will have taken its own reference */

- set_disk_ro(dm_disk(md), (param->flags & DM_READONLY_FLAG));
+ set_disk_ro(dm_disk(md), (param->flags & DM_READONLY_FLAG) ? 1 : 0);
dm_put(md);

r = info(param, user);
@@ -945,7 +948,7 @@
* As well as checking the version compatibility this always
* copies the kernel interface version out.
*/
-static int check_version(int cmd, struct dm_ioctl *user)
+static int check_version(unsigned int cmd, struct dm_ioctl *user)
{
uint32_t version[3];
int r = 0;
@@ -1028,7 +1031,8 @@
static int ctl_ioctl(struct inode *inode, struct file *file,
uint command, ulong u)
{
- int r = 0, cmd;
+ int r = 0;
+ unsigned int cmd;
struct dm_ioctl *param;
struct dm_ioctl *user = (struct dm_ioctl *) u;
ioctl_fn fn = NULL;
--- diff/drivers/md/dm-linear.c 2003-05-21 11:49:55.000000000 +0100
+++ source/drivers/md/dm-linear.c 2003-06-09 15:05:02.000000000 +0100
@@ -23,7 +23,7 @@
/*
* Construct a linear mapping: <dev_path> <offset>
*/
-static int linear_ctr(struct dm_target *ti, int argc, char **argv)
+static int linear_ctr(struct dm_target *ti, unsigned int argc, char **argv)
{
struct linear_c *lc;

@@ -76,7 +76,7 @@
}

static int linear_status(struct dm_target *ti, status_type_t type,
- char *result, int maxlen)
+ char *result, unsigned int maxlen)
{
struct linear_c *lc = (struct linear_c *) ti->private;
char b[BDEVNAME_SIZE];
--- diff/drivers/md/dm-stripe.c 2003-05-21 11:49:55.000000000 +0100
+++ source/drivers/md/dm-stripe.c 2003-06-09 15:05:02.000000000 +0100
@@ -30,7 +30,7 @@
struct stripe stripe[0];
};

-static inline struct stripe_c *alloc_context(int stripes)
+static inline struct stripe_c *alloc_context(unsigned int stripes)
{
size_t len;

@@ -47,7 +47,7 @@
* Parse a single <dev> <sector> pair
*/
static int get_stripe(struct dm_target *ti, struct stripe_c *sc,
- int stripe, char **argv)
+ unsigned int stripe, char **argv)
{
sector_t start;

@@ -91,14 +91,15 @@
* Construct a striped mapping.
* <number of stripes> <chunk size (2^^n)> [<dev_path> <offset>]+
*/
-static int stripe_ctr(struct dm_target *ti, int argc, char **argv)
+static int stripe_ctr(struct dm_target *ti, unsigned int argc, char **argv)
{
struct stripe_c *sc;
sector_t width;
uint32_t stripes;
uint32_t chunk_size;
char *end;
- int r, i;
+ int r;
+ unsigned int i;

if (argc < 2) {
ti->error = "dm-stripe: Not enough arguments";
@@ -204,11 +205,11 @@
}

static int stripe_status(struct dm_target *ti,
- status_type_t type, char *result, int maxlen)
+ status_type_t type, char *result, unsigned int maxlen)
{
struct stripe_c *sc = (struct stripe_c *) ti->private;
int offset;
- int i;
+ unsigned int i;
char b[BDEVNAME_SIZE];

switch (type) {
--- diff/drivers/md/dm-table.c 2003-06-09 15:04:57.000000000 +0100
+++ source/drivers/md/dm-table.c 2003-06-09 15:05:02.000000000 +0100
@@ -23,12 +23,12 @@
atomic_t holders;

/* btree table */
- int depth;
- int counts[MAX_DEPTH]; /* in nodes */
+ unsigned int depth;
+ unsigned int counts[MAX_DEPTH]; /* in nodes */
sector_t *index[MAX_DEPTH];

- int num_targets;
- int num_allocated;
+ unsigned int num_targets;
+ unsigned int num_allocated;
sector_t *highs;
struct dm_target *targets;

@@ -110,7 +110,7 @@
/*
* Calculate the index of the child node of the n'th node k'th key.
*/
-static inline int get_child(int n, int k)
+static inline unsigned int get_child(unsigned int n, unsigned int k)
{
return (n * CHILDREN_PER_NODE) + k;
}
@@ -118,7 +118,8 @@
/*
* Return the n'th node of level l from table t.
*/
-static inline sector_t *get_node(struct dm_table *t, int l, int n)
+static inline sector_t *get_node(struct dm_table *t,
+ unsigned int l, unsigned int n)
{
return t->index[l] + (n * KEYS_PER_NODE);
}
@@ -127,7 +128,7 @@
* Return the highest key that you could lookup from the n'th
* node on level l of the btree.
*/
-static sector_t high(struct dm_table *t, int l, int n)
+static sector_t high(struct dm_table *t, unsigned int l, unsigned int n)
{
for (; l < t->depth - 1; l++)
n = get_child(n, CHILDREN_PER_NODE - 1);
@@ -142,15 +143,15 @@
* Fills in a level of the btree based on the highs of the level
* below it.
*/
-static int setup_btree_index(int l, struct dm_table *t)
+static int setup_btree_index(unsigned int l, struct dm_table *t)
{
- int n, k;
+ unsigned int n, k;
sector_t *node;

- for (n = 0; n < t->counts[l]; n++) {
+ for (n = 0U; n < t->counts[l]; n++) {
node = get_node(t, l, n);

- for (k = 0; k < KEYS_PER_NODE; k++)
+ for (k = 0U; k < KEYS_PER_NODE; k++)
node[k] = high(t, l + 1, get_child(n, k));
}

@@ -180,7 +181,7 @@
* highs, and targets are managed as dynamic arrays during a
* table load.
*/
-static int alloc_targets(struct dm_table *t, int num)
+static int alloc_targets(struct dm_table *t, unsigned int num)
{
sector_t *n_highs;
struct dm_target *n_targets;
@@ -248,7 +249,7 @@

void table_destroy(struct dm_table *t)
{
- int i;
+ unsigned int i;

DMWARN("destroying table");

@@ -657,7 +658,8 @@

static int setup_indexes(struct dm_table *t)
{
- int i, total = 0;
+ int i;
+ unsigned int total = 0;
sector_t *indexes;

/* allocate the space for *all* the indexes */
@@ -685,7 +687,8 @@
*/
int dm_table_complete(struct dm_table *t)
{
- int leaf_nodes, r = 0;
+ int r = 0;
+ unsigned int leaf_nodes;

/* how many indexes will the btree have ? */
leaf_nodes = div_up(t->num_targets, KEYS_PER_NODE);
@@ -711,7 +714,7 @@
return t->num_targets ? (t->highs[t->num_targets - 1] + 1) : 0;
}

-struct dm_target *dm_table_get_target(struct dm_table *t, int index)
+struct dm_target *dm_table_get_target(struct dm_table *t, unsigned int index)
{
if (index > t->num_targets)
return NULL;
@@ -724,7 +727,7 @@
*/
struct dm_target *dm_table_find_target(struct dm_table *t, sector_t sector)
{
- int l, n = 0, k = 0;
+ unsigned int l, n = 0, k = 0;
sector_t *node;

for (l = 0; l < t->depth; l++) {
--- diff/drivers/md/dm-target.c 2003-06-09 14:18:18.000000000 +0100
+++ source/drivers/md/dm-target.c 2003-06-09 15:05:02.000000000 +0100
@@ -146,7 +146,7 @@
* io-err: always fails an io, useful for bringing
* up LVs that have holes in them.
*/
-static int io_err_ctr(struct dm_target *ti, int argc, char **args)
+static int io_err_ctr(struct dm_target *ti, unsigned int argc, char **args)
{
return 0;
}
--- diff/drivers/md/dm.c 2003-05-21 11:50:15.000000000 +0100
+++ source/drivers/md/dm.c 2003-06-09 15:05:02.000000000 +0100
@@ -17,8 +17,8 @@
static const char *_name = DM_NAME;
#define MAX_DEVICES 1024

-static int major = 0;
-static int _major = 0;
+static unsigned int major = 0;
+static unsigned int _major = 0;

struct dm_io {
struct mapped_device *md;
@@ -524,7 +524,7 @@
static spinlock_t _minor_lock = SPIN_LOCK_UNLOCKED;
static unsigned long _minor_bits[MAX_DEVICES / BITS_PER_LONG];

-static void free_minor(int minor)
+static void free_minor(unsigned int minor)
{
spin_lock(&_minor_lock);
clear_bit(minor, _minor_bits);
@@ -534,7 +534,7 @@
/*
* See if the device with a specific minor # is free.
*/
-static int specific_minor(int minor)
+static int specific_minor(unsigned int minor)
{
int r = -EBUSY;

@@ -546,21 +546,22 @@

spin_lock(&_minor_lock);
if (!test_and_set_bit(minor, _minor_bits))
- r = minor;
+ r = 0;
spin_unlock(&_minor_lock);

return r;
}

-static int next_free_minor(void)
+static int next_free_minor(unsigned int *minor)
{
- int minor, r = -EBUSY;
+ unsigned int m, r = -EBUSY;

spin_lock(&_minor_lock);
- minor = find_first_zero_bit(_minor_bits, MAX_DEVICES);
- if (minor != MAX_DEVICES) {
- set_bit(minor, _minor_bits);
- r = minor;
+ m = find_first_zero_bit(_minor_bits, MAX_DEVICES);
+ if (m != MAX_DEVICES) {
+ set_bit(m, _minor_bits);
+ *minor = m;
+ r = 0;
}
spin_unlock(&_minor_lock);

@@ -570,8 +571,9 @@
/*
* Allocate and initialise a blank device with a given minor.
*/
-static struct mapped_device *alloc_dev(int minor)
+static struct mapped_device *alloc_dev(unsigned int minor)
{
+ int r;
struct mapped_device *md = kmalloc(sizeof(*md), GFP_KERNEL);

if (!md) {
@@ -580,8 +582,8 @@
}

/* get a minor number for the dev */
- minor = (minor < 0) ? next_free_minor() : specific_minor(minor);
- if (minor < 0) {
+ r = (minor < 0) ? next_free_minor(&minor) : specific_minor(minor);
+ if (r < 0) {
kfree(md);
return NULL;
}
@@ -597,7 +599,7 @@
md->io_pool = mempool_create(MIN_IOS, mempool_alloc_slab,
mempool_free_slab, _io_cache);
if (!md->io_pool) {
- free_minor(md->disk->first_minor);
+ free_minor(minor);
kfree(md);
return NULL;
}
@@ -605,7 +607,7 @@
md->disk = alloc_disk(1);
if (!md->disk) {
mempool_destroy(md->io_pool);
- free_minor(md->disk->first_minor);
+ free_minor(minor);
kfree(md);
return NULL;
}
@@ -661,7 +663,8 @@
/*
* Constructor for a new device.
*/
-int dm_create(int minor, struct dm_table *table, struct mapped_device **result)
+int dm_create(unsigned int minor, struct dm_table *table,
+ struct mapped_device **result)
{
int r;
struct mapped_device *md;
--- diff/drivers/md/dm.h 2002-12-30 10:17:13.000000000 +0000
+++ source/drivers/md/dm.h 2003-06-09 15:05:02.000000000 +0100
@@ -51,7 +51,8 @@
* Functions for manipulating a struct mapped_device.
* Drop the reference with dm_put when you finish with the object.
*---------------------------------------------------------------*/
-int dm_create(int minor, struct dm_table *table, struct mapped_device **md);
+int dm_create(unsigned int minor, struct dm_table *table,
+ struct mapped_device **md);

/*
* Reference counting for md.
@@ -96,7 +97,7 @@
int dm_table_complete(struct dm_table *t);
void dm_table_event(struct dm_table *t);
sector_t dm_table_get_size(struct dm_table *t);
-struct dm_target *dm_table_get_target(struct dm_table *t, int index);
+struct dm_target *dm_table_get_target(struct dm_table *t, unsigned int index);
struct dm_target *dm_table_find_target(struct dm_table *t, sector_t sector);
void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q);
unsigned int dm_table_get_num_targets(struct dm_table *t);
--- diff/include/linux/device-mapper.h 2002-12-30 10:17:13.000000000 +0000
+++ source/include/linux/device-mapper.h 2003-06-09 15:05:02.000000000 +0100
@@ -17,7 +17,8 @@
* In the constructor the target parameter will already have the
* table, type, begin and len fields filled in.
*/
-typedef int (*dm_ctr_fn) (struct dm_target *target, int argc, char **argv);
+typedef int (*dm_ctr_fn) (struct dm_target *target,
+ unsigned int argc, char **argv);

/*
* The destructor doesn't need to free the dm_target, just
@@ -33,7 +34,7 @@
*/
typedef int (*dm_map_fn) (struct dm_target *ti, struct bio *bio);
typedef int (*dm_status_fn) (struct dm_target *ti, status_type_t status_type,
- char *result, int maxlen);
+ char *result, unsigned int maxlen);

void dm_error(const char *message);

2003-06-09 14:21:11

by Joe Thornber

[permalink] [raw]
Subject: [PATCH 1/7] dm: Replace __HIGH() and __LOW() macros

Replace __HIGH() and __LOW() with max() and min_not_zero().
--- diff/drivers/md/dm-table.c 2003-05-21 11:50:15.000000000 +0100
+++ source/drivers/md/dm-table.c 2003-06-09 15:04:57.000000000 +0100
@@ -78,22 +78,33 @@
return result;
}

-#define __HIGH(l, r) if (*(l) < (r)) *(l) = (r)
-#define __LOW(l, r) if (*(l) == 0 || *(l) > (r)) *(l) = (r)
+/*
+ * Returns the minimum that is _not_ zero, unless both are zero.
+ */
+#define min_not_zero(l, r) (l == 0) ? r : ((r == 0) ? l : min(l, r))

/*
* Combine two io_restrictions, always taking the lower value.
*/
-
static void combine_restrictions_low(struct io_restrictions *lhs,
struct io_restrictions *rhs)
{
- __LOW(&lhs->max_sectors, rhs->max_sectors);
- __LOW(&lhs->max_phys_segments, rhs->max_phys_segments);
- __LOW(&lhs->max_hw_segments, rhs->max_hw_segments);
- __HIGH(&lhs->hardsect_size, rhs->hardsect_size);
- __LOW(&lhs->max_segment_size, rhs->max_segment_size);
- __LOW(&lhs->seg_boundary_mask, rhs->seg_boundary_mask);
+ lhs->max_sectors =
+ min_not_zero(lhs->max_sectors, rhs->max_sectors);
+
+ lhs->max_phys_segments =
+ min_not_zero(lhs->max_phys_segments, rhs->max_phys_segments);
+
+ lhs->max_hw_segments =
+ min_not_zero(lhs->max_hw_segments, rhs->max_hw_segments);
+
+ lhs->hardsect_size = max(lhs->hardsect_size, rhs->hardsect_size);
+
+ lhs->max_segment_size =
+ min_not_zero(lhs->max_segment_size, rhs->max_segment_size);
+
+ lhs->seg_boundary_mask =
+ min_not_zero(lhs->seg_boundary_mask, rhs->seg_boundary_mask);
}

/*
@@ -481,13 +492,31 @@
request_queue_t *q = bdev_get_queue((*result)->bdev);
struct io_restrictions *rs = &ti->limits;

- /* combine the device limits low */
- __LOW(&rs->max_sectors, q->max_sectors);
- __LOW(&rs->max_phys_segments, q->max_phys_segments);
- __LOW(&rs->max_hw_segments, q->max_hw_segments);
- __HIGH(&rs->hardsect_size, q->hardsect_size);
- __LOW(&rs->max_segment_size, q->max_segment_size);
- __LOW(&rs->seg_boundary_mask, q->seg_boundary_mask);
+ /*
+ * Combine the device limits low.
+ *
+ * FIXME: if we move an io_restriction struct
+ * into q this would just be a call to
+ * combine_restrictions_low()
+ */
+ rs->max_sectors =
+ min_not_zero(rs->max_sectors, q->max_sectors);
+
+ rs->max_phys_segments =
+ min_not_zero(rs->max_phys_segments,
+ q->max_phys_segments);
+
+ rs->max_hw_segments =
+ min_not_zero(rs->max_hw_segments, q->max_hw_segments);
+
+ rs->hardsect_size = max(rs->hardsect_size, q->hardsect_size);
+
+ rs->max_segment_size =
+ min_not_zero(rs->max_segment_size, q->max_segment_size);
+
+ rs->seg_boundary_mask =
+ min_not_zero(rs->seg_boundary_mask,
+ q->seg_boundary_mask);
}

return r;

2003-06-09 14:31:39

by Joe Thornber

[permalink] [raw]
Subject: [PATCH 3/7] dm: new suspend/resume target methods

Some targets may perform io of their own volition, eg. a mirror
performing recovery, a cache target pulling in different chunks. We
cannot let them perform this io while the device is suspended. This
patch adds 2 new methods to the target type, which instruct the target
to suspend/resume itself. All targets start in the suspended state,
so should expect an initial resume call. Simple targets do not need
to implement these functions.
--- diff/drivers/md/dm-table.c 2003-06-09 15:05:02.000000000 +0100
+++ source/drivers/md/dm-table.c 2003-06-09 15:05:08.000000000 +0100
@@ -776,6 +776,31 @@
add_wait_queue(&t->eventq, wq);
}

+void dm_table_suspend_targets(struct dm_table *t)
+{
+ int i;
+
+ for (i = 0; i < t->num_targets; i++) {
+ struct dm_target *ti = t->targets + i;
+
+ if (ti->type->suspend)
+ ti->type->suspend(ti);
+ }
+}
+
+void dm_table_resume_targets(struct dm_table *t)
+{
+ int i;
+
+ for (i = 0; i < t->num_targets; i++) {
+ struct dm_target *ti = t->targets + i;
+
+ if (ti->type->resume)
+ ti->type->resume(ti);
+ }
+}
+
+
EXPORT_SYMBOL(dm_get_device);
EXPORT_SYMBOL(dm_put_device);
EXPORT_SYMBOL(dm_table_event);
--- diff/drivers/md/dm.c 2003-06-09 15:05:02.000000000 +0100
+++ source/drivers/md/dm.c 2003-06-09 15:05:08.000000000 +0100
@@ -678,6 +678,7 @@
free_dev(md);
return r;
}
+ dm_table_resume_targets(md->map);

*result = md;
return 0;
@@ -692,6 +693,8 @@
{
if (atomic_dec_and_test(&md->holders)) {
DMWARN("destroying md");
+ if (!test_bit(DMF_SUSPENDED, &md->flags))
+ dm_table_suspend_targets(md->map);
__unbind(md);
free_dev(md);
}
@@ -781,6 +784,7 @@
down_write(&md->lock);
remove_wait_queue(&md->wait, &wait);
set_bit(DMF_SUSPENDED, &md->flags);
+ dm_table_suspend_targets(md->map);
up_write(&md->lock);

return 0;
@@ -797,6 +801,7 @@
return -EINVAL;
}

+ dm_table_resume_targets(md->map);
clear_bit(DMF_SUSPENDED, &md->flags);
clear_bit(DMF_BLOCK_IO, &md->flags);
def = md->deferred;
--- diff/drivers/md/dm.h 2003-06-09 15:05:02.000000000 +0100
+++ source/drivers/md/dm.h 2003-06-09 15:05:08.000000000 +0100
@@ -104,6 +104,8 @@
struct list_head *dm_table_get_devices(struct dm_table *t);
int dm_table_get_mode(struct dm_table *t);
void dm_table_add_wait_queue(struct dm_table *t, wait_queue_t *wq);
+void dm_table_suspend_targets(struct dm_table *t);
+void dm_table_resume_targets(struct dm_table *t);

/*-----------------------------------------------------------------
* A registry of target types.
--- diff/include/linux/device-mapper.h 2003-06-09 15:05:02.000000000 +0100
+++ source/include/linux/device-mapper.h 2003-06-09 15:05:08.000000000 +0100
@@ -33,6 +33,10 @@
* > 0: simple remap complete
*/
typedef int (*dm_map_fn) (struct dm_target *ti, struct bio *bio);
+
+typedef void (*dm_suspend_fn) (struct dm_target *ti);
+typedef void (*dm_resume_fn) (struct dm_target *ti);
+
typedef int (*dm_status_fn) (struct dm_target *ti, status_type_t status_type,
char *result, unsigned int maxlen);

@@ -56,6 +60,8 @@
dm_ctr_fn ctr;
dm_dtr_fn dtr;
dm_map_fn map;
+ dm_suspend_fn suspend;
+ dm_resume_fn resume;
dm_status_fn status;
};

2003-06-09 14:28:12

by Joe Thornber

[permalink] [raw]
Subject: [PATCH 7/7] dm: Remove an old FIXME

Remove an old FIXME.
--- diff/drivers/md/dm.c 2003-06-09 15:05:35.000000000 +0100
+++ source/drivers/md/dm.c 2003-06-09 15:07:03.000000000 +0100
@@ -281,9 +281,6 @@
sector_t offset = sector - ti->begin;
sector_t len = ti->len - offset;

- /* FIXME: obey io_restrictions ! */
-
-
/*
* Does the target need to split even further ?
*/

2003-06-09 14:32:08

by Joe Thornber

[permalink] [raw]
Subject: [PATCH 6/7] dm: Remove some debug messages

Remove some debug messages.
--- diff/drivers/md/dm-table.c 2003-06-09 15:05:13.000000000 +0100
+++ source/drivers/md/dm-table.c 2003-06-09 15:06:29.000000000 +0100
@@ -243,8 +243,6 @@
{
unsigned int i;

- DMWARN("destroying table");
-
/* destroying the table counts as an event */
dm_table_event(t);

--- diff/drivers/md/dm.c 2003-06-09 15:05:08.000000000 +0100
+++ source/drivers/md/dm.c 2003-06-09 15:05:35.000000000 +0100
@@ -588,7 +588,6 @@
return NULL;
}

- DMWARN("allocating minor %d.", minor);
memset(md, 0, sizeof(*md));
init_rwsem(&md->lock);
atomic_set(&md->holders, 1);
@@ -692,7 +691,6 @@
void dm_put(struct mapped_device *md)
{
if (atomic_dec_and_test(&md->holders)) {
- DMWARN("destroying md");
if (!test_bit(DMF_SUSPENDED, &md->flags))
dm_table_suspend_targets(md->map);
__unbind(md);

2003-06-09 14:50:48

by Kevin Corry

[permalink] [raw]
Subject: Re: [PATCH 2/7] dm: signed/unsigned audit

On Monday 09 June 2003 09:35, Joe Thornber wrote:
> signed/unsigned audit.

> --- diff/drivers/md/dm.c 2003-05-21 11:50:15.000000000 +0100
> +++ source/drivers/md/dm.c 2003-06-09 15:05:02.000000000 +0100
> @@ -546,21 +546,22 @@
>
> spin_lock(&_minor_lock);
> if (!test_and_set_bit(minor, _minor_bits))
> - r = minor;
> + r = 0;
> spin_unlock(&_minor_lock);
>
> return r;
> }
>
> -static int next_free_minor(void)
> +static int next_free_minor(unsigned int *minor)
> {
> - int minor, r = -EBUSY;
> + unsigned int m, r = -EBUSY;

Looks like "r" should still be signed.

> spin_lock(&_minor_lock);
> - minor = find_first_zero_bit(_minor_bits, MAX_DEVICES);
> - if (minor != MAX_DEVICES) {
> - set_bit(minor, _minor_bits);
> - r = minor;
> + m = find_first_zero_bit(_minor_bits, MAX_DEVICES);
> + if (m != MAX_DEVICES) {
> + set_bit(m, _minor_bits);
> + *minor = m;
> + r = 0;
> }
> spin_unlock(&_minor_lock);
>

--
Kevin Corry
[email protected]
http://evms.sourceforge.net/

2003-06-09 15:20:58

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 2/7] dm: signed/unsigned audit


On Mon, 9 Jun 2003, Kevin Corry wrote:
>
> On Monday 09 June 2003 09:35, Joe Thornber wrote:
> > signed/unsigned audit.
>
> > - int minor, r = -EBUSY;
> > + unsigned int m, r = -EBUSY;
>
> Looks like "r" should still be signed.

Yes.

Guys, be _very_ careful if you do audits based on the -Wsigned flag to
gcc, because THE WARNING OUTPUT OF GCC IS OFTEN TOTAL CRAP.

I consider the -Wsigned flag to be more harmful than not, since a
noticeable percentage of the warnings are for code that is perfectly ok,
and rewriting the code to avoid the warning can lead to very subtle bugs
(or just makes the code less readable).

Some of the false warnings could probably be fixed with some fairly
trivial value range analysis, and I'm hopeful that -Wsign can some day
actually be worthwhile, but for now I really wish people be very very
careful.

Linus

2003-06-09 15:20:30

by Joe Thornber

[permalink] [raw]
Subject: Re: [PATCH 2/7 (corrected)] dm: signed/unsigned audit

signed/unsigned audit.
--- diff/drivers/md/dm-ioctl.c 2003-06-09 14:18:18.000000000 +0100
+++ source/drivers/md/dm-ioctl.c 2003-06-09 16:22:51.000000000 +0100
@@ -351,7 +351,8 @@

static int populate_table(struct dm_table *table, struct dm_ioctl *args)
{
- int i = 0, r, first = 1;
+ int r, first = 1;
+ unsigned int i = 0;
struct dm_target_spec *spec;
char *params;
void *begin, *end;
@@ -380,7 +381,8 @@
}

r = dm_table_add_target(table, spec->target_type,
- spec->sector_start, spec->length,
+ (sector_t) spec->sector_start,
+ (sector_t) spec->length,
params);
if (r) {
DMWARN("internal error adding target to table");
@@ -558,7 +560,7 @@
int r;
struct dm_table *t;
struct mapped_device *md;
- int minor;
+ unsigned int minor = 0;

r = check_name(param->name);
if (r)
@@ -574,8 +576,8 @@
return r;
}

- minor = (param->flags & DM_PERSISTENT_DEV_FLAG) ?
- minor(to_kdev_t(param->dev)) : -1;
+ if (param->flags & DM_PERSISTENT_DEV_FLAG)
+ minor = minor(to_kdev_t(param->dev));

r = dm_create(minor, t, &md);
if (r) {
@@ -584,7 +586,7 @@
}
dm_table_put(t); /* md will have grabbed its own reference */

- set_disk_ro(dm_disk(md), (param->flags & DM_READONLY_FLAG));
+ set_disk_ro(dm_disk(md), (param->flags & DM_READONLY_FLAG) ? 1 : 0);
r = dm_hash_insert(param->name, *param->uuid ? param->uuid : NULL, md);
dm_put(md);

@@ -595,9 +597,9 @@
* Build up the status struct for each target
*/
static int __status(struct mapped_device *md, struct dm_ioctl *param,
- char *outbuf, int *len)
+ char *outbuf, size_t *len)
{
- int i, num_targets;
+ unsigned int i, num_targets;
struct dm_target_spec *spec;
char *outptr;
status_type_t type;
@@ -657,7 +659,7 @@
static int get_status(struct dm_ioctl *param, struct dm_ioctl *user)
{
struct mapped_device *md;
- int len = 0;
+ size_t len = 0;
int ret;
char *outbuf = NULL;

@@ -738,7 +740,8 @@
*/
static int dep(struct dm_ioctl *param, struct dm_ioctl *user)
{
- int count, r;
+ int r;
+ unsigned int count;
struct mapped_device *md;
struct list_head *tmp;
size_t len = 0;
@@ -889,7 +892,7 @@
}
dm_table_put(t); /* md will have taken its own reference */

- set_disk_ro(dm_disk(md), (param->flags & DM_READONLY_FLAG));
+ set_disk_ro(dm_disk(md), (param->flags & DM_READONLY_FLAG) ? 1 : 0);
dm_put(md);

r = info(param, user);
@@ -945,7 +948,7 @@
* As well as checking the version compatibility this always
* copies the kernel interface version out.
*/
-static int check_version(int cmd, struct dm_ioctl *user)
+static int check_version(unsigned int cmd, struct dm_ioctl *user)
{
uint32_t version[3];
int r = 0;
@@ -1028,7 +1031,8 @@
static int ctl_ioctl(struct inode *inode, struct file *file,
uint command, ulong u)
{
- int r = 0, cmd;
+ int r = 0;
+ unsigned int cmd;
struct dm_ioctl *param;
struct dm_ioctl *user = (struct dm_ioctl *) u;
ioctl_fn fn = NULL;
--- diff/drivers/md/dm-linear.c 2003-05-21 11:49:55.000000000 +0100
+++ source/drivers/md/dm-linear.c 2003-06-09 16:22:51.000000000 +0100
@@ -23,7 +23,7 @@
/*
* Construct a linear mapping: <dev_path> <offset>
*/
-static int linear_ctr(struct dm_target *ti, int argc, char **argv)
+static int linear_ctr(struct dm_target *ti, unsigned int argc, char **argv)
{
struct linear_c *lc;

@@ -76,7 +76,7 @@
}

static int linear_status(struct dm_target *ti, status_type_t type,
- char *result, int maxlen)
+ char *result, unsigned int maxlen)
{
struct linear_c *lc = (struct linear_c *) ti->private;
char b[BDEVNAME_SIZE];
--- diff/drivers/md/dm-stripe.c 2003-05-21 11:49:55.000000000 +0100
+++ source/drivers/md/dm-stripe.c 2003-06-09 16:22:51.000000000 +0100
@@ -30,7 +30,7 @@
struct stripe stripe[0];
};

-static inline struct stripe_c *alloc_context(int stripes)
+static inline struct stripe_c *alloc_context(unsigned int stripes)
{
size_t len;

@@ -47,7 +47,7 @@
* Parse a single <dev> <sector> pair
*/
static int get_stripe(struct dm_target *ti, struct stripe_c *sc,
- int stripe, char **argv)
+ unsigned int stripe, char **argv)
{
sector_t start;

@@ -91,14 +91,15 @@
* Construct a striped mapping.
* <number of stripes> <chunk size (2^^n)> [<dev_path> <offset>]+
*/
-static int stripe_ctr(struct dm_target *ti, int argc, char **argv)
+static int stripe_ctr(struct dm_target *ti, unsigned int argc, char **argv)
{
struct stripe_c *sc;
sector_t width;
uint32_t stripes;
uint32_t chunk_size;
char *end;
- int r, i;
+ int r;
+ unsigned int i;

if (argc < 2) {
ti->error = "dm-stripe: Not enough arguments";
@@ -204,11 +205,11 @@
}

static int stripe_status(struct dm_target *ti,
- status_type_t type, char *result, int maxlen)
+ status_type_t type, char *result, unsigned int maxlen)
{
struct stripe_c *sc = (struct stripe_c *) ti->private;
int offset;
- int i;
+ unsigned int i;
char b[BDEVNAME_SIZE];

switch (type) {
--- diff/drivers/md/dm-table.c 2003-06-09 16:22:30.000000000 +0100
+++ source/drivers/md/dm-table.c 2003-06-09 16:22:51.000000000 +0100
@@ -23,12 +23,12 @@
atomic_t holders;

/* btree table */
- int depth;
- int counts[MAX_DEPTH]; /* in nodes */
+ unsigned int depth;
+ unsigned int counts[MAX_DEPTH]; /* in nodes */
sector_t *index[MAX_DEPTH];

- int num_targets;
- int num_allocated;
+ unsigned int num_targets;
+ unsigned int num_allocated;
sector_t *highs;
struct dm_target *targets;

@@ -110,7 +110,7 @@
/*
* Calculate the index of the child node of the n'th node k'th key.
*/
-static inline int get_child(int n, int k)
+static inline unsigned int get_child(unsigned int n, unsigned int k)
{
return (n * CHILDREN_PER_NODE) + k;
}
@@ -118,7 +118,8 @@
/*
* Return the n'th node of level l from table t.
*/
-static inline sector_t *get_node(struct dm_table *t, int l, int n)
+static inline sector_t *get_node(struct dm_table *t,
+ unsigned int l, unsigned int n)
{
return t->index[l] + (n * KEYS_PER_NODE);
}
@@ -127,7 +128,7 @@
* Return the highest key that you could lookup from the n'th
* node on level l of the btree.
*/
-static sector_t high(struct dm_table *t, int l, int n)
+static sector_t high(struct dm_table *t, unsigned int l, unsigned int n)
{
for (; l < t->depth - 1; l++)
n = get_child(n, CHILDREN_PER_NODE - 1);
@@ -142,15 +143,15 @@
* Fills in a level of the btree based on the highs of the level
* below it.
*/
-static int setup_btree_index(int l, struct dm_table *t)
+static int setup_btree_index(unsigned int l, struct dm_table *t)
{
- int n, k;
+ unsigned int n, k;
sector_t *node;

- for (n = 0; n < t->counts[l]; n++) {
+ for (n = 0U; n < t->counts[l]; n++) {
node = get_node(t, l, n);

- for (k = 0; k < KEYS_PER_NODE; k++)
+ for (k = 0U; k < KEYS_PER_NODE; k++)
node[k] = high(t, l + 1, get_child(n, k));
}

@@ -180,7 +181,7 @@
* highs, and targets are managed as dynamic arrays during a
* table load.
*/
-static int alloc_targets(struct dm_table *t, int num)
+static int alloc_targets(struct dm_table *t, unsigned int num)
{
sector_t *n_highs;
struct dm_target *n_targets;
@@ -248,7 +249,7 @@

void table_destroy(struct dm_table *t)
{
- int i;
+ unsigned int i;

DMWARN("destroying table");

@@ -657,7 +658,8 @@

static int setup_indexes(struct dm_table *t)
{
- int i, total = 0;
+ int i;
+ unsigned int total = 0;
sector_t *indexes;

/* allocate the space for *all* the indexes */
@@ -685,7 +687,8 @@
*/
int dm_table_complete(struct dm_table *t)
{
- int leaf_nodes, r = 0;
+ int r = 0;
+ unsigned int leaf_nodes;

/* how many indexes will the btree have ? */
leaf_nodes = div_up(t->num_targets, KEYS_PER_NODE);
@@ -711,7 +714,7 @@
return t->num_targets ? (t->highs[t->num_targets - 1] + 1) : 0;
}

-struct dm_target *dm_table_get_target(struct dm_table *t, int index)
+struct dm_target *dm_table_get_target(struct dm_table *t, unsigned int index)
{
if (index > t->num_targets)
return NULL;
@@ -724,7 +727,7 @@
*/
struct dm_target *dm_table_find_target(struct dm_table *t, sector_t sector)
{
- int l, n = 0, k = 0;
+ unsigned int l, n = 0, k = 0;
sector_t *node;

for (l = 0; l < t->depth; l++) {
--- diff/drivers/md/dm-target.c 2003-06-09 14:18:18.000000000 +0100
+++ source/drivers/md/dm-target.c 2003-06-09 16:22:51.000000000 +0100
@@ -146,7 +146,7 @@
* io-err: always fails an io, useful for bringing
* up LVs that have holes in them.
*/
-static int io_err_ctr(struct dm_target *ti, int argc, char **args)
+static int io_err_ctr(struct dm_target *ti, unsigned int argc, char **args)
{
return 0;
}
--- diff/drivers/md/dm.c 2003-05-21 11:50:15.000000000 +0100
+++ source/drivers/md/dm.c 2003-06-09 16:23:34.000000000 +0100
@@ -17,8 +17,8 @@
static const char *_name = DM_NAME;
#define MAX_DEVICES 1024

-static int major = 0;
-static int _major = 0;
+static unsigned int major = 0;
+static unsigned int _major = 0;

struct dm_io {
struct mapped_device *md;
@@ -524,7 +524,7 @@
static spinlock_t _minor_lock = SPIN_LOCK_UNLOCKED;
static unsigned long _minor_bits[MAX_DEVICES / BITS_PER_LONG];

-static void free_minor(int minor)
+static void free_minor(unsigned int minor)
{
spin_lock(&_minor_lock);
clear_bit(minor, _minor_bits);
@@ -534,7 +534,7 @@
/*
* See if the device with a specific minor # is free.
*/
-static int specific_minor(int minor)
+static int specific_minor(unsigned int minor)
{
int r = -EBUSY;

@@ -546,21 +546,23 @@

spin_lock(&_minor_lock);
if (!test_and_set_bit(minor, _minor_bits))
- r = minor;
+ r = 0;
spin_unlock(&_minor_lock);

return r;
}

-static int next_free_minor(void)
+static int next_free_minor(unsigned int *minor)
{
- int minor, r = -EBUSY;
+ int r = -EBUSY;
+ unsigned int m;

spin_lock(&_minor_lock);
- minor = find_first_zero_bit(_minor_bits, MAX_DEVICES);
- if (minor != MAX_DEVICES) {
- set_bit(minor, _minor_bits);
- r = minor;
+ m = find_first_zero_bit(_minor_bits, MAX_DEVICES);
+ if (m != MAX_DEVICES) {
+ set_bit(m, _minor_bits);
+ *minor = m;
+ r = 0;
}
spin_unlock(&_minor_lock);

@@ -570,8 +572,9 @@
/*
* Allocate and initialise a blank device with a given minor.
*/
-static struct mapped_device *alloc_dev(int minor)
+static struct mapped_device *alloc_dev(unsigned int minor)
{
+ int r;
struct mapped_device *md = kmalloc(sizeof(*md), GFP_KERNEL);

if (!md) {
@@ -580,8 +583,8 @@
}

/* get a minor number for the dev */
- minor = (minor < 0) ? next_free_minor() : specific_minor(minor);
- if (minor < 0) {
+ r = (minor < 0) ? next_free_minor(&minor) : specific_minor(minor);
+ if (r < 0) {
kfree(md);
return NULL;
}
@@ -597,7 +600,7 @@
md->io_pool = mempool_create(MIN_IOS, mempool_alloc_slab,
mempool_free_slab, _io_cache);
if (!md->io_pool) {
- free_minor(md->disk->first_minor);
+ free_minor(minor);
kfree(md);
return NULL;
}
@@ -605,7 +608,7 @@
md->disk = alloc_disk(1);
if (!md->disk) {
mempool_destroy(md->io_pool);
- free_minor(md->disk->first_minor);
+ free_minor(minor);
kfree(md);
return NULL;
}
@@ -661,7 +664,8 @@
/*
* Constructor for a new device.
*/
-int dm_create(int minor, struct dm_table *table, struct mapped_device **result)
+int dm_create(unsigned int minor, struct dm_table *table,
+ struct mapped_device **result)
{
int r;
struct mapped_device *md;
--- diff/drivers/md/dm.h 2002-12-30 10:17:13.000000000 +0000
+++ source/drivers/md/dm.h 2003-06-09 16:22:51.000000000 +0100
@@ -51,7 +51,8 @@
* Functions for manipulating a struct mapped_device.
* Drop the reference with dm_put when you finish with the object.
*---------------------------------------------------------------*/
-int dm_create(int minor, struct dm_table *table, struct mapped_device **md);
+int dm_create(unsigned int minor, struct dm_table *table,
+ struct mapped_device **md);

/*
* Reference counting for md.
@@ -96,7 +97,7 @@
int dm_table_complete(struct dm_table *t);
void dm_table_event(struct dm_table *t);
sector_t dm_table_get_size(struct dm_table *t);
-struct dm_target *dm_table_get_target(struct dm_table *t, int index);
+struct dm_target *dm_table_get_target(struct dm_table *t, unsigned int index);
struct dm_target *dm_table_find_target(struct dm_table *t, sector_t sector);
void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q);
unsigned int dm_table_get_num_targets(struct dm_table *t);
--- diff/include/linux/device-mapper.h 2002-12-30 10:17:13.000000000 +0000
+++ source/include/linux/device-mapper.h 2003-06-09 16:22:51.000000000 +0100
@@ -17,7 +17,8 @@
* In the constructor the target parameter will already have the
* table, type, begin and len fields filled in.
*/
-typedef int (*dm_ctr_fn) (struct dm_target *target, int argc, char **argv);
+typedef int (*dm_ctr_fn) (struct dm_target *target,
+ unsigned int argc, char **argv);

/*
* The destructor doesn't need to free the dm_target, just
@@ -33,7 +34,7 @@
*/
typedef int (*dm_map_fn) (struct dm_target *ti, struct bio *bio);
typedef int (*dm_status_fn) (struct dm_target *ti, status_type_t status_type,
- char *result, int maxlen);
+ char *result, unsigned int maxlen);

void dm_error(const char *message);

2003-06-17 22:56:17

by Adrian Bunk

[permalink] [raw]
Subject: Re: [PATCH 1/7] dm: Replace __HIGH() and __LOW() macros

On Mon, Jun 09, 2003 at 03:34:40PM +0100, Joe Thornber wrote:
> Replace __HIGH() and __LOW() with max() and min_not_zero().
> --- diff/drivers/md/dm-table.c 2003-05-21 11:50:15.000000000 +0100
> +++ source/drivers/md/dm-table.c 2003-06-09 15:04:57.000000000 +0100
> @@ -78,22 +78,33 @@
> return result;
> }
>
> -#define __HIGH(l, r) if (*(l) < (r)) *(l) = (r)
> -#define __LOW(l, r) if (*(l) == 0 || *(l) > (r)) *(l) = (r)
> +/*
> + * Returns the minimum that is _not_ zero, unless both are zero.
> + */
> +#define min_not_zero(l, r) (l == 0) ? r : ((r == 0) ? l : min(l, r))
>...

Are there potential other users of min_not_zero?
If yes, shouldn't it go into kernel.h?

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed