linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] Generate uevents for all DM events
@ 2016-10-03 19:22 Andy Grover
  2016-10-03 19:22 ` [PATCH 1/9] dm: Do not export dm_send_uevents Andy Grover
                   ` (10 more replies)
  0 siblings, 11 replies; 19+ messages in thread
From: Andy Grover @ 2016-10-03 19:22 UTC (permalink / raw)
  To: gregkh, snitzer; +Cc: dm-devel, linux-kernel

Hi Mike and GregKH,

I want a way to get devicemapper events without using the DM ioctl,
because that requires creating a thread to sleep in the ioctl for each
dm device I want events from.

It would seem like using uevents and KOBJ_CHANGE would be a good way
to do this, but Mike said that the uevent maintainers (Greg that's
you?) did not think this was a good idea?

If so, I was hoping you could talk a little more about why -- grep
shows KOBJ_CHANGE used all over, but its usage is not documented in
kobject.txt. In any case I can update kobject.txt with some more
guidelines.

The following patchset may be appliable if you're actually ok with
using KOBJ_CHANGE for dm events, or if not, then I'll look to rework
it to use a dm-specific genetlink approach.

Thanks -- Regards -- Andy

Andy Grover (9):
  dm: Do not export dm_send_uevents
  dm: Move multipath-specific stuff out of dm-uevent.c
  dm: Inline dm_build_path_uevent into dm_path_uevent
  dm: Update dm-uevent.txt
  dm: Rename dm_build_uevent to dm_uevent_build
  dm: Rename dm_event_add to dm_event_queue
  dm: Implement dm_uevent_add()
  dm: Generate uevents for thin targets
  dm: Generate uevents for other targets

 Documentation/device-mapper/dm-uevent.txt |  54 ++++++++++++++--
 drivers/md/dm-cache-target.c              |   5 +-
 drivers/md/dm-log-userspace-base.c        |   8 ++-
 drivers/md/dm-log.c                       |   1 +
 drivers/md/dm-mpath.c                     |  56 ++++++++++++++++
 drivers/md/dm-raid1.c                     |   1 +
 drivers/md/dm-snap.c                      |   1 +
 drivers/md/dm-thin.c                      |   3 +
 drivers/md/dm-uevent.c                    | 102 ++++++++++--------------------
 drivers/md/dm-uevent.h                    |  30 +++++++--
 drivers/md/dm.c                           |   3 +-
 include/linux/device-mapper.h             |   3 +-
 12 files changed, 180 insertions(+), 87 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/9] dm: Do not export dm_send_uevents
  2016-10-03 19:22 [PATCH 0/9] Generate uevents for all DM events Andy Grover
@ 2016-10-03 19:22 ` Andy Grover
  2016-10-03 19:22 ` [PATCH 2/9] dm: Move multipath-specific stuff out of dm-uevent.c Andy Grover
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Andy Grover @ 2016-10-03 19:22 UTC (permalink / raw)
  To: gregkh, snitzer; +Cc: dm-devel, linux-kernel

Since dm-uevent.c (if CONFIG_DM_UEVENT) is part of the same module as where
dm_sent_uevents is called, it does not need to be exported.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/md/dm-uevent.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/md/dm-uevent.c b/drivers/md/dm-uevent.c
index 8efe033..2d5f858 100644
--- a/drivers/md/dm-uevent.c
+++ b/drivers/md/dm-uevent.c
@@ -169,7 +169,6 @@ uevent_free:
 		dm_uevent_free(event);
 	}
 }
-EXPORT_SYMBOL_GPL(dm_send_uevents);
 
 /**
  * dm_path_uevent - called to create a new path event and queue it
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/9] dm: Move multipath-specific stuff out of dm-uevent.c
  2016-10-03 19:22 [PATCH 0/9] Generate uevents for all DM events Andy Grover
  2016-10-03 19:22 ` [PATCH 1/9] dm: Do not export dm_send_uevents Andy Grover
@ 2016-10-03 19:22 ` Andy Grover
  2016-10-03 19:22 ` [PATCH 3/9] dm: Inline dm_build_path_uevent into dm_path_uevent Andy Grover
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Andy Grover @ 2016-10-03 19:22 UTC (permalink / raw)
  To: gregkh, snitzer; +Cc: dm-devel, linux-kernel

There's a little bit of mpath-specific stuff that is in dm-uevent.c,
because all current uevents want to attach DM_PATH and DM_NR_VALID_PATHS
variables to the uevent. Makes sense since all currently defined DM
uevents are for dm-mpath, but is not true for future uevents. Move the
addition of these to dm-mpath.c and expose a few lower-level functions,
dm_build_uevent, dm_uevent_add and dm_uevent_free, for other dm targets to
build their own uevents.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/md/dm-mpath.c  | 71 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/md/dm-uevent.c | 75 +++++---------------------------------------------
 drivers/md/dm-uevent.h | 30 ++++++++++++++++----
 drivers/md/dm.c        |  1 +
 4 files changed, 103 insertions(+), 74 deletions(-)

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index ac734e5..c563b6d 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -30,6 +30,15 @@
 #define DM_PG_INIT_DELAY_MSECS 2000
 #define DM_PG_INIT_DELAY_DEFAULT ((unsigned) -1)
 
+static const struct {
+	enum dm_uevent_type type;
+	enum kobject_action action;
+	char *name;
+} _dm_mpath_uevent_type_names[] = {
+	{DM_UEVENT_PATH_FAILED, KOBJ_CHANGE, "PATH_FAILED"},
+	{DM_UEVENT_PATH_REINSTATED, KOBJ_CHANGE, "PATH_REINSTATED"},
+};
+
 /* Path properties */
 struct pgpath {
 	struct list_head list;
@@ -1232,6 +1241,68 @@ static void multipath_dtr(struct dm_target *ti)
 	free_multipath(m);
 }
 
+static struct dm_uevent *dm_build_path_uevent(struct mapped_device *md,
+					      struct dm_target *ti,
+					      enum kobject_action action,
+					      const char *dm_action,
+					      const char *path,
+					      unsigned nr_valid_paths)
+{
+	struct dm_uevent *event;
+
+	event = dm_build_uevent(md, ti, action, dm_action);
+	if (IS_ERR(event))
+		return event;
+
+	if (add_uevent_var(&event->ku_env, "DM_PATH=%s", path)) {
+		DMERR("%s: add_uevent_var() for DM_PATH failed", __func__);
+		goto err_add;
+	}
+
+	if (add_uevent_var(&event->ku_env, "DM_NR_VALID_PATHS=%d",
+			   nr_valid_paths)) {
+		DMERR("%s: add_uevent_var() for DM_NR_VALID_PATHS failed",
+		      __func__);
+		goto err_add;
+	}
+
+	return event;
+
+err_add:
+	dm_uevent_free(event);
+	return ERR_PTR(-ENOMEM);
+}
+
+/**
+ * dm_path_uevent - called to create a new path event and queue it
+ *
+ * @event_type:	path event type enum
+ * @ti:			pointer to a dm_target
+ * @path:		string containing pathname
+ * @nr_valid_paths:	number of valid paths remaining
+ *
+ */
+static void dm_path_uevent(enum dm_uevent_type event_type, struct dm_target *ti,
+		   const char *path, unsigned nr_valid_paths)
+{
+	struct mapped_device *md = dm_table_get_md(ti->table);
+	struct dm_uevent *event;
+
+	if (event_type >= ARRAY_SIZE(_dm_mpath_uevent_type_names)) {
+		DMERR("%s: Invalid event_type %d", __func__, event_type);
+		return;
+	}
+
+	event = dm_build_path_uevent(md, ti,
+				     _dm_mpath_uevent_type_names[event_type].action,
+				     _dm_mpath_uevent_type_names[event_type].name,
+				     path, nr_valid_paths);
+	if (IS_ERR(event))
+		return;
+
+	dm_uevent_add(md, &event->elist);
+}
+
 /*
  * Take a path out of use.
  */
diff --git a/drivers/md/dm-uevent.c b/drivers/md/dm-uevent.c
index 2d5f858..f7089dd 100644
--- a/drivers/md/dm-uevent.c
+++ b/drivers/md/dm-uevent.c
@@ -29,30 +29,13 @@
 
 #define DM_MSG_PREFIX "uevent"
 
-static const struct {
-	enum dm_uevent_type type;
-	enum kobject_action action;
-	char *name;
-} _dm_uevent_type_names[] = {
-	{DM_UEVENT_PATH_FAILED, KOBJ_CHANGE, "PATH_FAILED"},
-	{DM_UEVENT_PATH_REINSTATED, KOBJ_CHANGE, "PATH_REINSTATED"},
-};
-
 static struct kmem_cache *_dm_event_cache;
 
-struct dm_uevent {
-	struct mapped_device *md;
-	enum kobject_action action;
-	struct kobj_uevent_env ku_env;
-	struct list_head elist;
-	char name[DM_NAME_LEN];
-	char uuid[DM_UUID_LEN];
-};
-
-static void dm_uevent_free(struct dm_uevent *event)
+void dm_uevent_free(struct dm_uevent *event)
 {
 	kmem_cache_free(_dm_event_cache, event);
 }
+EXPORT_SYMBOL_GPL(dm_uevent_free);
 
 static struct dm_uevent *dm_uevent_alloc(struct mapped_device *md)
 {
@@ -68,12 +51,10 @@ static struct dm_uevent *dm_uevent_alloc(struct mapped_device *md)
 	return event;
 }
 
-static struct dm_uevent *dm_build_path_uevent(struct mapped_device *md,
-					      struct dm_target *ti,
-					      enum kobject_action action,
-					      const char *dm_action,
-					      const char *path,
-					      unsigned nr_valid_paths)
+struct dm_uevent *dm_build_uevent(struct mapped_device *md,
+					 struct dm_target *ti,
+					 enum kobject_action action,
+					 const char *dm_action)
 {
 	struct dm_uevent *event;
 
@@ -104,18 +85,6 @@ static struct dm_uevent *dm_build_path_uevent(struct mapped_device *md,
 		goto err_add;
 	}
 
-	if (add_uevent_var(&event->ku_env, "DM_PATH=%s", path)) {
-		DMERR("%s: add_uevent_var() for DM_PATH failed", __func__);
-		goto err_add;
-	}
-
-	if (add_uevent_var(&event->ku_env, "DM_NR_VALID_PATHS=%d",
-			   nr_valid_paths)) {
-		DMERR("%s: add_uevent_var() for DM_NR_VALID_PATHS failed",
-		      __func__);
-		goto err_add;
-	}
-
 	return event;
 
 err_add:
@@ -123,6 +92,7 @@ err_add:
 err_nomem:
 	return ERR_PTR(-ENOMEM);
 }
+EXPORT_SYMBOL_GPL(dm_build_uevent);
 
 /**
  * dm_send_uevents - send uevents for given list
@@ -170,37 +140,6 @@ uevent_free:
 	}
 }
 
-/**
- * dm_path_uevent - called to create a new path event and queue it
- *
- * @event_type:	path event type enum
- * @ti:			pointer to a dm_target
- * @path:		string containing pathname
- * @nr_valid_paths:	number of valid paths remaining
- *
- */
-void dm_path_uevent(enum dm_uevent_type event_type, struct dm_target *ti,
-		   const char *path, unsigned nr_valid_paths)
-{
-	struct mapped_device *md = dm_table_get_md(ti->table);
-	struct dm_uevent *event;
-
-	if (event_type >= ARRAY_SIZE(_dm_uevent_type_names)) {
-		DMERR("%s: Invalid event_type %d", __func__, event_type);
-		return;
-	}
-
-	event = dm_build_path_uevent(md, ti,
-				     _dm_uevent_type_names[event_type].action,
-				     _dm_uevent_type_names[event_type].name,
-				     path, nr_valid_paths);
-	if (IS_ERR(event))
-		return;
-
-	dm_uevent_add(md, &event->elist);
-}
-EXPORT_SYMBOL_GPL(dm_path_uevent);
-
 int dm_uevent_init(void)
 {
 	_dm_event_cache = KMEM_CACHE(dm_uevent, 0);
diff --git a/drivers/md/dm-uevent.h b/drivers/md/dm-uevent.h
index 2eccc8b..4ff2ad1 100644
--- a/drivers/md/dm-uevent.h
+++ b/drivers/md/dm-uevent.h
@@ -21,6 +21,17 @@
 #ifndef DM_UEVENT_H
 #define DM_UEVENT_H
 
+#include <linux/dm-ioctl.h> // for DM_*_LEN
+
+struct dm_uevent {
+	struct mapped_device *md;
+	enum kobject_action action;
+	struct kobj_uevent_env ku_env;
+	struct list_head elist;
+	char name[DM_NAME_LEN];
+	char uuid[DM_UUID_LEN];
+};
+
 enum dm_uevent_type {
 	DM_UEVENT_PATH_FAILED,
 	DM_UEVENT_PATH_REINSTATED,
@@ -31,9 +42,11 @@ enum dm_uevent_type {
 extern int dm_uevent_init(void);
 extern void dm_uevent_exit(void);
 extern void dm_send_uevents(struct list_head *events, struct kobject *kobj);
-extern void dm_path_uevent(enum dm_uevent_type event_type,
-			   struct dm_target *ti, const char *path,
-			   unsigned nr_valid_paths);
+struct dm_uevent *dm_build_uevent(struct mapped_device *md,
+					 struct dm_target *ti,
+					 enum kobject_action action,
+					 const char *dm_action);
+void dm_uevent_free(struct dm_uevent *event);
 
 #else
 
@@ -48,9 +61,14 @@ static inline void dm_send_uevents(struct list_head *events,
 				   struct kobject *kobj)
 {
 }
-static inline void dm_path_uevent(enum dm_uevent_type event_type,
-				  struct dm_target *ti, const char *path,
-				  unsigned nr_valid_paths)
+static inline struct dm_uevent *dm_build_uevent(struct mapped_device *md,
+						struct dm_target *ti,
+						enum kobject_action action,
+						const char *dm_action)
+{
+	return ERR_PTR(-ENOMEM);
+}
+static void dm_uevent_free(struct dm_uevent *event)
 {
 }
 
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index fa9b1cb..701c75f 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -2427,6 +2427,7 @@ void dm_uevent_add(struct mapped_device *md, struct list_head *elist)
 	list_add(elist, &md->uevent_list);
 	spin_unlock_irqrestore(&md->uevent_lock, flags);
 }
+EXPORT_SYMBOL_GPL(dm_uevent_add);
 
 /*
  * The gendisk is only valid as long as you have a reference
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/9] dm: Inline dm_build_path_uevent into dm_path_uevent
  2016-10-03 19:22 [PATCH 0/9] Generate uevents for all DM events Andy Grover
  2016-10-03 19:22 ` [PATCH 1/9] dm: Do not export dm_send_uevents Andy Grover
  2016-10-03 19:22 ` [PATCH 2/9] dm: Move multipath-specific stuff out of dm-uevent.c Andy Grover
@ 2016-10-03 19:22 ` Andy Grover
  2016-10-03 19:22 ` [PATCH 4/9] dm: Update dm-uevent.txt Andy Grover
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Andy Grover @ 2016-10-03 19:22 UTC (permalink / raw)
  To: gregkh, snitzer; +Cc: dm-devel, linux-kernel

Since it's no longer an API boundary we can consolidate these two
functions.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/md/dm-mpath.c | 59 +++++++++++++++++++--------------------------------
 1 file changed, 22 insertions(+), 37 deletions(-)

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index c563b6d..75fe28a 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -1241,38 +1241,6 @@ static void multipath_dtr(struct dm_target *ti)
 	free_multipath(m);
 }
 
-static struct dm_uevent *dm_build_path_uevent(struct mapped_device *md,
-					      struct dm_target *ti,
-					      enum kobject_action action,
-					      const char *dm_action,
-					      const char *path,
-					      unsigned nr_valid_paths)
-{
-	struct dm_uevent *event;
-
-	event = dm_build_uevent(md, ti, action, dm_action);
-	if (IS_ERR(event))
-		return event;
-
-	if (add_uevent_var(&event->ku_env, "DM_PATH=%s", path)) {
-		DMERR("%s: add_uevent_var() for DM_PATH failed", __func__);
-		goto err_add;
-	}
-
-	if (add_uevent_var(&event->ku_env, "DM_NR_VALID_PATHS=%d",
-			   nr_valid_paths)) {
-		DMERR("%s: add_uevent_var() for DM_NR_VALID_PATHS failed",
-		      __func__);
-		goto err_add;
-	}
-
-	return event;
-
-err_add:
-	dm_uevent_free(event);
-	return ERR_PTR(-ENOMEM);
-}
-
 /**
  * dm_path_uevent - called to create a new path event and queue it
  *
@@ -1293,14 +1261,31 @@ static void dm_path_uevent(enum dm_uevent_type event_type, struct dm_target *ti,
 		return;
 	}
 
-	event = dm_build_path_uevent(md, ti,
-				     _dm_mpath_uevent_type_names[event_type].action,
-				     _dm_mpath_uevent_type_names[event_type].name,
-				     path, nr_valid_paths);
-	if (IS_ERR(event))
+	event = dm_build_uevent(md,
+				ti,
+				_dm_mpath_uevent_type_names[event_type].action,
+				_dm_mpath_uevent_type_names[event_type].name);
+	if (IS_ERR(event)) {
+		DMERR("%s: dm_build_uevent() failed", __func__);
 		return;
+	}
+
+	if (add_uevent_var(&event->ku_env, "DM_PATH=%s", path)) {
+		DMERR("%s: add_uevent_var() for DM_PATH failed", __func__);
+		goto err;
+	}
+
+	if (add_uevent_var(&event->ku_env, "DM_NR_VALID_PATHS=%d",
+			   nr_valid_paths)) {
+		DMERR("%s: add_uevent_var() for DM_NR_VALID_PATHS failed",
+		      __func__);
+		goto err;
+	}
 
 	dm_uevent_add(md, &event->elist);
+	return;
+err:
+	dm_uevent_free(event);
 }
 
 /*
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/9] dm: Update dm-uevent.txt
  2016-10-03 19:22 [PATCH 0/9] Generate uevents for all DM events Andy Grover
                   ` (2 preceding siblings ...)
  2016-10-03 19:22 ` [PATCH 3/9] dm: Inline dm_build_path_uevent into dm_path_uevent Andy Grover
@ 2016-10-03 19:22 ` Andy Grover
  2016-10-03 19:22 ` [PATCH 5/9] dm: Rename dm_build_uevent to dm_uevent_build Andy Grover
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Andy Grover @ 2016-10-03 19:22 UTC (permalink / raw)
  To: gregkh, snitzer; +Cc: dm-devel, linux-kernel

Document the current dm uevent API, as modified by this patchset.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 Documentation/device-mapper/dm-uevent.txt | 49 ++++++++++++++++++++++++++-----
 1 file changed, 42 insertions(+), 7 deletions(-)

diff --git a/Documentation/device-mapper/dm-uevent.txt b/Documentation/device-mapper/dm-uevent.txt
index 07edbd85..0a5d909 100644
--- a/Documentation/device-mapper/dm-uevent.txt
+++ b/Documentation/device-mapper/dm-uevent.txt
@@ -1,18 +1,53 @@
 The device-mapper uevent code adds the capability to device-mapper to create
 and send kobject uevents (uevents).  Previously device-mapper events were only
-available through the ioctl interface.  The advantage of the uevents interface
-is the event contains environment attributes providing increased context for
+available through the ioctl interface.  The advantages of the uevents interface
+are the event contains environment attributes providing increased context for
 the event avoiding the need to query the state of the device-mapper device after
-the event is received.
+the event is received, and uevents are poll()-able in userspace, whereas the
+ioctl event interface is not.
 
-There are two functions currently for device-mapper events.  The first function
-listed creates the event and the second function sends the event(s).
+There are five functions DM targets should use.
 
-void dm_path_uevent(enum dm_uevent_type event_type, struct dm_target *ti,
-                    const char *path, unsigned nr_valid_paths)
+struct dm_uevent *dm_build_uevent(struct mapped_device *md,
+					 struct dm_target *ti,
+					 enum kobject_action action,
+					 const char *dm_action)
+
+Construct a dm uevent with default DM variables attached(DM_TARGET,
+DM_ACTION, DM_SEQNUM).
+
+int add_uevent_var(struct kobj_uevent_env *env, const char *format, ...)
+
+Optionally add event-specific data to the generated uevent. e.g. dm-mpath's
+PATH_FAILED and PATH_REINSTATED uevents add path and number-of-remaining-paths
+vars.
+
+void dm_uevent_add(struct mapped_device *md, struct list_head *elist)
+
+Hand off the uevent to the device's list of pending events.
+
+void dm_uevent_free(struct dm_uevent *event)
+
+Something went wrong, free the uevent instead of queueing it.
+
+void dm_table_event(struct dm_table *t)
+
+Triggers sending of queued uevents as well as waking up processes waiting on
+the ioctl.
+
+
+The device-mapper uevent code also supplies three functions for device-mapper core
+to call:
+
+int dm_uevent_init(void)
+void dm_uevent_exit(void)
+
+Setup and teardown.
 
 void dm_send_uevents(struct list_head *events, struct kobject *kobj)
 
+Actually send the uevents (called indirectly from dm_table_event, above).
+
 
 The variables added to the uevent environment are:
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 5/9] dm: Rename dm_build_uevent to dm_uevent_build
  2016-10-03 19:22 [PATCH 0/9] Generate uevents for all DM events Andy Grover
                   ` (3 preceding siblings ...)
  2016-10-03 19:22 ` [PATCH 4/9] dm: Update dm-uevent.txt Andy Grover
@ 2016-10-03 19:22 ` Andy Grover
  2016-10-03 19:22 ` [PATCH 6/9] dm: Rename dm_event_add to dm_event_queue Andy Grover
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Andy Grover @ 2016-10-03 19:22 UTC (permalink / raw)
  To: gregkh, snitzer; +Cc: dm-devel, linux-kernel

For consistency with other function names that start with 'dm_uevent'.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 Documentation/device-mapper/dm-uevent.txt | 2 +-
 drivers/md/dm-mpath.c                     | 4 ++--
 drivers/md/dm-uevent.c                    | 4 ++--
 drivers/md/dm-uevent.h                    | 4 ++--
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/device-mapper/dm-uevent.txt b/Documentation/device-mapper/dm-uevent.txt
index 0a5d909..5780e76 100644
--- a/Documentation/device-mapper/dm-uevent.txt
+++ b/Documentation/device-mapper/dm-uevent.txt
@@ -8,7 +8,7 @@ ioctl event interface is not.
 
 There are five functions DM targets should use.
 
-struct dm_uevent *dm_build_uevent(struct mapped_device *md,
+struct dm_uevent *dm_uevent_build(struct mapped_device *md,
 					 struct dm_target *ti,
 					 enum kobject_action action,
 					 const char *dm_action)
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 75fe28a..26f99d0 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -1261,12 +1261,12 @@ static void dm_path_uevent(enum dm_uevent_type event_type, struct dm_target *ti,
 		return;
 	}
 
-	event = dm_build_uevent(md,
+	event = dm_uevent_build(md,
 				ti,
 				_dm_mpath_uevent_type_names[event_type].action,
 				_dm_mpath_uevent_type_names[event_type].name);
 	if (IS_ERR(event)) {
-		DMERR("%s: dm_build_uevent() failed", __func__);
+		DMERR("%s: dm_uevent_build() failed", __func__);
 		return;
 	}
 
diff --git a/drivers/md/dm-uevent.c b/drivers/md/dm-uevent.c
index f7089dd..6a72de7 100644
--- a/drivers/md/dm-uevent.c
+++ b/drivers/md/dm-uevent.c
@@ -51,7 +51,7 @@ static struct dm_uevent *dm_uevent_alloc(struct mapped_device *md)
 	return event;
 }
 
-struct dm_uevent *dm_build_uevent(struct mapped_device *md,
+struct dm_uevent *dm_uevent_build(struct mapped_device *md,
 					 struct dm_target *ti,
 					 enum kobject_action action,
 					 const char *dm_action)
@@ -92,7 +92,7 @@ err_add:
 err_nomem:
 	return ERR_PTR(-ENOMEM);
 }
-EXPORT_SYMBOL_GPL(dm_build_uevent);
+EXPORT_SYMBOL_GPL(dm_uevent_build);
 
 /**
  * dm_send_uevents - send uevents for given list
diff --git a/drivers/md/dm-uevent.h b/drivers/md/dm-uevent.h
index 4ff2ad1..4dc99c2 100644
--- a/drivers/md/dm-uevent.h
+++ b/drivers/md/dm-uevent.h
@@ -42,7 +42,7 @@ enum dm_uevent_type {
 extern int dm_uevent_init(void);
 extern void dm_uevent_exit(void);
 extern void dm_send_uevents(struct list_head *events, struct kobject *kobj);
-struct dm_uevent *dm_build_uevent(struct mapped_device *md,
+struct dm_uevent *dm_uevent_build(struct mapped_device *md,
 					 struct dm_target *ti,
 					 enum kobject_action action,
 					 const char *dm_action);
@@ -61,7 +61,7 @@ static inline void dm_send_uevents(struct list_head *events,
 				   struct kobject *kobj)
 {
 }
-static inline struct dm_uevent *dm_build_uevent(struct mapped_device *md,
+static inline struct dm_uevent *dm_uevent_build(struct mapped_device *md,
 						struct dm_target *ti,
 						enum kobject_action action,
 						const char *dm_action)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 6/9] dm: Rename dm_event_add to dm_event_queue
  2016-10-03 19:22 [PATCH 0/9] Generate uevents for all DM events Andy Grover
                   ` (4 preceding siblings ...)
  2016-10-03 19:22 ` [PATCH 5/9] dm: Rename dm_build_uevent to dm_uevent_build Andy Grover
@ 2016-10-03 19:22 ` Andy Grover
  2016-10-03 19:22 ` [PATCH 7/9] dm: Implement dm_uevent_add() Andy Grover
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Andy Grover @ 2016-10-03 19:22 UTC (permalink / raw)
  To: gregkh, snitzer; +Cc: dm-devel, linux-kernel

For clarity. This function queues an event to be sent, which is not
totally clear from the previous name. And, we want to reuse the name
for something else.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 Documentation/device-mapper/dm-uevent.txt | 2 +-
 drivers/md/dm-mpath.c                     | 2 +-
 drivers/md/dm.c                           | 4 ++--
 include/linux/device-mapper.h             | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/device-mapper/dm-uevent.txt b/Documentation/device-mapper/dm-uevent.txt
index 5780e76..4fcb71d 100644
--- a/Documentation/device-mapper/dm-uevent.txt
+++ b/Documentation/device-mapper/dm-uevent.txt
@@ -22,7 +22,7 @@ Optionally add event-specific data to the generated uevent. e.g. dm-mpath's
 PATH_FAILED and PATH_REINSTATED uevents add path and number-of-remaining-paths
 vars.
 
-void dm_uevent_add(struct mapped_device *md, struct list_head *elist)
+void dm_uevent_queue(struct mapped_device *md, struct list_head *elist)
 
 Hand off the uevent to the device's list of pending events.
 
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 26f99d0..467724c 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -1282,7 +1282,7 @@ static void dm_path_uevent(enum dm_uevent_type event_type, struct dm_target *ti,
 		goto err;
 	}
 
-	dm_uevent_add(md, &event->elist);
+	dm_uevent_queue(md, &event->elist);
 	return;
 err:
 	dm_uevent_free(event);
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 701c75f..37b3a82 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -2419,7 +2419,7 @@ int dm_wait_event(struct mapped_device *md, int event_nr)
 			(event_nr != atomic_read(&md->event_nr)));
 }
 
-void dm_uevent_add(struct mapped_device *md, struct list_head *elist)
+void dm_uevent_queue(struct mapped_device *md, struct list_head *elist)
 {
 	unsigned long flags;
 
@@ -2427,7 +2427,7 @@ void dm_uevent_add(struct mapped_device *md, struct list_head *elist)
 	list_add(elist, &md->uevent_list);
 	spin_unlock_irqrestore(&md->uevent_lock, flags);
 }
-EXPORT_SYMBOL_GPL(dm_uevent_add);
+EXPORT_SYMBOL_GPL(dm_uevent_queue);
 
 /*
  * The gendisk is only valid as long as you have a reference
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index 91acfce..024207c 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -419,7 +419,7 @@ int dm_resume(struct mapped_device *md);
 uint32_t dm_get_event_nr(struct mapped_device *md);
 int dm_wait_event(struct mapped_device *md, int event_nr);
 uint32_t dm_next_uevent_seq(struct mapped_device *md);
-void dm_uevent_add(struct mapped_device *md, struct list_head *elist);
+void dm_uevent_queue(struct mapped_device *md, struct list_head *elist);
 
 /*
  * Info functions.
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 7/9] dm: Implement dm_uevent_add()
  2016-10-03 19:22 [PATCH 0/9] Generate uevents for all DM events Andy Grover
                   ` (5 preceding siblings ...)
  2016-10-03 19:22 ` [PATCH 6/9] dm: Rename dm_event_add to dm_event_queue Andy Grover
@ 2016-10-03 19:22 ` Andy Grover
  2016-10-03 19:22 ` [PATCH 8/9] dm: Generate uevents for thin targets Andy Grover
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Andy Grover @ 2016-10-03 19:22 UTC (permalink / raw)
  To: gregkh, snitzer; +Cc: dm-devel, linux-kernel

This helper function builds and enqueues the uevent. Targets can use this
when they do not need to customize the uevent beyond its name.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 Documentation/device-mapper/dm-uevent.txt |  5 +++++
 drivers/md/dm-uevent.c                    | 26 ++++++++++++++++++++++++++
 include/linux/device-mapper.h             |  1 +
 3 files changed, 32 insertions(+)

diff --git a/Documentation/device-mapper/dm-uevent.txt b/Documentation/device-mapper/dm-uevent.txt
index 4fcb71d..097e7f4 100644
--- a/Documentation/device-mapper/dm-uevent.txt
+++ b/Documentation/device-mapper/dm-uevent.txt
@@ -30,6 +30,11 @@ void dm_uevent_free(struct dm_uevent *event)
 
 Something went wrong, free the uevent instead of queueing it.
 
+void dm_uevent_add(struct dm_target *ti, enum kobject_action action, char *name)
+
+Helper function that builds and queues a uevent if no additional
+variables need to be added.
+
 void dm_table_event(struct dm_table *t)
 
 Triggers sending of queued uevents as well as waking up processes waiting on
diff --git a/drivers/md/dm-uevent.c b/drivers/md/dm-uevent.c
index 6a72de7..d139135 100644
--- a/drivers/md/dm-uevent.c
+++ b/drivers/md/dm-uevent.c
@@ -95,6 +95,32 @@ err_nomem:
 EXPORT_SYMBOL_GPL(dm_uevent_build);
 
 /**
+ * dm_uevent_add - Create and queue a basic uevent
+ *
+ * @ti:		The target to add a uevent for
+ * @action:	The kobject action
+ * @name:	The name of the uevent to add and queue
+ *
+ * If targets wish to create and queue a uevent but don't need to add
+ * extra vars, they can do so more easily by calling this.
+ * This is usually followed by a call to dm_table_event().
+ */
+void dm_uevent_add(struct dm_target *ti, enum kobject_action action, char *name)
+{
+	struct mapped_device *md = dm_table_get_md(ti->table);
+	struct dm_uevent *event;
+
+	event = dm_uevent_build(md, ti, action, name);
+	if (IS_ERR(event)) {
+		DMERR("%s: dm_uevent_build() failed", __func__);
+		return;
+	}
+
+	dm_uevent_queue(md, &event->elist);
+}
+EXPORT_SYMBOL_GPL(dm_uevent_add);
+
+/**
  * dm_send_uevents - send uevents for given list
  *
  * @events:	list of events to send
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index 024207c..c102e29 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -420,6 +420,7 @@ uint32_t dm_get_event_nr(struct mapped_device *md);
 int dm_wait_event(struct mapped_device *md, int event_nr);
 uint32_t dm_next_uevent_seq(struct mapped_device *md);
 void dm_uevent_queue(struct mapped_device *md, struct list_head *elist);
+void dm_uevent_add(struct dm_target *ti, enum kobject_action action, char *name);
 
 /*
  * Info functions.
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 8/9] dm: Generate uevents for thin targets
  2016-10-03 19:22 [PATCH 0/9] Generate uevents for all DM events Andy Grover
                   ` (6 preceding siblings ...)
  2016-10-03 19:22 ` [PATCH 7/9] dm: Implement dm_uevent_add() Andy Grover
@ 2016-10-03 19:22 ` Andy Grover
  2016-10-03 19:23 ` [PATCH 9/9] dm: Generate uevents for other targets Andy Grover
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Andy Grover @ 2016-10-03 19:22 UTC (permalink / raw)
  To: gregkh, snitzer; +Cc: dm-devel, linux-kernel

Generate uevents when thin pool devices hit data or metadata low water
marks, and when pool mode changes.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/md/dm-thin.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c
index d1c05c1..0778a2a 100644
--- a/drivers/md/dm-thin.c
+++ b/drivers/md/dm-thin.c
@@ -1429,6 +1429,7 @@ static void check_low_water_mark(struct pool *pool, dm_block_t free_blocks)
 		spin_lock_irqsave(&pool->lock, flags);
 		pool->low_water_triggered = true;
 		spin_unlock_irqrestore(&pool->lock, flags);
+		dm_uevent_add(pool->ti, KOBJ_CHANGE, "THIN_LOW_WATER_DATA");
 		dm_table_event(pool->ti->table);
 	}
 }
@@ -2397,6 +2398,7 @@ static enum pool_mode get_pool_mode(struct pool *pool)
 
 static void notify_of_pool_mode_change(struct pool *pool, const char *new_mode)
 {
+	dm_uevent_add(pool->ti, KOBJ_CHANGE, "THIN_POOL_MODE");
 	dm_table_event(pool->ti->table);
 	DMINFO("%s: switching pool to %s mode",
 	       dm_device_name(pool->pool_md), new_mode);
@@ -3095,6 +3097,7 @@ static void metadata_low_callback(void *context)
 	DMWARN("%s: reached low water mark for metadata device: sending event.",
 	       dm_device_name(pool->pool_md));
 
+	dm_uevent_add(pool->ti, KOBJ_CHANGE, "THIN_LOW_WATER_METADATA");
 	dm_table_event(pool->ti->table);
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 9/9] dm: Generate uevents for other targets
  2016-10-03 19:22 [PATCH 0/9] Generate uevents for all DM events Andy Grover
                   ` (7 preceding siblings ...)
  2016-10-03 19:22 ` [PATCH 8/9] dm: Generate uevents for thin targets Andy Grover
@ 2016-10-03 19:23 ` Andy Grover
  2016-10-03 19:29 ` [PATCH 0/9] Generate uevents for all DM events Mike Snitzer
  2016-10-04  7:20 ` Greg KH
  10 siblings, 0 replies; 19+ messages in thread
From: Andy Grover @ 2016-10-03 19:23 UTC (permalink / raw)
  To: gregkh, snitzer; +Cc: dm-devel, linux-kernel

Generate uevents for other targets: cache, log, raid1, and snap.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/md/dm-cache-target.c       | 5 ++++-
 drivers/md/dm-log-userspace-base.c | 8 ++++++--
 drivers/md/dm-log.c                | 1 +
 drivers/md/dm-raid1.c              | 1 +
 drivers/md/dm-snap.c               | 1 +
 5 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
index 59b2c50..29f0bc9 100644
--- a/drivers/md/dm-cache-target.c
+++ b/drivers/md/dm-cache-target.c
@@ -586,8 +586,10 @@ static void clear_dirty(struct cache *cache, dm_oblock_t oblock, dm_cblock_t cbl
 {
 	if (test_and_clear_bit(from_cblock(cblock), cache->dirty_bitset)) {
 		policy_clear_dirty(cache->policy, oblock);
-		if (atomic_dec_return(&cache->nr_dirty) == 0)
+		if (atomic_dec_return(&cache->nr_dirty) == 0) {
+			dm_uevent_add(cache->ti, KOBJ_CHANGE, "CACHE_CLEAN");
 			dm_table_event(cache->ti->table);
+		}
 	}
 }
 
@@ -978,6 +980,7 @@ static void notify_mode_switch(struct cache *cache, enum cache_metadata_mode mod
 		"fail"
 	};
 
+	dm_uevent_add(cache->ti, KOBJ_CHANGE, "CACHE_MODE_SWITCH");
 	dm_table_event(cache->ti->table);
 	DMINFO("%s: switching cache to %s mode",
 	       cache_device_name(cache), descs[(int)mode]);
diff --git a/drivers/md/dm-log-userspace-base.c b/drivers/md/dm-log-userspace-base.c
index 53b7b06d..cb0b6cb 100644
--- a/drivers/md/dm-log-userspace-base.c
+++ b/drivers/md/dm-log-userspace-base.c
@@ -162,8 +162,10 @@ static void do_flush(struct work_struct *work)
 
 	r = userspace_do_request(lc, lc->uuid, DM_ULOG_FLUSH, NULL, 0, NULL, NULL);
 
-	if (r)
+	if (r) {
+		dm_uevent_add(lc->ti->table, KOBJ_CHANGE, "LOG_FLUSHED");
 		dm_table_event(lc->ti->table);
+	}
 }
 
 /*
@@ -634,8 +636,10 @@ out:
 		mempool_free(fe, flush_entry_pool);
 	}
 
-	if (r)
+	if (r) {
+		dm_uevent_add(lc->ti->table, KOBJ_CHANGE, "LOG_FLUSHED");
 		dm_table_event(lc->ti->table);
+	}
 
 	return r;
 }
diff --git a/drivers/md/dm-log.c b/drivers/md/dm-log.c
index 07fc1ad..8d144d2 100644
--- a/drivers/md/dm-log.c
+++ b/drivers/md/dm-log.c
@@ -579,6 +579,7 @@ static void fail_log_device(struct log_c *lc)
 		return;
 
 	lc->log_dev_failed = 1;
+	dm_uevent_add(lc->ti, KOBJ_CHANGE, "LOG_FAILED");
 	dm_table_event(lc->ti->table);
 }
 
diff --git a/drivers/md/dm-raid1.c b/drivers/md/dm-raid1.c
index bdf1606..daffbab 100644
--- a/drivers/md/dm-raid1.c
+++ b/drivers/md/dm-raid1.c
@@ -410,6 +410,7 @@ static void do_recovery(struct mirror_set *ms)
 	if (!ms->in_sync &&
 	    (log->type->get_sync_count(log) == ms->nr_regions)) {
 		/* the sync is complete */
+		dm_uevent_add(ms->ti, KOBJ_CHANGE, "TABLE_SYNCED");
 		dm_table_event(ms->ti->table);
 		ms->in_sync = 1;
 		reset_ms_flags(ms);
diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
index c65feea..b47e28f6 100644
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -1443,6 +1443,7 @@ static void __invalidate_snapshot(struct dm_snapshot *s, int err)
 
 	s->valid = 0;
 
+	dm_uevent_add(s->ti, KOBJ_CHANGE, "SNAPSHOT_INVALIDATED");
 	dm_table_event(s->ti->table);
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/9] Generate uevents for all DM events
  2016-10-03 19:22 [PATCH 0/9] Generate uevents for all DM events Andy Grover
                   ` (8 preceding siblings ...)
  2016-10-03 19:23 ` [PATCH 9/9] dm: Generate uevents for other targets Andy Grover
@ 2016-10-03 19:29 ` Mike Snitzer
  2016-10-04  7:20 ` Greg KH
  10 siblings, 0 replies; 19+ messages in thread
From: Mike Snitzer @ 2016-10-03 19:29 UTC (permalink / raw)
  To: Andy Grover; +Cc: gregkh, dm-devel, linux-kernel

On Mon, Oct 03 2016 at  3:22pm -0400,
Andy Grover <agrover@redhat.com> wrote:

> Hi Mike and GregKH,
> 
> I want a way to get devicemapper events without using the DM ioctl,
> because that requires creating a thread to sleep in the ioctl for each
> dm device I want events from.
> 
> It would seem like using uevents and KOBJ_CHANGE would be a good way
> to do this, but Mike said that the uevent maintainers (Greg that's
> you?) did not think this was a good idea?

I think you might be confusing me with Alasdair?
 
> If so, I was hoping you could talk a little more about why -- grep
> shows KOBJ_CHANGE used all over, but its usage is not documented in
> kobject.txt. In any case I can update kobject.txt with some more
> guidelines.
> 
> The following patchset may be appliable if you're actually ok with
> using KOBJ_CHANGE for dm events, or if not, then I'll look to rework
> it to use a dm-specific genetlink approach.

I don't have strong opinions on this.  If Greg is OK with your approach
we can move forward (bringing Alasdair into the discussion too).

That said I haven't looked at your changes.  But your subjects make for
a confusing mix of "uevent" and "event".  Could just be that I'm too far
removed from the details to appreciate what is happening.

Mike

> Andy Grover (9):
>   dm: Do not export dm_send_uevents
>   dm: Move multipath-specific stuff out of dm-uevent.c
>   dm: Inline dm_build_path_uevent into dm_path_uevent
>   dm: Update dm-uevent.txt
>   dm: Rename dm_build_uevent to dm_uevent_build
>   dm: Rename dm_event_add to dm_event_queue
>   dm: Implement dm_uevent_add()
>   dm: Generate uevents for thin targets
>   dm: Generate uevents for other targets
> 
>  Documentation/device-mapper/dm-uevent.txt |  54 ++++++++++++++--
>  drivers/md/dm-cache-target.c              |   5 +-
>  drivers/md/dm-log-userspace-base.c        |   8 ++-
>  drivers/md/dm-log.c                       |   1 +
>  drivers/md/dm-mpath.c                     |  56 ++++++++++++++++
>  drivers/md/dm-raid1.c                     |   1 +
>  drivers/md/dm-snap.c                      |   1 +
>  drivers/md/dm-thin.c                      |   3 +
>  drivers/md/dm-uevent.c                    | 102 ++++++++++--------------------
>  drivers/md/dm-uevent.h                    |  30 +++++++--
>  drivers/md/dm.c                           |   3 +-
>  include/linux/device-mapper.h             |   3 +-
>  12 files changed, 180 insertions(+), 87 deletions(-)
> 
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/9] Generate uevents for all DM events
  2016-10-03 19:22 [PATCH 0/9] Generate uevents for all DM events Andy Grover
                   ` (9 preceding siblings ...)
  2016-10-03 19:29 ` [PATCH 0/9] Generate uevents for all DM events Mike Snitzer
@ 2016-10-04  7:20 ` Greg KH
  2016-10-04 23:39   ` Andy Grover
  10 siblings, 1 reply; 19+ messages in thread
From: Greg KH @ 2016-10-04  7:20 UTC (permalink / raw)
  To: Andy Grover; +Cc: snitzer, dm-devel, linux-kernel

On Mon, Oct 03, 2016 at 12:22:51PM -0700, Andy Grover wrote:
> Hi Mike and GregKH,
> 
> I want a way to get devicemapper events without using the DM ioctl,
> because that requires creating a thread to sleep in the ioctl for each
> dm device I want events from.
> 
> It would seem like using uevents and KOBJ_CHANGE would be a good way
> to do this, but Mike said that the uevent maintainers (Greg that's
> you?) did not think this was a good idea?

KBOJ_CHANGE is a tricky one.  It has been used for a variety of
different things, but usually it is used to show that a major change has
happened with a device like a docking station plug in or out.

How often do DM events happen?  What is triggering them?  How do you
want to send them to userspace?  Through a sysfs file?  Why not just use
your own netlink connection?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/9] Generate uevents for all DM events
  2016-10-04  7:20 ` Greg KH
@ 2016-10-04 23:39   ` Andy Grover
  2016-10-05  0:40     ` [dm-devel] " Alasdair G Kergon
  0 siblings, 1 reply; 19+ messages in thread
From: Andy Grover @ 2016-10-04 23:39 UTC (permalink / raw)
  To: Greg KH; +Cc: snitzer, dm-devel, linux-kernel

On 10/04/2016 12:20 AM, Greg KH wrote:
> On Mon, Oct 03, 2016 at 12:22:51PM -0700, Andy Grover wrote:
>> Hi Mike and GregKH,
>>
>> I want a way to get devicemapper events without using the DM ioctl,
>> because that requires creating a thread to sleep in the ioctl for each
>> dm device I want events from.
>>
>> It would seem like using uevents and KOBJ_CHANGE would be a good way
>> to do this, but Mike said that the uevent maintainers (Greg that's
>> you?) did not think this was a good idea?
>
> KBOJ_CHANGE is a tricky one.  It has been used for a variety of
> different things, but usually it is used to show that a major change has
> happened with a device like a docking station plug in or out.
>
> How often do DM events happen?  What is triggering them?  How do you
> want to send them to userspace?  Through a sysfs file?  Why not just use
> your own netlink connection?

devicemapper is using uevents for:

a. dm-verity detected corruption
b. dm-multipath: path failed or reinstated
c. dm device renamed
d. there's also some use in md and bcache.

devicemapper uses DM_EVENT ioctl (yuck) for:

1. dm-thin pool data/metadata filling up (hit a threshold)
2. dm-cache is now clean
3. dm-log flushed or log failed
4. dm-raid error detected or sync complete

Instead of using uevent for everything, we could go to a separate 
genetlink for 1-4 instead of making them use uevent like a-d, but we'd 
end up with two different userspace notification techniques. At least to 
me, there doesn't seem to be much technical differentiation between the 
two lists.

(Would we want to then at some point transition any of a-d off of uevent 
and onto genetlink for consistency's sake?)

Thanks -- Regards -- Andy

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dm-devel] [PATCH 0/9] Generate uevents for all DM events
  2016-10-04 23:39   ` Andy Grover
@ 2016-10-05  0:40     ` Alasdair G Kergon
  2016-10-05  6:26       ` Hannes Reinecke
  2016-10-05  6:51       ` Greg KH
  0 siblings, 2 replies; 19+ messages in thread
From: Alasdair G Kergon @ 2016-10-05  0:40 UTC (permalink / raw)
  To: Andy Grover; +Cc: Greg KH, dm-devel, linux-kernel, snitzer

On Tue, Oct 04, 2016 at 04:39:28PM -0700, Andy Grover wrote:
> devicemapper is using uevents for:
> a. dm-verity detected corruption
> b. dm-multipath: path failed or reinstated
> c. dm device renamed
> d. there's also some use in md and bcache.
>
> devicemapper uses DM_EVENT ioctl (yuck) for:
> 1. dm-thin pool data/metadata filling up (hit a threshold)
> 2. dm-cache is now clean
> 3. dm-log flushed or log failed
> 4. dm-raid error detected or sync complete

> there doesn't seem to be much technical differentiation between the  
> two lists.

The distinction in dm is that events in the first category may affect
the availability of the device: they represent major (and hopefully
rare) changes.

Events in the second category are just notifications: no impact on /dev,
no need to trigger udev rules, and their use will continue to be
extended, and (rarely at the moment) could be frequent (which is no
problem for the existing polling-based mechanism).

> Instead of using uevent for everything, we could go to a separate  
> genetlink for 1-4 instead of making them use uevent like a-d, but we'd  
> end up with two different userspace notification techniques.

We see these as two different categories of notifications, and prefer
the greater flexibility a mechanism independent of uevents would
provide.  The team has discussed several alternatives over the years but
didn't make a decision as we've not yet reached a point where we're
straining the existing mechanism too far.

Alasdair

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dm-devel] [PATCH 0/9] Generate uevents for all DM events
  2016-10-05  0:40     ` [dm-devel] " Alasdair G Kergon
@ 2016-10-05  6:26       ` Hannes Reinecke
  2016-10-05  6:51       ` Greg KH
  1 sibling, 0 replies; 19+ messages in thread
From: Hannes Reinecke @ 2016-10-05  6:26 UTC (permalink / raw)
  To: Andy Grover, Greg KH, dm-devel, linux-kernel, snitzer

On 10/05/2016 02:40 AM, Alasdair G Kergon wrote:
> On Tue, Oct 04, 2016 at 04:39:28PM -0700, Andy Grover wrote:
>> devicemapper is using uevents for:
>> a. dm-verity detected corruption
>> b. dm-multipath: path failed or reinstated
>> c. dm device renamed
>> d. there's also some use in md and bcache.
>>
>> devicemapper uses DM_EVENT ioctl (yuck) for:
>> 1. dm-thin pool data/metadata filling up (hit a threshold)
>> 2. dm-cache is now clean
>> 3. dm-log flushed or log failed
>> 4. dm-raid error detected or sync complete
> 
>> there doesn't seem to be much technical differentiation between the  
>> two lists.
> 
> The distinction in dm is that events in the first category may affect
> the availability of the device: they represent major (and hopefully
> rare) changes.
> 
> Events in the second category are just notifications: no impact on /dev,
> no need to trigger udev rules, and their use will continue to be
> extended, and (rarely at the moment) could be frequent (which is no
> problem for the existing polling-based mechanism).
> 
>> Instead of using uevent for everything, we could go to a separate  
>> genetlink for 1-4 instead of making them use uevent like a-d, but we'd  
>> end up with two different userspace notification techniques.
> 
> We see these as two different categories of notifications, and prefer
> the greater flexibility a mechanism independent of uevents would
> provide.  The team has discussed several alternatives over the years but
> didn't make a decision as we've not yet reached a point where we're
> straining the existing mechanism too far.
> 
I would love to remove the dm-event usage from multipathing.
As it stands using dm-events from multipathing is a massive resource
drain (we have to allocate a waiting thread for each device), and we
more-or-less disregard that information anyway as it is an ex-post
notification made _after_ someone modified the table.
Given that in most cases it was actually _us_ doing the reconfiguration
it has very little value.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dm-devel] [PATCH 0/9] Generate uevents for all DM events
  2016-10-05  0:40     ` [dm-devel] " Alasdair G Kergon
  2016-10-05  6:26       ` Hannes Reinecke
@ 2016-10-05  6:51       ` Greg KH
  2016-10-05 17:06         ` Andy Grover
  1 sibling, 1 reply; 19+ messages in thread
From: Greg KH @ 2016-10-05  6:51 UTC (permalink / raw)
  To: Andy Grover, dm-devel, linux-kernel, snitzer

On Wed, Oct 05, 2016 at 01:40:05AM +0100, Alasdair G Kergon wrote:
> On Tue, Oct 04, 2016 at 04:39:28PM -0700, Andy Grover wrote:
> > devicemapper is using uevents for:
> > a. dm-verity detected corruption
> > b. dm-multipath: path failed or reinstated
> > c. dm device renamed
> > d. there's also some use in md and bcache.
> >
> > devicemapper uses DM_EVENT ioctl (yuck) for:
> > 1. dm-thin pool data/metadata filling up (hit a threshold)
> > 2. dm-cache is now clean
> > 3. dm-log flushed or log failed
> > 4. dm-raid error detected or sync complete
> 
> > there doesn't seem to be much technical differentiation between the  
> > two lists.
> 
> The distinction in dm is that events in the first category may affect
> the availability of the device: they represent major (and hopefully
> rare) changes.
> 
> Events in the second category are just notifications: no impact on /dev,
> no need to trigger udev rules, and their use will continue to be
> extended, and (rarely at the moment) could be frequent (which is no
> problem for the existing polling-based mechanism).
> 
> > Instead of using uevent for everything, we could go to a separate  
> > genetlink for 1-4 instead of making them use uevent like a-d, but we'd  
> > end up with two different userspace notification techniques.
> 
> We see these as two different categories of notifications, and prefer
> the greater flexibility a mechanism independent of uevents would
> provide.  The team has discussed several alternatives over the years but
> didn't make a decision as we've not yet reached a point where we're
> straining the existing mechanism too far.

So, no changes need to be made?  I'm confused here, who is wanting this
changed?

greg k-h

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dm-devel] [PATCH 0/9] Generate uevents for all DM events
  2016-10-05  6:51       ` Greg KH
@ 2016-10-05 17:06         ` Andy Grover
  2016-10-05 17:43           ` Alasdair G Kergon
  0 siblings, 1 reply; 19+ messages in thread
From: Andy Grover @ 2016-10-05 17:06 UTC (permalink / raw)
  To: Greg KH, dm-devel, linux-kernel, snitzer

On 10/04/2016 11:51 PM, Greg KH wrote:
> On Wed, Oct 05, 2016 at 01:40:05AM +0100, Alasdair G Kergon wrote:

>> We see these as two different categories of notifications, and prefer
>> the greater flexibility a mechanism independent of uevents would
>> provide.  The team has discussed several alternatives over the years but
>> didn't make a decision as we've not yet reached a point where we're
>> straining the existing mechanism too far.
>
> So, no changes need to be made?  I'm confused here, who is wanting this
> changed?

Hehe, Alasdair and I are both RH but working on different projects. 
We're not the Borg :)

My project *would* like this added sooner, so I'll work on a revised 
patchset that uses netlink instead of uevents, and will also work on a 
revision to uevents.txt that talks about KOBJ_CHANGE and when it should 
and should not be used, as described by agk, to help out the next person.

Thanks -- Regards -- Andy

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dm-devel] [PATCH 0/9] Generate uevents for all DM events
  2016-10-05 17:06         ` Andy Grover
@ 2016-10-05 17:43           ` Alasdair G Kergon
  2016-10-05 22:30             ` Andy Grover
  0 siblings, 1 reply; 19+ messages in thread
From: Alasdair G Kergon @ 2016-10-05 17:43 UTC (permalink / raw)
  To: Andy Grover; +Cc: Greg KH, dm-devel, linux-kernel, snitzer

On Wed, Oct 05, 2016 at 10:06:41AM -0700, Andy Grover wrote:
> My project *would* like this added sooner, so I'll work on a revised  
> patchset that uses netlink instead of uevents, and will also work on a  
> revision to uevents.txt that talks about KOBJ_CHANGE and when it should  
> and should not be used, as described by agk, to help out the next person.

netlink might still be overkill for this at this stage, if it's only
making one thread able to monitor multiple devices efficiently.

Let's find out your requirements and see if there's anything different from
the ones we've debated before.

Alasdair

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dm-devel] [PATCH 0/9] Generate uevents for all DM events
  2016-10-05 17:43           ` Alasdair G Kergon
@ 2016-10-05 22:30             ` Andy Grover
  0 siblings, 0 replies; 19+ messages in thread
From: Andy Grover @ 2016-10-05 22:30 UTC (permalink / raw)
  To: Greg KH, dm-devel, linux-kernel, snitzer

On 10/05/2016 10:43 AM, Alasdair G Kergon wrote:
> On Wed, Oct 05, 2016 at 10:06:41AM -0700, Andy Grover wrote:
>> My project *would* like this added sooner, so I'll work on a revised
>> patchset that uses netlink instead of uevents, and will also work on a
>> revision to uevents.txt that talks about KOBJ_CHANGE and when it should
>> and should not be used, as described by agk, to help out the next person.
>
> netlink might still be overkill for this at this stage, if it's only
> making one thread able to monitor multiple devices efficiently.
>
> Let's find out your requirements and see if there's anything different from
> the ones we've debated before.

Yeah just like Hannes said, I just want to get dm events for multiple dm 
devices without burning a thread for each device. After your 
clarification, netlink seems like the best option and not overkill.

Give me a few days and I can have something for you to review.

Thanks -- Regards -- Andy

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2016-10-05 22:30 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-03 19:22 [PATCH 0/9] Generate uevents for all DM events Andy Grover
2016-10-03 19:22 ` [PATCH 1/9] dm: Do not export dm_send_uevents Andy Grover
2016-10-03 19:22 ` [PATCH 2/9] dm: Move multipath-specific stuff out of dm-uevent.c Andy Grover
2016-10-03 19:22 ` [PATCH 3/9] dm: Inline dm_build_path_uevent into dm_path_uevent Andy Grover
2016-10-03 19:22 ` [PATCH 4/9] dm: Update dm-uevent.txt Andy Grover
2016-10-03 19:22 ` [PATCH 5/9] dm: Rename dm_build_uevent to dm_uevent_build Andy Grover
2016-10-03 19:22 ` [PATCH 6/9] dm: Rename dm_event_add to dm_event_queue Andy Grover
2016-10-03 19:22 ` [PATCH 7/9] dm: Implement dm_uevent_add() Andy Grover
2016-10-03 19:22 ` [PATCH 8/9] dm: Generate uevents for thin targets Andy Grover
2016-10-03 19:23 ` [PATCH 9/9] dm: Generate uevents for other targets Andy Grover
2016-10-03 19:29 ` [PATCH 0/9] Generate uevents for all DM events Mike Snitzer
2016-10-04  7:20 ` Greg KH
2016-10-04 23:39   ` Andy Grover
2016-10-05  0:40     ` [dm-devel] " Alasdair G Kergon
2016-10-05  6:26       ` Hannes Reinecke
2016-10-05  6:51       ` Greg KH
2016-10-05 17:06         ` Andy Grover
2016-10-05 17:43           ` Alasdair G Kergon
2016-10-05 22:30             ` Andy Grover

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).