[dm-devel] [PATCH 14/14] dm, dm-mpath: Make it easier to detect unintended I/O request flushes
I/O errors triggered by multipathd incorrectly not enabling the
no-flush flag for DM_DEVICE_SUSPEND or DM_DEVICE_RESUME are hard to
debug. Add more logging to make it easier to debug this.
Signed-off-by: Bart Van Assche
---
drivers/md/dm-mpath.c | 25 +
drivers/md/dm.c | 3 +++
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 4a3afec..c3d7e3e 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -489,6 +489,23 @@ static struct pgpath *choose_pgpath(struct multipath *m,
size_t nr_bytes)
}
/*
+ * Note: dm_report_eio() is a macro instead of a function to make pr_debug()
+ * report the function name and line number of the function from which it has
+ * been invoked.
+ */
+#define dm_report_eio(m) \
+({ \
+ struct mapped_device *md = dm_table_get_md((m)->ti->table); \
+ \
+ pr_debug("%s: returning EIO; QIFNP = %d; SQIFNP = %d; DNFS = %d\n", \
+dm_device_name(md),\
+test_bit(MPATHF_QUEUE_IF_NO_PATH, &(m)->flags),\
+test_bit(MPATHF_SAVED_QUEUE_IF_NO_PATH, &(m)->flags), \
+dm_noflush_suspending((m)->ti)); \
+ -EIO; \
+})
+
+/*
* Map cloned requests (request-based multipath)
*/
static int __multipath_map(struct dm_target *ti, struct request *clone,
@@ -510,7 +527,7 @@ static int __multipath_map(struct dm_target *ti, struct
request *clone,
if (!pgpath) {
if (test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags))
return DM_MAPIO_DELAY_REQUEUE;
- return -EIO;/* Failed */
+ return dm_report_eio(m);/* Failed */
} else if (test_bit(MPATHF_QUEUE_IO, &m->flags) ||
test_bit(MPATHF_PG_INIT_REQUIRED, &m->flags)) {
pg_init_all_paths(m);
@@ -612,7 +629,7 @@ static int __multipath_map_bio(struct multipath *m, struct
bio *bio, struct dm_m
if (!pgpath) {
if (test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags))
return DM_MAPIO_REQUEUE;
- return -EIO;
+ return dm_report_eio(m);
}
mpio->pgpath = pgpath;
@@ -1536,7 +1553,7 @@ static int do_end_io(struct multipath *m, struct request
*clone,
if (atomic_read(&m->nr_valid_paths) == 0 &&
!test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags))
- r = -EIO;
+ r = dm_report_eio(m);
return r;
}
@@ -1580,7 +1597,7 @@ static int do_end_io_bio(struct multipath *m, struct bio
*clone,
if (atomic_read(&m->nr_valid_paths) == 0 &&
!test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags))
- return -EIO;
+ return dm_report_eio(m);
/* Queue for the daemon to resubmit */
dm_bio_restore(get_bio_details_from_bio(clone), clone);
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index d19c372..08a21d1 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -2098,6 +2098,9 @@ static int __dm_suspend(struct mapped_device *md, struct
dm_table *map,
*/
if (noflush)
set_bit(DMF_NOFLUSH_SUSPENDING, &md->flags);
+ else
+ pr_debug("%s: suspending without no-flush flag\n",
+dm_device_name(md));
/*
* This gets reverted if there's an error later and the targets
--
2.10.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 12/14] dm-mpath: Micro-optimize the hot path
Instead of checking MPATHF_QUEUE_IF_NO_PATH,
MPATHF_SAVED_QUEUE_IF_NO_PATH and the no_flush flag to decide whether
or not to push back a request if there are no paths available, only
clear MPATHF_QUEUE_IF_NO_PATH in queue_if_no_path() if no_flush has
not been set. The result is that only a single bit has to be tested
in the hot path to decide whether or not a request must be pushed
back and also that m->lock does not have to be taken in the hot path.
Signed-off-by: Bart Van Assche
---
drivers/md/dm-mpath.c | 70 ---
1 file changed, 11 insertions(+), 59 deletions(-)
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 5c73818..fff5d12 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -489,47 +489,6 @@ static struct pgpath *choose_pgpath(struct multipath *m,
size_t nr_bytes)
}
/*
- * Check whether bios must be queued in the device-mapper core rather
- * than here in the target.
- *
- * If m->queue_if_no_path and m->saved_queue_if_no_path hold the
- * same value then we are not between multipath_presuspend()
- * and multipath_resume() calls and we have no need to check
- * for the DMF_NOFLUSH_SUSPENDING flag.
- */
-static bool __must_push_back(struct multipath *m)
-{
- return ((test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags) !=
-test_bit(MPATHF_SAVED_QUEUE_IF_NO_PATH, &m->flags)) &&
- dm_noflush_suspending(m->ti));
-}
-
-static bool must_push_back_rq(struct multipath *m)
-{
- bool r;
- unsigned long flags;
-
- spin_lock_irqsave(&m->lock, flags);
- r = (test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags) ||
-__must_push_back(m));
- spin_unlock_irqrestore(&m->lock, flags);
-
- return r;
-}
-
-static bool must_push_back_bio(struct multipath *m)
-{
- bool r;
- unsigned long flags;
-
- spin_lock_irqsave(&m->lock, flags);
- r = __must_push_back(m);
- spin_unlock_irqrestore(&m->lock, flags);
-
- return r;
-}
-
-/*
* Map cloned requests (request-based multipath)
*/
static int __multipath_map(struct dm_target *ti, struct request *clone,
@@ -549,7 +508,7 @@ static int __multipath_map(struct dm_target *ti, struct
request *clone,
pgpath = choose_pgpath(m, nr_bytes);
if (!pgpath) {
- if (must_push_back_rq(m))
+ if (test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags))
return DM_MAPIO_DELAY_REQUEUE;
return -EIO;/* Failed */
} else if (test_bit(MPATHF_QUEUE_IO, &m->flags) ||
@@ -651,9 +610,9 @@ static int __multipath_map_bio(struct multipath *m, struct
bio *bio, struct dm_m
}
if (!pgpath) {
- if (!must_push_back_bio(m))
- return -EIO;
- return DM_MAPIO_REQUEUE;
+ if (test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags))
+ return DM_MAPIO_REQUEUE;
+ return -EIO;
}
mpio->pgpath = pgpath;
@@ -745,7 +704,7 @@ static int queue_if_no_path(struct multipath *m, bool
queue_if_no_path,
else
clear_bit(MPATHF_SAVED_QUEUE_IF_NO_PATH, &m->flags);
}
- if (queue_if_no_path)
+ if (queue_if_no_path || dm_noflush_suspending(m->ti))
set_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags);
else
clear_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags);
@@ -1578,12 +1537,9 @@ static int do_end_io(struct multipath *m, struct request
*clone,
if (mpio->pgpath)
fail_path(mpio->pgpath);
- if (!atomic_read(&m->nr_valid_paths)) {
- if (!test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags)) {
- if (!must_push_back_rq(m))
- r = -EIO;
- }
- }
+ if (atomic_read(&m->nr_valid_paths) == 0 &&
+ !test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags))
+ r = -EIO;
return r;
}
@@ -1625,13 +1581,9 @@ static int do_end_io_bio(struct multipath *m, struct bio
*clone,
if (mpio->pgpath)
fail_path(mpio->pgpath);
- if (!atomic_read(&m->nr_valid_paths)) {
- if (!test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags)) {
- if (!must_push_back_bio(m))
- return -EIO;
- return DM_ENDIO_REQUEUE;
- }
- }
+ if (atomic_read(&m->nr_valid_paths) == 0 &&
+ !test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags))
+ return -EIO;
/* Queue for the daemon to resubmit */
dm_bio_restore(get_bio_details_from_bio(clone), clone);
--
2.10.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 11/14] dm-mpath: Do not touch *__clone if request allocation fails
Do not modify *__clone if blk_mq_alloc_request() fails. This makes
it easier to figure out what is going on if the caller accidentally
dereferences *__clone if blk_mq_alloc_request() failed.
Signed-off-by: Bart Van Assche
---
drivers/md/dm-mpath.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 1c97f0e..5c73818 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -582,16 +582,17 @@ static int __multipath_map(struct dm_target *ti, struct
request *clone,
* .request_fn stacked on blk-mq path(s) and
* blk-mq stacked on blk-mq path(s).
*/
- *__clone = blk_mq_alloc_request(bdev_get_queue(bdev),
- rq_data_dir(rq),
BLK_MQ_REQ_NOWAIT);
- if (IS_ERR(*__clone)) {
- /* ENOMEM, requeue */
+ clone = blk_mq_alloc_request(bdev_get_queue(bdev),
+ rq_data_dir(rq), BLK_MQ_REQ_NOWAIT);
+ if (IS_ERR(clone)) {
+ /* EBUSY, ENODEV or EWOULDBLOCK; requeue */
clear_request_fn_mpio(m, map_context);
return r;
}
- (*__clone)->bio = (*__clone)->biotail = NULL;
- (*__clone)->rq_disk = bdev->bd_disk;
- (*__clone)->cmd_flags |= REQ_FAILFAST_TRANSPORT;
+ *__clone = clone;
+ clone->bio = clone->biotail = NULL;
+ clone->rq_disk = bdev->bd_disk;
+ clone->cmd_flags |= REQ_FAILFAST_TRANSPORT;
}
if (pgpath->pg->ps.type->start_io)
--
2.10.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 06/14] dm-ioctl: Use offsetof() instead of open-coding it
Subtracting sizes is a fragile approach because the result is only
correct if the compiler has not added any padding at the end of the
structure. Hence use offsetof() instead of size subtraction. An
additional advantage of offsetof() is that it makes the intent more
clear.
Signed-off-by: Bart Van Assche
---
drivers/md/dm-ioctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 966eb4b..c72a770 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1697,7 +1697,7 @@ static int copy_params(struct dm_ioctl __user *user,
struct dm_ioctl *param_kern
{
struct dm_ioctl *dmi;
int secure_data;
- const size_t minimum_data_size = sizeof(*param_kernel) -
sizeof(param_kernel->data);
+ const size_t minimum_data_size = offsetof(struct dm_ioctl, data);
if (copy_from_user(param_kernel, user, minimum_data_size))
return -EFAULT;
--
2.10.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 03/14] dm: Fix a race condition in rq_completed()
It is required to hold the queue lock when calling blk_run_queue_async()
to avoid that a race between blk_run_queue_async() and
blk_cleanup_queue() is triggered.
Signed-off-by: Bart Van Assche
---
drivers/md/dm-rq.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
index f9f37ad..7df7948 100644
--- a/drivers/md/dm-rq.c
+++ b/drivers/md/dm-rq.c
@@ -210,6 +210,9 @@ static void rq_end_stats(struct mapped_device *md, struct
request *orig)
*/
static void rq_completed(struct mapped_device *md, int rw, bool run_queue)
{
+ struct request_queue *q = md->queue;
+ unsigned long flags;
+
atomic_dec(&md->pending[rw]);
/* nudge anyone waiting on suspend queue */
@@ -222,8 +225,11 @@ static void rq_completed(struct mapped_device *md, int rw,
bool run_queue)
* back into ->request_fn() could deadlock attempting to grab the
* queue lock again.
*/
- if (!md->queue->mq_ops && run_queue)
- blk_run_queue_async(md->queue);
+ if (!q->mq_ops && run_queue) {
+ spin_lock_irqsave(q->queue_lock, flags);
+ blk_run_queue_async(q);
+ spin_unlock_irqrestore(q->queue_lock, flags);
+ }
/*
* dm_put() must be at the end of this function. See the comment above
--
2.10.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 13/14] dm-mpath: Introduce assign_bit()
This patch does not change any functionality but makes the code
easier to read.
Signed-off-by: Bart Van Assche
---
drivers/md/dm-mpath.c | 37 -
1 file changed, 16 insertions(+), 21 deletions(-)
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index fff5d12..4a3afec 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -683,6 +683,14 @@ static void process_queued_bios(struct work_struct *work)
blk_finish_plug(&plug);
}
+static void assign_bit(bool value, long nr, unsigned long *addr)
+{
+ if (value)
+ set_bit(nr, addr);
+ else
+ clear_bit(nr, addr);
+}
+
/*
* If we run out of usable paths, should we queue I/O or error it?
*/
@@ -692,23 +700,12 @@ static int queue_if_no_path(struct multipath *m, bool
queue_if_no_path,
unsigned long flags;
spin_lock_irqsave(&m->lock, flags);
-
- if (save_old_value) {
- if (test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags))
- set_bit(MPATHF_SAVED_QUEUE_IF_NO_PATH, &m->flags);
- else
- clear_bit(MPATHF_SAVED_QUEUE_IF_NO_PATH, &m->flags);
- } else {
- if (queue_if_no_path)
- set_bit(MPATHF_SAVED_QUEUE_IF_NO_PATH, &m->flags);
- else
- clear_bit(MPATHF_SAVED_QUEUE_IF_NO_PATH, &m->flags);
- }
- if (queue_if_no_path || dm_noflush_suspending(m->ti))
- set_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags);
- else
- clear_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags);
-
+ assign_bit((save_old_value &&
+ test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags)) ||
+ (!save_old_value && queue_if_no_path),
+ MPATHF_SAVED_QUEUE_IF_NO_PATH, &m->flags);
+ assign_bit(queue_if_no_path || dm_noflush_suspending(m->ti),
+ MPATHF_QUEUE_IF_NO_PATH, &m->flags);
spin_unlock_irqrestore(&m->lock, flags);
if (!queue_if_no_path) {
@@ -1649,10 +1646,8 @@ static void multipath_resume(struct dm_target *ti)
unsigned long flags;
spin_lock_irqsave(&m->lock, flags);
- if (test_bit(MPATHF_SAVED_QUEUE_IF_NO_PATH, &m->flags))
- set_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags);
- else
- clear_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags);
+ assign_bit(test_bit(MPATHF_SAVED_QUEUE_IF_NO_PATH, &m->flags),
+ MPATHF_QUEUE_IF_NO_PATH, &m->flags);
spin_unlock_irqrestore(&m->lock, flags);
}
--
2.10.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 09/14] dm-mpath: Verify 'm->lock' locking assumptions at runtime
Verify at runtime that __pg_init_all_paths() is called with multipath.lock held if lockdep is enabled. Signed-off-by: Bart Van Assche --- drivers/md/dm-mpath.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index e477af8..f1e226d 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -348,6 +348,8 @@ static int __pg_init_all_paths(struct multipath *m) struct pgpath *pgpath; unsigned long pg_init_delay = 0; + lockdep_assert_held(&m->lock); + if (atomic_read(&m->pg_init_in_progress) || test_bit(MPATHF_PG_INIT_DISABLED, &m->flags)) return 0; -- 2.10.1 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 10/14] dm-mpath: Change return type of pg_init_all_paths() from int into void
None of the callers of pg_init_all_paths() check its return value.
Hence change the return type of pg_init_all_paths() from int into
void.
Signed-off-by: Bart Van Assche
---
drivers/md/dm-mpath.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index f1e226d..1c97f0e 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -374,16 +374,13 @@ static int __pg_init_all_paths(struct multipath *m)
return atomic_read(&m->pg_init_in_progress);
}
-static int pg_init_all_paths(struct multipath *m)
+static void pg_init_all_paths(struct multipath *m)
{
- int r;
unsigned long flags;
spin_lock_irqsave(&m->lock, flags);
- r = __pg_init_all_paths(m);
+ __pg_init_all_paths(m);
spin_unlock_irqrestore(&m->lock, flags);
-
- return r;
}
static void __switch_pg(struct multipath *m, struct priority_group *pg)
--
2.10.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 07/14] dm, persistence: Remove an unused argument from dm_block_manager_create()
The 'cache_size' argument of dm_block_manager_create() has never
been used. Hence remove it and also the definitions of the constants
passed as 'cache_size' argument.
Signed-off-by: Bart Van Assche
---
drivers/md/dm-cache-metadata.c| 3 ---
drivers/md/dm-era-target.c| 2 --
drivers/md/dm-thin-metadata.c | 2 --
drivers/md/persistent-data/dm-block-manager.c | 1 -
drivers/md/persistent-data/dm-block-manager.h | 2 +-
5 files changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/md/dm-cache-metadata.c b/drivers/md/dm-cache-metadata.c
index 6955778..d45 100644
--- a/drivers/md/dm-cache-metadata.c
+++ b/drivers/md/dm-cache-metadata.c
@@ -27,8 +27,6 @@
#define MIN_CACHE_VERSION 1
#define MAX_CACHE_VERSION 1
-#define CACHE_METADATA_CACHE_SIZE 64
-
/*
* 3 for btree insert +
* 2 for btree lookup used within space map
@@ -504,7 +502,6 @@ static int __create_persistent_data_objects(struct
dm_cache_metadata *cmd,
{
int r;
cmd->bm = dm_block_manager_create(cmd->bdev,
DM_CACHE_METADATA_BLOCK_SIZE << SECTOR_SHIFT,
- CACHE_METADATA_CACHE_SIZE,
CACHE_MAX_CONCURRENT_LOCKS);
if (IS_ERR(cmd->bm)) {
DMERR("could not create block manager");
diff --git a/drivers/md/dm-era-target.c b/drivers/md/dm-era-target.c
index bf2b267..77f0b4e 100644
--- a/drivers/md/dm-era-target.c
+++ b/drivers/md/dm-era-target.c
@@ -254,7 +254,6 @@ static struct dm_block_validator sb_validator = {
* Low level metadata handling
*--*/
#define DM_ERA_METADATA_BLOCK_SIZE 4096
-#define DM_ERA_METADATA_CACHE_SIZE 64
#define ERA_MAX_CONCURRENT_LOCKS 5
struct era_metadata {
@@ -615,7 +614,6 @@ static int create_persistent_data_objects(struct
era_metadata *md,
int r;
md->bm = dm_block_manager_create(md->bdev, DM_ERA_METADATA_BLOCK_SIZE,
-DM_ERA_METADATA_CACHE_SIZE,
ERA_MAX_CONCURRENT_LOCKS);
if (IS_ERR(md->bm)) {
DMERR("could not create block manager");
diff --git a/drivers/md/dm-thin-metadata.c b/drivers/md/dm-thin-metadata.c
index a15091a..0f0251d 100644
--- a/drivers/md/dm-thin-metadata.c
+++ b/drivers/md/dm-thin-metadata.c
@@ -77,7 +77,6 @@
#define THIN_SUPERBLOCK_MAGIC 27022010
#define THIN_SUPERBLOCK_LOCATION 0
#define THIN_VERSION 2
-#define THIN_METADATA_CACHE_SIZE 64
#define SECTOR_TO_BLOCK_SHIFT 3
/*
@@ -686,7 +685,6 @@ static int __create_persistent_data_objects(struct
dm_pool_metadata *pmd, bool f
int r;
pmd->bm = dm_block_manager_create(pmd->bdev, THIN_METADATA_BLOCK_SIZE
<< SECTOR_SHIFT,
- THIN_METADATA_CACHE_SIZE,
THIN_MAX_CONCURRENT_LOCKS);
if (IS_ERR(pmd->bm)) {
DMERR("could not create block manager");
diff --git a/drivers/md/persistent-data/dm-block-manager.c
b/drivers/md/persistent-data/dm-block-manager.c
index 1e33dd5..4aea053 100644
--- a/drivers/md/persistent-data/dm-block-manager.c
+++ b/drivers/md/persistent-data/dm-block-manager.c
@@ -360,7 +360,6 @@ struct dm_block_manager {
struct dm_block_manager *dm_block_manager_create(struct block_device *bdev,
unsigned block_size,
-unsigned cache_size,
unsigned max_held_per_thread)
{
int r;
diff --git a/drivers/md/persistent-data/dm-block-manager.h
b/drivers/md/persistent-data/dm-block-manager.h
index 3627d1b..e728937 100644
--- a/drivers/md/persistent-data/dm-block-manager.h
+++ b/drivers/md/persistent-data/dm-block-manager.h
@@ -33,7 +33,7 @@ void *dm_block_data(struct dm_block *b);
struct dm_block_manager;
struct dm_block_manager *dm_block_manager_create(
struct block_device *bdev, unsigned block_size,
- unsigned cache_size, unsigned max_held_per_thread);
+ unsigned max_held_per_thread);
void dm_block_manager_destroy(struct dm_block_manager *bm);
unsigned dm_bm_block_size(struct dm_block_manager *bm);
--
2.10.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 08/14] dm, persistence: Remove a dead assignment
A value is assigned to 'nr_entries' but is never used. Hence remove
that variable.
Signed-off-by: Bart Van Assche
---
drivers/md/persistent-data/dm-array.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/md/persistent-data/dm-array.c
b/drivers/md/persistent-data/dm-array.c
index e83047c..7938cd2 100644
--- a/drivers/md/persistent-data/dm-array.c
+++ b/drivers/md/persistent-data/dm-array.c
@@ -700,13 +700,11 @@ static int populate_ablock_with_values(struct
dm_array_info *info, struct array_
{
int r;
unsigned i;
- uint32_t nr_entries;
struct dm_btree_value_type *vt = &info->value_type;
BUG_ON(le32_to_cpu(ab->nr_entries));
BUG_ON(new_nr > le32_to_cpu(ab->max_entries));
- nr_entries = le32_to_cpu(ab->nr_entries);
for (i = 0; i < new_nr; i++) {
r = fn(base + i, element_at(info, ab, i), context);
if (r)
--
2.10.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 05/14] dm: Simplify use_blk_mq initialization
Use a single statement to declare and initialize 'use_blk_mq' instead
of two statements.
Signed-off-by: Bart Van Assche
---
drivers/md/dm-rq.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
index 7df7948..12ed327 100644
--- a/drivers/md/dm-rq.c
+++ b/drivers/md/dm-rq.c
@@ -23,11 +23,7 @@ static unsigned dm_mq_queue_depth = DM_MQ_QUEUE_DEPTH;
#define RESERVED_REQUEST_BASED_IOS 256
static unsigned reserved_rq_based_ios = RESERVED_REQUEST_BASED_IOS;
-#ifdef CONFIG_DM_MQ_DEFAULT
-static bool use_blk_mq = true;
-#else
-static bool use_blk_mq = false;
-#endif
+static bool use_blk_mq = IS_ENABLED(CONFIG_DM_MQ_DEFAULT);
bool dm_use_blk_mq_default(void)
{
--
2.10.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 04/14] dm: Simplify dm_table_determine_type()
Use a single loop instead of two loops to determine whether or not
all_blk_mq has to be set.
Signed-off-by: Bart Van Assche
---
drivers/md/dm-table.c | 21 -
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 49893fdc..991d514 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -871,7 +871,7 @@ static int dm_table_determine_type(struct dm_table *t)
{
unsigned i;
unsigned bio_based = 0, request_based = 0, hybrid = 0;
- bool verify_blk_mq = false;
+ unsigned sq_count = 0, mq_count = 0;
struct dm_target *tgt;
struct dm_dev_internal *dd;
struct list_head *devices = dm_table_get_devices(t);
@@ -959,20 +959,15 @@ static int dm_table_determine_type(struct dm_table *t)
}
if (q->mq_ops)
- verify_blk_mq = true;
+ mq_count++;
+ else
+ sq_count++;
}
-
- if (verify_blk_mq) {
- /* verify _all_ devices in the table are blk-mq devices */
- list_for_each_entry(dd, devices, list)
- if (!bdev_get_queue(dd->dm_dev->bdev)->mq_ops) {
- DMERR("table load rejected: not all devices"
- " are blk-mq request-stackable");
- return -EINVAL;
- }
-
- t->all_blk_mq = true;
+ if (sq_count && mq_count) {
+ DMERR("table load rejected: not all devices are blk-mq
request-stackable");
+ return -EINVAL;
}
+ t->all_blk_mq = mq_count > 0;
return 0;
}
--
2.10.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 01/14] dm: Verify suspend_locking assumptions at runtime
Ensure that the assumptions about the caller holding suspend_lock
are checked at runtime if lockdep is enabled.
Signed-off-by: Bart Van Assche
---
drivers/md/dm-table.c | 4
drivers/md/dm.c | 9 -
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index c4b53b3..49893fdc 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -1654,6 +1654,8 @@ static void suspend_targets(struct dm_table *t, enum
suspend_mode mode)
int i = t->num_targets;
struct dm_target *ti = t->targets;
+ lockdep_assert_held(&t->md->suspend_lock);
+
while (i--) {
switch (mode) {
case PRESUSPEND:
@@ -1701,6 +1703,8 @@ int dm_table_resume_targets(struct dm_table *t)
{
int i, r = 0;
+ lockdep_assert_held(&t->md->suspend_lock);
+
for (i = 0; i < t->num_targets; i++) {
struct dm_target *ti = t->targets + i;
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index ef7bf1d..49c4d00 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1628,11 +1628,10 @@ static void event_callback(void *context)
wake_up(&md->eventq);
}
-/*
- * Protected by md->suspend_lock obtained by dm_swap_table().
- */
static void __set_size(struct mapped_device *md, sector_t size)
{
+ lockdep_assert_held(&md->suspend_lock);
+
set_capacity(md->disk, size);
i_size_write(md->bdev->bd_inode, (loff_t)size << SECTOR_SHIFT);
@@ -2084,8 +2083,6 @@ static void unlock_fs(struct mapped_device *md)
* If __dm_suspend returns 0, the device is completely quiescent
* now. There is no request-processing activity. All new requests
* are being added to md->deferred list.
- *
- * Caller must hold md->suspend_lock
*/
static int __dm_suspend(struct mapped_device *md, struct dm_table *map,
unsigned suspend_flags, long task_state,
@@ -2301,6 +2298,8 @@ static void __dm_internal_suspend(struct mapped_device
*md, unsigned suspend_fla
{
struct dm_table *map = NULL;
+ lockdep_assert_held(&md->suspend_lock);
+
if (md->internal_suspend_count++)
return; /* nested internal suspend */
--
2.10.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 02/14] dm: Use blk_set_queue_dying() in __dm_destroy()
After QUEUE_FLAG_DYING has been set any code that is waiting in get_request() should be woken up. Hence call blk_set_queue_dying() instead of only setting QUEUE_FLAG_DYING. Signed-off-by: Bart Van Assche --- drivers/md/dm.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 49c4d00..d19c372 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -1885,9 +1885,7 @@ static void __dm_destroy(struct mapped_device *md, bool wait) set_bit(DMF_FREEING, &md->flags); spin_unlock(&_minor_lock); - spin_lock_irq(q->queue_lock); - queue_flag_set(QUEUE_FLAG_DYING, q); - spin_unlock_irq(q->queue_lock); + blk_set_queue_dying(q); if (dm_request_based(md) && md->kworker_task) kthread_flush_worker(&md->kworker); -- 2.10.1 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] Improve processing efficiency for addition and deletion of multipath devices
My $.02 I'm not sure that we want to wait for a predetermined time to grab the uevents. If they are coming in slowly, multipathd is more responsive by processing them immediately, and if they are coming in a storm, they will naturally pile up faster than we deal with them. I also don't really like the idea of waiting when we get an event to see if another one comes along, for the same reasons. That being said, I wouldn't NACK an config option (turned off by default) that set a minimum wait time between uevent dispatches, because I might be wrong here. However, It sees like what uevent_dispatch() dispatch already does is the right thing, which is to pull all oustanding uevents off the uevent queue into a new list. The issue is that we simply need to work on processing the whole list at a time. Martin's filtering definitely has a place here. He is correct that if we get a delete uevent for a device, we don't need to process any of the earlier ones. We do need to look at both the add and change uevents individually. For instance, one of the change uevents out of a group could be for changing the read-only status of a device. If we just took the most recent, we would miss that information. The good news is that we don't need to actually call uev_add_path and uev_update_path once for each of these events. All we need to do is read through the uev environment variables for all of them to find the information we need (like change in ro status), and then call the function once with that information. Hannes pointed out that for adding paths we don't need to do any locking until we want to add the path to the pathvec. We could grab all the outstanding add events from the list that gets passed to service_uevq, and collect the pathinfo for all the paths, add them all into the pathvec, and then create or update the multipath devices. delete uevents could work similarly, where we find all the paths for a multipath device in our list, remove them all, and reload the device once. I'm not sure how much a separate thread for doing the multipath work would buy us, however. It's true that we could have a seperate lock for the mpvec, so we didn't need to hold the pathvec lock while updating the mpvec, but actually updating the mpvec only takes a moment. The time consuming part is building the multipath device and passing it to the kernel. For this, we do need the the paths locked. Otherwise what would keep a path from getting removed while the multipath device was using it to set get set up. If working with multipath devices and proccessing path uevents is happening at the same time, this is a very real possibility. But even if we keep one thread processing the uevents, simply avoiding calling uev_add_path and uev_update_path for repeated add and change events, and batching multiple add and delete events together could provide a real speedup, IMHO. Now, the holy grail of multipathd efficiency would be to totally rework multipathd's locking into something sensible. We could have locks for the vectors that only got held when you were actually traversing or manipulating the vectors, coupled with reference counts on the individual paths and multipaths, so that they didn't get removed while they were in use, and probably locks on the individual paths and multipaths for just the sections that really needed to be protected. The problem is that this would take a huge amount of code auditting to get mostly right, and then a while to flush out all the missed corner cases. At least I think it would. But I dismissed decoupling the config structure from the vecs lock as too painful, and clearly that was possible. At any rate, I'd rather get rid of the gazillion waiter threads first. -Ben On Thu, Nov 17, 2016 at 11:48:32AM +0100, Martin Wilck wrote: > Hi Tang, > > > As to process several uevents for the same physical devices, I think > > the opinions > > different between us is "FILTER" or "MERGER". Personally, I think > > Merger is more > > accuracy, for example, we receive 4 paths addition uevent messages > > from the same > > physical devices: > > 1)uevent add sdb > > 2)uevent add sdc > > 3)uevent add sdd > > 4)uevent add sde > > > > We cannot just filter the 1)2)3) uevent messages but only process the > > 4)uevent message, > > which would cause losing paths of this multipath devices. > > Of course. My "filtering" idea was meant for cases where several events > for the same device are queued, e.g. > > 1) add sda > 2) change sda > 3) delete sda > > Is it always sufficient to look only at the last event in such a case? > I think so, but I'm not 100% certain. > > Regards > Martin -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH v2 0/14] Device mapper patches for Linux kernel v4.10
Hello Mike, The fourteen patches in this series is what I came up with while reviewing and testing the device mapper kernel code. Compared to version 1 one patch has been left out and several other patches have been added. The commit message of several patches that had already been posted has been updated with the feedback I received on the dm-devel mailing list. It would be appreciated if these patches would be considered for inclusion in the upstream kernel. Thanks, Bart. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] Avoid that reloading a map sporadically triggers I/O errors
On 11/18/2016 01:43 PM, Mike Snitzer wrote: On Fri, Nov 18 2016 at 4:33pm -0500, Bart Van Assche wrote: Avoid that reloading a map while there are no paths triggers a flush and hence unwanted I/O errors if 'queue_if_no_path' is enabled. I assume you meant: "if 'queue_if_no_path' is _not_ enabled." ? No. Only with 'queue_if_no_path' enabled reporting an I/O error because no paths are available is undesired. Bart. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] Avoid that reloading a map sporadically triggers I/O errors
On Fri, Nov 18 2016 at 4:33pm -0500, Bart Van Assche wrote: > Avoid that reloading a map while there are no paths triggers a flush > and hence unwanted I/O errors if 'queue_if_no_path' is enabled. I assume you meant: "if 'queue_if_no_path' is _not_ enabled." ? -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH] Avoid that reloading a map sporadically triggers I/O errors
Avoid that reloading a map while there are no paths triggers a flush
and hence unwanted I/O errors if 'queue_if_no_path' is enabled.
Fixes: commit d569988e7528 ("libmultipath: Fixup 'DM_DEVICE_RELOAD' handling")
Signed-off-by: Bart Van Assche
Cc: Hannes Reinecke
---
libmultipath/devmapper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index f92ebce..31f1962 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -390,7 +390,7 @@ int dm_addmap_reload(struct multipath *mpp, char *params,
int flush)
params, ADDMAP_RO, SKIP_KPARTX_OFF);
}
if (r)
- r = dm_simplecmd(DM_DEVICE_RESUME, mpp->alias, flush,
+ r = dm_simplecmd(DM_DEVICE_RESUME, mpp->alias, !flush,
1, udev_flags, 0);
return r;
}
--
2.10.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH] Remove superfluous instances of the "extern" keyword
This avoids that the sparse static analyzer complains about
the "extern" keyword.
Signed-off-by: Bart Van Assche
---
kpartx/devmapper.c | 18 -
kpartx/lopart.c | 15 +++-
libmpathpersist/mpath_pr_ioctl.c | 2 +-
libmultipath/blacklist.c | 9 ++---
libmultipath/callout.c | 3 +-
libmultipath/checkers/cciss_tur.c| 3 +-
libmultipath/checkers/emc_clariion.c | 3 +-
libmultipath/checkers/hp_sw.c| 3 +-
libmultipath/checkers/rdac.c | 3 +-
libmultipath/checkers/tur.c | 3 +-
libmultipath/config.c| 6 +--
libmultipath/configure.c | 21 +-
libmultipath/devmapper.c | 49 +--
libmultipath/discovery.c | 5 +--
libmultipath/dmparser.c | 8 ++--
libmultipath/hwtable.c | 3 +-
libmultipath/pgpolicies.c| 23 +--
libmultipath/print.c | 55 ++
libmultipath/propsel.c | 75
libmultipath/structs.c | 21 --
libmultipath/structs_vec.c | 49 +--
libmultipath/switchgroup.c | 6 +--
libmultipath/uevent.c| 12 ++
libmultipath/util.c | 3 +-
24 files changed, 145 insertions(+), 253 deletions(-)
diff --git a/kpartx/devmapper.c b/kpartx/devmapper.c
index 474d879..2acae25 100644
--- a/kpartx/devmapper.c
+++ b/kpartx/devmapper.c
@@ -14,8 +14,7 @@
#define MAX_PREFIX_LEN 8
#define PARAMS_SIZE 1024
-extern int
-dm_prereq (char * str, int x, int y, int z)
+int dm_prereq(char * str, int x, int y, int z)
{
int r = 1;
struct dm_task *dmt;
@@ -52,8 +51,8 @@ out:
return r;
}
-extern int
-dm_simplecmd (int task, const char *name, int no_flush, uint16_t udev_flags) {
+int dm_simplecmd(int task, const char *name, int no_flush, uint16_t udev_flags)
+{
int r = 0;
int udev_wait_flag = (task == DM_DEVICE_RESUME ||
task == DM_DEVICE_REMOVE);
@@ -90,10 +89,10 @@ out:
return r;
}
-extern int
-dm_addmap (int task, const char *name, const char *target,
- const char *params, uint64_t size, int ro, const char *uuid, int
part,
- mode_t mode, uid_t uid, gid_t gid) {
+int dm_addmap(int task, const char *name, const char *target,
+ const char *params, uint64_t size, int ro, const char *uuid,
+ int part, mode_t mode, uid_t uid, gid_t gid)
+{
int r = 0;
struct dm_task *dmt;
char *prefixed_uuid = NULL;
@@ -154,8 +153,7 @@ addout:
return r;
}
-extern int
-dm_map_present (char * str, char **uuid)
+int dm_map_present(char * str, char **uuid)
{
int r = 0;
struct dm_task *dmt;
diff --git a/kpartx/lopart.c b/kpartx/lopart.c
index 14af34f..2eb3f63 100644
--- a/kpartx/lopart.c
+++ b/kpartx/lopart.c
@@ -62,8 +62,7 @@ xstrdup (const char *s)
return t;
}
-extern int
-is_loop_device (const char *device)
+int is_loop_device(const char *device)
{
struct stat statbuf;
int loopmajor;
@@ -96,8 +95,7 @@ is_loop_device (const char *device)
#define SIZE(a) (sizeof(a)/sizeof(a[0]))
-extern char *
-find_loop_by_file (const char * filename)
+char *find_loop_by_file(const char *filename)
{
DIR *dir;
struct dirent *dent;
@@ -144,8 +142,7 @@ find_loop_by_file (const char * filename)
return found;
}
-extern char *
-find_unused_loop_device (void)
+char *find_unused_loop_device(void)
{
char dev[20], *next_loop_dev = NULL;
int fd, next_loop = 0, somedev = 0, someloop = 0, loop_known = 0;
@@ -231,8 +228,7 @@ find_unused_loop_device (void)
return NULL;
}
-extern int
-set_loop (const char *device, const char *file, int offset, int *loopro)
+int set_loop(const char *device, const char *file, int offset, int *loopro)
{
struct loop_info loopinfo;
int fd, ffd, mode;
@@ -284,8 +280,7 @@ set_loop (const char *device, const char *file, int offset,
int *loopro)
return 0;
}
-extern int
-del_loop (const char *device)
+int del_loop(const char *device)
{
int retries = 5;
int fd;
diff --git a/libmpathpersist/mpath_pr_ioctl.c b/libmpathpersist/mpath_pr_ioctl.c
index 8b9ac3d..31b2fe6 100644
--- a/libmpathpersist/mpath_pr_ioctl.c
+++ b/libmpathpersist/mpath_pr_ioctl.c
@@ -36,7 +36,7 @@ void decode_transport_id(struct prin_fulldescr *fdesc,
unsigned char * p, int le
int get_prin_length(int rq_servact);
int mpath_isLittleEndian(void);
-extern unsigned int mpath_mx_alloc_len;
+unsigned int mpath_mx_alloc_len;
int prout_do_scsi_ioctl(char * dev, int rq_servact, int rq_scope,
unsigned int rq_type, struct prout_param_descriptor *paramp,
int noisy)
diff --git a/libmultipath/blacklist.c b/libmultipath/blacklist.c
index f6c4506..d59d37e 100644
---
[dm-devel] [PATCH] Makefiles: Fix function availability checks
The current implementation of the code that checks for function
presence is not correct because it checks for a prefix match only.
Introduce a function that checks for the exact function name.
Additionally, report whether or not the function has been found.
An example of the output produced by this function if 'make' is
run:
Checking for dm_task_no_flush in /usr/include/libdevmapper.h ... yes
Checking for dm_task_set_cookie in /usr/include/libdevmapper.h ... yes
Checking for udev_monitor_set_receive_buffer_size in /usr/include/libudev.h ...
yes
Checking for dm_task_deferred_remove in /usr/include/libdevmapper.h ... yes
Signed-off-by: Bart Van Assche
---
Makefile.inc | 14 ++
kpartx/Makefile | 4 +---
libmultipath/Makefile | 16
3 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/Makefile.inc b/Makefile.inc
index 1cc8f44..e7f4e05 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -69,5 +69,19 @@ OPTFLAGS = -O2 -g -pipe -Wall -Wextra -Wformat=2 \
CFLAGS = $(OPTFLAGS) -fPIC -DLIB_STRING=\"${LIB}\" -DRUN_DIR=\"${RUN}\"
SHARED_FLAGS = -shared
+# Check whether a function with name $1 has been declared in header file $2.
+check_func = \
+$(shell \
+ if grep -Eq "^[^[:blank:]]+[[:blank:]]+$1[[:blank:]]*(.*)*" "$2"; then \
+ found=1;\
+ status="yes"; \
+ else \
+ found=0;\
+ status="no";\
+ fi;\
+ echo 1>&2 "Checking for $1 in $2 ... $$status";\
+ echo "$$found" \
+)
+
%.o: %.c
$(CC) $(CFLAGS) -c -o $@ $<
diff --git a/kpartx/Makefile b/kpartx/Makefile
index e8a59f2..9441a2b 100644
--- a/kpartx/Makefile
+++ b/kpartx/Makefile
@@ -7,9 +7,7 @@ CFLAGS += -I. -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64
LIBDEPS += -ldevmapper
-LIBDM_API_COOKIE = $(shell grep -Ecs '^[a-z]*[[:space:]]+dm_task_set_cookie'
/usr/include/libdevmapper.h)
-
-ifneq ($(strip $(LIBDM_API_COOKIE)),0)
+ifneq ($(call check_func,dm_task_set_cookie,/usr/include/libdevmapper.h),0)
CFLAGS += -DLIBDM_API_COOKIE
endif
diff --git a/libmultipath/Makefile b/libmultipath/Makefile
index 495cebe..a11e483 100644
--- a/libmultipath/Makefile
+++ b/libmultipath/Makefile
@@ -20,27 +20,19 @@ ifdef SYSTEMD
endif
endif
-LIBDM_API_FLUSH = $(shell grep -Ecs '^[a-z]*[[:space:]]+dm_task_no_flush'
/usr/include/libdevmapper.h)
-
-ifneq ($(strip $(LIBDM_API_FLUSH)),0)
+ifneq ($(call check_func,dm_task_no_flush,/usr/include/libdevmapper.h),0)
CFLAGS += -DLIBDM_API_FLUSH -D_GNU_SOURCE
endif
-LIBDM_API_COOKIE = $(shell grep -Ecs '^[a-z]*[[:space:]]+dm_task_set_cookie'
/usr/include/libdevmapper.h)
-
-ifneq ($(strip $(LIBDM_API_COOKIE)),0)
+ifneq ($(call check_func,dm_task_set_cookie,/usr/include/libdevmapper.h),0)
CFLAGS += -DLIBDM_API_COOKIE
endif
-LIBUDEV_API_RECVBUF = $(shell grep -Ecs
'^[a-z]*[[:space:]]+udev_monitor_set_receive_buffer_size'
/usr/include/libudev.h)
-
-ifneq ($(strip $(LIBUDEV_API_RECVBUF)),0)
+ifneq ($(call
check_func,udev_monitor_set_receive_buffer_size,/usr/include/libudev.h),0)
CFLAGS += -DLIBUDEV_API_RECVBUF
endif
-LIBDM_API_DEFERRED = $(shell grep -Ecs
'^[a-z]*[[:space:]]+dm_task_deferred_remove' /usr/include/libdevmapper.h)
-
-ifneq ($(strip $(LIBDM_API_DEFERRED)),0)
+ifneq ($(call
check_func,dm_task_deferred_remove,/usr/include/libdevmapper.h),0)
CFLAGS += -DLIBDM_API_DEFERRED
endif
--
2.10.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v2] dm-crypt: add ability to use keys from the kernel key retention service
On 11/17/2016 11:06 PM, Ondrej Kozina wrote: > On 11/17/2016 05:35 PM, Andrey Ryabinin wrote: >> On 11/16/2016 11:47 PM, Ondrej Kozina wrote: >>> (Please still consider it to be RFC only, I need to modify the uspace >>> teststuite >>> again due to changes in key_string format. Also the changes to dm-crypt >>> documentation >>> will follow before final submit. Feature wide I'd consider the patch being >>> complete >>> unless any bugs would emerge) >>> >>> The kernel key service is a generic way to store keys for the use of >>> other subsystems. Currently there is no way to use kernel keys in dm-crypt. >>> This patch aims to fix that. Instead of key userspace may pass a key >>> description with preceding ':'. So message that constructs encryption >>> mapping now looks like this: >>> >>>[|:] >>> [<#opt_params> ] >>> >>> where is in format: :: >>> >>> Currently we only support two elementary key types: 'user' and 'logon'. >>> Keys may be loaded in dm-crypt either via or using >>> classical method and pass the key in hex representation directly. >>> >> >> I think we need to hexify key description too, because it can contain spaces. > > I see. You're right the kernel key description may really contain whitespace > chars, bummer. Well what I'm thinking atm is rejecting any keys with > descriptions containing whitespaces. But let me ask Mike or Alasdair what do > they think about it. > >> seems like unnecessary complication. Kernel knows key_size, it >> doesn't need >> that information from userspace. > > Milan already explained that. I just add that general rule for dm tables is > what you input via load ioctl() you should get back exactly the same via > table status ioctl(). And also there's no other way how to get dm-crypt key > size if you input it via kernel keyring key. > Yeah, I get that, but Milan said that we need to *get* that information from the kernel. My concern is about loading key_size into the kernel. If I understand you right, we need it just for consistency between table_load and table_status ioctls(). I guess it's ok then. >> Handling different types is probably an overkill too. If it works with logon >> keys, >> why would we need to use 'user' keys? > > Well your original patch used 'user' type so I assumed you have good reason > to use it. I used 'user' because I wasn't sure if userspace requires ability to read keys or not. > But anyway it's not so painful to add option to choose from 2 different key > types imo. Loading tables is not a hot path. > Ok, fair enough. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] Improve processing efficiency for addition and deletion of multipath devices
Hi Martin, 〉But once you start merging, you'd rather be prepared for 〉several events for the same phys device, too. We can base on such a threshold that there is no repeat uevents from the same sd device, otherwise, we pause doing merger, and kick uevent processing thread to process the merged uevents. Regards, Tang 发件人: Martin Wilck 收件人: tang.jun...@zte.com.cn, 抄送: dm-devel@redhat.com 日期: 2016/11/18 16:38 主题: Re: [dm-devel] Improve processing efficiency for addition and deletion of multipath devices 发件人: dm-devel-boun...@redhat.com On Fri, 2016-11-18 at 16:24 +0800, tang.jun...@zte.com.cn wrote: > Hi Martin, > > In your case, my action is: > 1) merger uevents 1) 2) to one uevent "add sda sdb", and process them > togother This will fail because sdb is non-existent at the time you try - no? > Though the processing efficiency in such scenario is lower than > yours, but it is simple and reliable, > more importantly, Martin, you still focus on such special scene, > which I concerned is like this: [...] I understand what you're concerned with. I just think we need to do both. I agree that many events for many different devices are more likely. But once you start merging, you'd rather be prepared for several events for the same phys device, too. Martin -- Dr. Martin Wilck , Tel. +49 (0)911 74053 2107 SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] Improve processing efficiency for addition and deletion of multipath devices
Hi Martin, > This will fail because sdb is non-existent at the time you try - no? Non-existent paths in merger uevents do not affect the entire algorithm. It should be considered at the time of implementation. Your suggestion is meaningful, but I think we need to focus on the existed problem, resolve it, and then do more work to make it better in other areas. is not it? Regards,Tang 发件人: Martin Wilck 收件人: tang.jun...@zte.com.cn, 抄送: dm-devel@redhat.com 日期: 2016/11/18 16:31 主题: Re: Re: Re: [dm-devel] Improve processing efficiency for addition and deletion of multipath devices On Fri, 2016-11-18 at 16:24 +0800, tang.jun...@zte.com.cn wrote: > Hi Martin, > > In your case, my action is: > 1) merger uevents 1) 2) to one uevent "add sda sdb", and process them > togother This will fail because sdb is non-existent at the time you try - no? > Though the processing efficiency in such scenario is lower than > yours, but it is simple and reliable, > more importantly, Martin, you still focus on such special scene, > which I concerned is like this: [...] I understand what you're concerned with. I just think we need to do both. I agree that many events for many different devices are more likely. But once you start merging, you'd rather be prepared for several events for the same phys device, too. Martin -- Dr. Martin Wilck , Tel. +49 (0)911 74053 2107 SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] Improve processing efficiency for addition and deletion of multipath devices
On Fri, 2016-11-18 at 16:24 +0800, tang.jun...@zte.com.cn wrote: > Hi Martin, > > In your case, my action is: > 1) merger uevents 1) 2) to one uevent "add sda sdb", and process them > togother This will fail because sdb is non-existent at the time you try - no? > Though the processing efficiency in such scenario is lower than > yours, but it is simple and reliable, > more importantly, Martin, you still focus on such special scene, > which I concerned is like this: [...] I understand what you're concerned with. I just think we need to do both. I agree that many events for many different devices are more likely. But once you start merging, you'd rather be prepared for several events for the same phys device, too. Martin -- Dr. Martin Wilck , Tel. +49 (0)911 74053 2107 SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] Improve processing efficiency for addition and deletion of multipath devices
Hi Martin, In your case, my action is: 1) merger uevents 1) 2) to one uevent "add sda sdb", and process them togother 2) process uevent "del sda" 3) process uevent "add sdc" 4) process uevent "del sdb" 5) process uevent "add sda" Though the processing efficiency in such scenario is lower than yours, but it is simple and reliable, more importantly, Martin, you still focus on such special scene, which I concerned is like this: UDEV [19172.014482] add /devices/platform/host3/session17/target3:0:0/3:0:0:0/block/sdc (block) UDEV [19172.249389] add /devices/platform/host4/session18/target4:0:0/4:0:0:0/block/sdd (block) UDEV [19172.343791] add /devices/platform/host3/session17/target3:0:0/3:0:0:2/block/sdf (block) UDEV [19172.364496] add /devices/platform/host5/session19/target5:0:0/5:0:0:0/block/sdh (block) UDEV [19172.523794] add /devices/platform/host4/session18/target4:0:0/4:0:0:2/block/sdi (block) UDEV [19172.621333] add /devices/platform/host3/session17/target3:0:0/3:0:0:4/block/sdn (block) UDEV [19172.699473] add /devices/platform/host4/session18/target4:0:0/4:0:0:1/block/sdg (block) UDEV [19172.704605] add /devices/platform/host3/session17/target3:0:0/3:0:0:1/block/sde (block) UDEV [19172.709687] add /devices/platform/host3/session17/target3:0:0/3:0:0:3/block/sdj (block) UDEV [19172.714919] add /devices/platform/host4/session18/target4:0:0/4:0:0:3/block/sdl (block) UDEV [19172.728891] add /devices/platform/host4/session18/target4:0:0/4:0:0:4/block/sdo (block) UDEV [19172.872156] add /devices/platform/host3/session17/target3:0:0/3:0:0:6/block/sdt (block) UDEV [19172.915542] add /devices/platform/host4/session18/target4:0:0/4:0:0:6/block/sdu (block) UDEV [19173.086935] add /devices/platform/host6/session20/target6:0:0/6:0:0:0/block/sdw (block) UDEV [19173.108278] add /devices/platform/host6/session20/target6:0:0/6:0:0:1/block/sdz (block) UDEV [19173.195153] add /devices/platform/host4/session18/target4:0:0/4:0:0:5/block/sdr (block) UDEV [19173.228397] add /devices/platform/host3/session17/target3:0:0/3:0:0:5/block/sdq (block) UDEV [19173.363632] add /devices/platform/host5/session19/target5:0:0/5:0:0:2/block/sdm (block) UDEV [19173.386560] add /devices/platform/host3/session17/target3:0:0/3:0:0:7/block/sdx (block) UDEV [19173.394515] add /devices/platform/host4/session18/target4:0:0/4:0:0:7/block/sdy (block) UDEV [19173.410152] add /devices/platform/host5/session19/target5:0:0/5:0:0:1/block/sdk (block) UDEV [19173.474286] add /devices/platform/host6/session20/target6:0:0/6:0:0:2/block/sdab (block) UDEV [19173.508438] add /devices/platform/host5/session19/target5:0:0/5:0:0:3/block/sdp (block) UDEV [19173.713146] add /devices/platform/host5/session19/target5:0:0/5:0:0:4/block/sds (block) UDEV [19173.782065] add /devices/platform/host5/session19/target5:0:0/5:0:0:5/block/sdv (block) UDEV [19173.787447] add /devices/platform/host5/session19/target5:0:0/5:0:0:6/block/sdaa (block) UDEV [19173.803218] add /devices/platform/host6/session20/target6:0:0/6:0:0:3/block/sdad (block) UDEV [19173.849411] add /devices/platform/host5/session19/target5:0:0/5:0:0:7/block/sdac (block) UDEV [19173.918301] add /devices/platform/host6/session20/target6:0:0/6:0:0:5/block/sdaf (block) UDEV [19173.941005] add /devices/platform/host6/session20/target6:0:0/6:0:0:4/block/sdae (block) UDEV [19173.987206] add /devices/platform/host6/session20/target6:0:0/6:0:0:7/block/sdah (block) UDEV [19173.992431] add /devices/platform/host6/session20/target6:0:0/6:0:0:6/block/sdag (block) Or like this: UDEV [20712.402631] remove /devices/platform/host3/session17/target3:0:0/3:0:0:0/block/sdc (block) UDEV [20712.427716] remove /devices/platform/host6/session20/target6:0:0/6:0:0:0/block/sdw (block) UDEV [20712.459854] remove /devices/platform/host4/session18/target4:0:0/4:0:0:0/block/sdd (block) UDEV [20712.471124] remove /devices/platform/host5/session19/target5:0:0/5:0:0:0/block/sdh (block) UDEV [20712.492190] remove /devices/platform/host6/session20/target6:0:0/6:0:0:1/block/sdz (block) UDEV [20712.495245] remove /devices/platform/host4/session18/target4:0:0/4:0:0:1/block/sdg (block) UDEV [20712.512007] remove /devices/platform/host3/session17/target3:0:0/3:0:0:1/block/sde (block) UDEV [20712.522986] remove /devices/platform/host5/session19/target5:0:0/5:0:0:1/block/sdk (block) UDEV [20712.528676] remove /devices/platform/host6/session20/target6:0:0/6:0:0:2/block/sdab (block) UDEV [20712.529891] remove /devices/platform/host5/session19/target5:0:0/5:0:0:2/block/sdm (block) UDEV [20712.536178] remove /devices/platform/host4/session18/target4:0:0/4:0:0:2/block/sdi (block) UDEV [20712.545444] remove /devices/platform/host4/session18/target4:0:0/4:0:0:3/block/sdl (block) UDEV [20712.548006] remove /devices/platform/host3/session17/target3:0:0/3:0:0:3/block/sdj (block) UDEV [20712.551935] remove /devices/platform/host5/session19/target5:0:0/5:0:0:3/block/sdp
