Linux-Raid Archives on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/4] mdmonitor improvements
@ 2020-09-09  8:31 Mariusz Tkaczyk
  2020-09-09  8:31 ` [PATCH 3/4] mdmonitor: set small delay once Mariusz Tkaczyk
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Mariusz Tkaczyk @ 2020-09-09  8:31 UTC (permalink / raw)
  To: jes; +Cc: linux-raid

This patchset is targeting issues observed across distributions:
- polling on a wrong resource when mdstat is empty
- eventing for external containers
- dealing with udev and mdadm
- quiet fail if other instance is running

Mdmonitor is started automitically if needed by udev. This patchset
introduces mdmonitor stoping if no redundant array presents.

Blazej Kucman (2):
  mdmonitor: set small delay once
  Check if other Monitor instance running before fork.

Mariusz Tkaczyk (2):
  Monitor: refresh mdstat fd after select
  Monitor: stop notifing about containers.

 Monitor.c | 83 ++++++++++++++++++++++++++++++++++++++++---------------
 mdadm.h   |  2 +-
 mdstat.c  | 20 +++++++++++---
 3 files changed, 77 insertions(+), 28 deletions(-)

-- 
2.25.0


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

* [PATCH 3/4] mdmonitor: set small delay once
  2020-09-09  8:31 [PATCH 0/4] mdmonitor improvements Mariusz Tkaczyk
@ 2020-09-09  8:31 ` Mariusz Tkaczyk
  2020-09-15 11:33   ` Nix
  2020-09-09  8:31 ` [PATCH 4/4] Check if other Monitor instance running before fork Mariusz Tkaczyk
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Mariusz Tkaczyk @ 2020-09-09  8:31 UTC (permalink / raw)
  To: jes; +Cc: linux-raid

From: Blazej Kucman <blazej.kucman@intel.com>

If mdmonitor is awakened by event, set small delay once
to deal with udev and mdadm.

Signed-off-by: Blazej Kucman <blazej.kucman@intel.com>
Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@intel.com>
---
 Monitor.c | 14 +++++++++++++-
 mdadm.h   |  2 +-
 mdstat.c  | 18 +++++++++++++++---
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/Monitor.c b/Monitor.c
index aed7a692..0fb4f77d 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -128,6 +128,7 @@ int Monitor(struct mddev_dev *devlist,
 	char *mailfrom;
 	struct alert_info info;
 	struct mddev_ident *mdlist;
+	int delay_for_event = c->delay;
 
 	if (!mailaddr) {
 		mailaddr = conf_get_mailaddr();
@@ -249,7 +250,18 @@ int Monitor(struct mddev_dev *devlist,
 				break;
 			}
 			else {
-				mdstat_wait(c->delay);
+				int wait_result = mdstat_wait(delay_for_event);
+
+				/*
+				 * If mdmonitor is awaken by event, set small delay once
+				 * to deal with udev and mdadm.
+				 */
+				if (wait_result != 0) {
+					if (c->delay > 5)
+						delay_for_event = 5;
+				} else
+					delay_for_event = c->delay;
+
 				mdstat_close();
 			}
 		}
diff --git a/mdadm.h b/mdadm.h
index 399478b8..4961c0f9 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -628,7 +628,7 @@ struct mdstat_ent {
 extern struct mdstat_ent *mdstat_read(int hold, int start);
 extern void mdstat_close(void);
 extern void free_mdstat(struct mdstat_ent *ms);
-extern void mdstat_wait(int seconds);
+extern int mdstat_wait(int seconds);
 extern void mdstat_wait_fd(int fd, const sigset_t *sigmask);
 extern int mddev_busy(char *devnm);
 extern struct mdstat_ent *mdstat_by_component(char *name);
diff --git a/mdstat.c b/mdstat.c
index 48559e64..dd96cca7 100644
--- a/mdstat.c
+++ b/mdstat.c
@@ -302,7 +302,17 @@ void mdstat_close(void)
 	mdstat_fd = -1;
 }
 
-void mdstat_wait(int seconds)
+/*
+ * function: mdstat_wait
+ * Description: Function waits for event on mdstat.
+ * Parameters:
+ *		seconds - timeout for waiting
+ * Returns:
+ *		> 0 - detected event
+ *		0 - timeout
+ *		< 0 - detected error
+ */
+int mdstat_wait(int seconds)
 {
 	fd_set fds;
 	struct timeval tm;
@@ -312,10 +322,12 @@ void mdstat_wait(int seconds)
 		FD_SET(mdstat_fd, &fds);
 		maxfd = mdstat_fd;
 	} else
-		return;
+		return -1;
+
 	tm.tv_sec = seconds;
 	tm.tv_usec = 0;
-	select(maxfd + 1, NULL, NULL, &fds, &tm);
+
+	return select(maxfd + 1, NULL, NULL, &fds, &tm);
 }
 
 void mdstat_wait_fd(int fd, const sigset_t *sigmask)
-- 
2.25.0


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

* [PATCH 4/4] Check if other Monitor instance running before fork.
  2020-09-09  8:31 [PATCH 0/4] mdmonitor improvements Mariusz Tkaczyk
  2020-09-09  8:31 ` [PATCH 3/4] mdmonitor: set small delay once Mariusz Tkaczyk
@ 2020-09-09  8:31 ` Mariusz Tkaczyk
  2020-10-14 15:38   ` Jes Sorensen
  2020-09-29 14:34 ` [PATCH 0/4] mdmonitor improvements Tkaczyk, Mariusz
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Mariusz Tkaczyk @ 2020-09-09  8:31 UTC (permalink / raw)
  To: jes; +Cc: linux-raid

From: Blazej Kucman <blazej.kucman@intel.com>

Make error message visible to the user.

Signed-off-by: Blazej Kucman <blazej.kucman@intel.com>
Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@intel.com>
---
 Monitor.c | 44 ++++++++++++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/Monitor.c b/Monitor.c
index 0fb4f77d..7fd48084 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -63,6 +63,7 @@ struct alert_info {
 };
 static int make_daemon(char *pidfile);
 static int check_one_sharer(int scan);
+static void write_autorebuild_pid(void);
 static void alert(char *event, char *dev, char *disc, struct alert_info *info);
 static int check_array(struct state *st, struct mdstat_ent *mdstat,
 		       int test, struct alert_info *info,
@@ -153,6 +154,11 @@ int Monitor(struct mddev_dev *devlist,
 	info.mailfrom = mailfrom;
 	info.dosyslog = dosyslog;
 
+	if (share){
+		if (check_one_sharer(c->scan))
+			return 1;
+	}
+
 	if (daemonise) {
 		int rv = make_daemon(pidfile);
 		if (rv >= 0)
@@ -160,8 +166,7 @@ int Monitor(struct mddev_dev *devlist,
 	}
 
 	if (share)
-		if (check_one_sharer(c->scan))
-			return 1;
+		write_autorebuild_pid();
 
 	if (devlist == NULL) {
 		mdlist = conf_get_ident(NULL);
@@ -328,8 +333,8 @@ static int check_one_sharer(int scan)
 	int pid;
 	FILE *comm_fp;
 	FILE *fp;
-	char comm_path[100];
-	char path[100];
+	char comm_path[PATH_MAX];
+	char path[PATH_MAX];
 	char comm[20];
 
 	sprintf(path, "%s/autorebuild.pid", MDMON_DIR);
@@ -356,21 +361,28 @@ static int check_one_sharer(int scan)
 		}
 		fclose(fp);
 	}
-	if (scan) {
-		if (mkdir(MDMON_DIR, S_IRWXU) < 0 && errno != EEXIST) {
+	return 0;
+}
+
+static void write_autorebuild_pid()
+{
+	char path[PATH_MAX];
+	int pid;
+	FILE *fp;
+	sprintf(path, "%s/autorebuild.pid", MDMON_DIR);
+
+	if (mkdir(MDMON_DIR, S_IRWXU) < 0 && errno != EEXIST) {
+		pr_err("Can't create autorebuild.pid file\n");
+	} else {
+		fp = fopen(path, "w");
+		if (!fp)
 			pr_err("Can't create autorebuild.pid file\n");
-		} else {
-			fp = fopen(path, "w");
-			if (!fp)
-				pr_err("Cannot create autorebuild.pidfile\n");
-			else {
-				pid = getpid();
-				fprintf(fp, "%d\n", pid);
-				fclose(fp);
-			}
+		else {
+			pid = getpid();
+			fprintf(fp, "%d\n", pid);
+			fclose(fp);
 		}
 	}
-	return 0;
 }
 
 static void alert(char *event, char *dev, char *disc, struct alert_info *info)
-- 
2.25.0


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

* Re: [PATCH 3/4] mdmonitor: set small delay once
  2020-09-09  8:31 ` [PATCH 3/4] mdmonitor: set small delay once Mariusz Tkaczyk
@ 2020-09-15 11:33   ` Nix
  2020-10-14 15:35     ` Jes Sorensen
  0 siblings, 1 reply; 10+ messages in thread
From: Nix @ 2020-09-15 11:33 UTC (permalink / raw)
  To: Mariusz Tkaczyk; +Cc: jes, linux-raid

On 9 Sep 2020, Mariusz Tkaczyk uttered the following:

> +				/*
> +				 * If mdmonitor is awaken by event, set small delay once
> +				 * to deal with udev and mdadm.
> +				 */
> +				if (wait_result != 0) {
> +					if (c->delay > 5)
> +						delay_for_event = 5;
> +				} else
> +					delay_for_event = c->delay;

This is racy: if any delay is needed, any finite delay value will
now and then be too short.

I think this should be fixed by arranging for mdmonitor to be signalled
when udev or whatever has finished whatever it's doing. (udev has lots
of ways it could be asked to do that.)

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

* RE: [PATCH 0/4] mdmonitor improvements
  2020-09-09  8:31 [PATCH 0/4] mdmonitor improvements Mariusz Tkaczyk
  2020-09-09  8:31 ` [PATCH 3/4] mdmonitor: set small delay once Mariusz Tkaczyk
  2020-09-09  8:31 ` [PATCH 4/4] Check if other Monitor instance running before fork Mariusz Tkaczyk
@ 2020-09-29 14:34 ` Tkaczyk, Mariusz
  2020-10-14 15:30   ` Jes Sorensen
       [not found] ` <20200909083120.10396-2-mariusz.tkaczyk@intel.com>
       [not found] ` <20200909083120.10396-3-mariusz.tkaczyk@intel.com>
  4 siblings, 1 reply; 10+ messages in thread
From: Tkaczyk, Mariusz @ 2020-09-29 14:34 UTC (permalink / raw)
  To: jes; +Cc: linux-raid

Hi Jes,
Do you receive whole patchset?
If not let me know. I will send it again.
I don't know why two patches were lost, I cannot find them anywhere on linux-raid.

Thanks,
Mariusz

-----Original Message-----
From: linux-raid-owner@vger.kernel.org <linux-raid-owner@vger.kernel.org> On Behalf Of Mariusz Tkaczyk
Sent: Wednesday, September 9, 2020 10:31 AM
To: jes@trained-monkey.org
Cc: linux-raid@vger.kernel.org
Subject: [PATCH 0/4] mdmonitor improvements

This patchset is targeting issues observed across distributions:
- polling on a wrong resource when mdstat is empty
- eventing for external containers
- dealing with udev and mdadm
- quiet fail if other instance is running

Mdmonitor is started automitically if needed by udev. This patchset introduces mdmonitor stoping if no redundant array presents.

Blazej Kucman (2):
  mdmonitor: set small delay once
  Check if other Monitor instance running before fork.

Mariusz Tkaczyk (2):
  Monitor: refresh mdstat fd after select
  Monitor: stop notifing about containers.

 Monitor.c | 83 ++++++++++++++++++++++++++++++++++++++++---------------
 mdadm.h   |  2 +-
 mdstat.c  | 20 +++++++++++---
 3 files changed, 77 insertions(+), 28 deletions(-)

--
2.25.0


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

* Re: [PATCH 0/4] mdmonitor improvements
  2020-09-29 14:34 ` [PATCH 0/4] mdmonitor improvements Tkaczyk, Mariusz
@ 2020-10-14 15:30   ` Jes Sorensen
  0 siblings, 0 replies; 10+ messages in thread
From: Jes Sorensen @ 2020-10-14 15:30 UTC (permalink / raw)
  To: Tkaczyk, Mariusz; +Cc: linux-raid

On 9/29/20 10:34 AM, Tkaczyk, Mariusz wrote:
> Hi Jes,
> Do you receive whole patchset?
> If not let me know. I will send it again.
> I don't know why two patches were lost, I cannot find them anywhere on linux-raid.
> 
> Thanks,
> Mariusz

Hi Mariusz,

I got four patches. Sorry I have been completely swamped with other
stuff at work the last couple of months, so I have gotten behind on this.

I'll try and go through them now.

Thanks,
Jes

> -----Original Message-----
> From: linux-raid-owner@vger.kernel.org <linux-raid-owner@vger.kernel.org> On Behalf Of Mariusz Tkaczyk
> Sent: Wednesday, September 9, 2020 10:31 AM
> To: jes@trained-monkey.org
> Cc: linux-raid@vger.kernel.org
> Subject: [PATCH 0/4] mdmonitor improvements
> 
> This patchset is targeting issues observed across distributions:
> - polling on a wrong resource when mdstat is empty
> - eventing for external containers
> - dealing with udev and mdadm
> - quiet fail if other instance is running
> 
> Mdmonitor is started automitically if needed by udev. This patchset introduces mdmonitor stoping if no redundant array presents.
> 
> Blazej Kucman (2):
>   mdmonitor: set small delay once
>   Check if other Monitor instance running before fork.
> 
> Mariusz Tkaczyk (2):
>   Monitor: refresh mdstat fd after select
>   Monitor: stop notifing about containers.
> 
>  Monitor.c | 83 ++++++++++++++++++++++++++++++++++++++++---------------
>  mdadm.h   |  2 +-
>  mdstat.c  | 20 +++++++++++---
>  3 files changed, 77 insertions(+), 28 deletions(-)
> 
> --
> 2.25.0
> 


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

* Re: [PATCH 1/4] Monitor: refresh mdstat fd after select
       [not found] ` <20200909083120.10396-2-mariusz.tkaczyk@intel.com>
@ 2020-10-14 15:34   ` Jes Sorensen
  0 siblings, 0 replies; 10+ messages in thread
From: Jes Sorensen @ 2020-10-14 15:34 UTC (permalink / raw)
  To: Mariusz Tkaczyk; +Cc: linux-raid

On 9/9/20 4:31 AM, Mariusz Tkaczyk wrote:
> After 52209d6ee118 ("Monitor: release /proc/mdstat fd when no arrays
> present") mdstat fd is closed if mdstat is empty or cannot be opened.
> It causes that monitor is not able to select on mdstat. Select
> doesn't fail because it gets valid descriptor to a different resource.
> As a result any new event will be unnoticed until timeout (delay).
> 
> Refresh mdstat after wake up, don't poll on wrong resource.
> 
> Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@intel.com>
> ---
>  Monitor.c | 6 +++---
>  mdstat.c  | 4 ++--
>  2 files changed, 5 insertions(+), 5 deletions(-)

Applied!

Thanks,
Jes


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

* Re: [PATCH 2/4] Monitor: stop notifing about containers.
       [not found] ` <20200909083120.10396-3-mariusz.tkaczyk@intel.com>
@ 2020-10-14 15:34   ` Jes Sorensen
  0 siblings, 0 replies; 10+ messages in thread
From: Jes Sorensen @ 2020-10-14 15:34 UTC (permalink / raw)
  To: Mariusz Tkaczyk; +Cc: linux-raid

On 9/9/20 4:31 AM, Mariusz Tkaczyk wrote:
> Stop reporting any events from container but still track them,
> it is important for spare migration.
> Stop mdmonitor if no redundant array is presented in mdstat.
> There is nothing to follow.
> 
> Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@intel.com>
> ---
>  Monitor.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)

Applied!

Thanks,
Jes


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

* Re: [PATCH 3/4] mdmonitor: set small delay once
  2020-09-15 11:33   ` Nix
@ 2020-10-14 15:35     ` Jes Sorensen
  0 siblings, 0 replies; 10+ messages in thread
From: Jes Sorensen @ 2020-10-14 15:35 UTC (permalink / raw)
  To: Nix, Mariusz Tkaczyk; +Cc: linux-raid

On 9/15/20 7:33 AM, Nix wrote:
> On 9 Sep 2020, Mariusz Tkaczyk uttered the following:
> 
>> +				/*
>> +				 * If mdmonitor is awaken by event, set small delay once
>> +				 * to deal with udev and mdadm.
>> +				 */
>> +				if (wait_result != 0) {
>> +					if (c->delay > 5)
>> +						delay_for_event = 5;
>> +				} else
>> +					delay_for_event = c->delay;
> 
> This is racy: if any delay is needed, any finite delay value will
> now and then be too short.
> 
> I think this should be fixed by arranging for mdmonitor to be signalled
> when udev or whatever has finished whatever it's doing. (udev has lots
> of ways it could be asked to do that.)
> 

Hi

I have applied this patch for now, since it is better than what we had.
However, I agree it would be better to do this using udev or other
signalling, so please have a look at that.

Cheers,
Jes

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

* Re: [PATCH 4/4] Check if other Monitor instance running before fork.
  2020-09-09  8:31 ` [PATCH 4/4] Check if other Monitor instance running before fork Mariusz Tkaczyk
@ 2020-10-14 15:38   ` Jes Sorensen
  0 siblings, 0 replies; 10+ messages in thread
From: Jes Sorensen @ 2020-10-14 15:38 UTC (permalink / raw)
  To: Mariusz Tkaczyk; +Cc: linux-raid

On 9/9/20 4:31 AM, Mariusz Tkaczyk wrote:
> From: Blazej Kucman <blazej.kucman@intel.com>
> 
> Make error message visible to the user.
> 
> Signed-off-by: Blazej Kucman <blazej.kucman@intel.com>
> Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@intel.com>
> ---
>  Monitor.c | 44 ++++++++++++++++++++++++++++----------------
>  1 file changed, 28 insertions(+), 16 deletions(-)

Applied!

Thanks,
Jes



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

end of thread, back to index

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-09  8:31 [PATCH 0/4] mdmonitor improvements Mariusz Tkaczyk
2020-09-09  8:31 ` [PATCH 3/4] mdmonitor: set small delay once Mariusz Tkaczyk
2020-09-15 11:33   ` Nix
2020-10-14 15:35     ` Jes Sorensen
2020-09-09  8:31 ` [PATCH 4/4] Check if other Monitor instance running before fork Mariusz Tkaczyk
2020-10-14 15:38   ` Jes Sorensen
2020-09-29 14:34 ` [PATCH 0/4] mdmonitor improvements Tkaczyk, Mariusz
2020-10-14 15:30   ` Jes Sorensen
     [not found] ` <20200909083120.10396-2-mariusz.tkaczyk@intel.com>
2020-10-14 15:34   ` [PATCH 1/4] Monitor: refresh mdstat fd after select Jes Sorensen
     [not found] ` <20200909083120.10396-3-mariusz.tkaczyk@intel.com>
2020-10-14 15:34   ` [PATCH 2/4] Monitor: stop notifing about containers Jes Sorensen

Linux-Raid Archives on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-raid/0 linux-raid/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-raid linux-raid/ https://lore.kernel.org/linux-raid \
		linux-raid@vger.kernel.org
	public-inbox-index linux-raid

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-raid


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git