linux-raid.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] md: use TASK_IDLE instead of blocking signals
@ 2017-10-18 23:01 Mikulas Patocka
  2017-10-18 23:22 ` NeilBrown
  2017-10-19  3:48 ` Shaohua Li
  0 siblings, 2 replies; 3+ messages in thread
From: Mikulas Patocka @ 2017-10-18 23:01 UTC (permalink / raw)
  To: Shaohua Li, NeilBrown; +Cc: linux-raid

Hi - I submit this patch for the next merge window:


Some times ago, I made a patch f9c79bc05a2a that blocks signals around the
schedule() calls in MD. The MD subsystem needs to do an uninterruptible
sleep that is not accounted in load average - so we block signals and use
interruptible sleep.

The kernel has a special TASK_IDLE state for this purpose, so we can use
it instead of blocking signals. This patch doesn't fix any bug, it just
makes the code simpler.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 drivers/md/raid1.c |    7 +------
 drivers/md/raid5.c |    7 +------
 2 files changed, 2 insertions(+), 12 deletions(-)

Index: linux-2.6/drivers/md/raid1.c
===================================================================
--- linux-2.6.orig/drivers/md/raid1.c
+++ linux-2.6/drivers/md/raid1.c
@@ -37,7 +37,6 @@
 #include <linux/module.h>
 #include <linux/seq_file.h>
 #include <linux/ratelimit.h>
-#include <linux/sched/signal.h>
 
 #include <trace/events/block.h>
 
@@ -1322,9 +1321,8 @@ static void raid1_write_request(struct m
 		 */
 		DEFINE_WAIT(w);
 		for (;;) {
-			sigset_t full, old;
 			prepare_to_wait(&conf->wait_barrier,
-					&w, TASK_INTERRUPTIBLE);
+					&w, TASK_IDLE);
 			if (bio_end_sector(bio) <= mddev->suspend_lo ||
 			    bio->bi_iter.bi_sector >= mddev->suspend_hi ||
 			    (mddev_is_clustered(mddev) &&
@@ -1332,10 +1330,7 @@ static void raid1_write_request(struct m
 				     bio->bi_iter.bi_sector,
 				     bio_end_sector(bio))))
 				break;
-			sigfillset(&full);
-			sigprocmask(SIG_BLOCK, &full, &old);
 			schedule();
-			sigprocmask(SIG_SETMASK, &old, NULL);
 		}
 		finish_wait(&conf->wait_barrier, &w);
 	}
Index: linux-2.6/drivers/md/raid5.c
===================================================================
--- linux-2.6.orig/drivers/md/raid5.c
+++ linux-2.6/drivers/md/raid5.c
@@ -55,7 +55,6 @@
 #include <linux/ratelimit.h>
 #include <linux/nodemask.h>
 #include <linux/flex_array.h>
-#include <linux/sched/signal.h>
 
 #include <trace/events/block.h>
 #include <linux/list_sort.h>
@@ -5691,14 +5690,10 @@ static bool raid5_make_request(struct md
 				 * wait.
 				 */
 				prepare_to_wait(&conf->wait_for_overlap,
-						&w, TASK_INTERRUPTIBLE);
+						&w, TASK_IDLE);
 				if (logical_sector >= mddev->suspend_lo &&
 				    logical_sector < mddev->suspend_hi) {
-					sigset_t full, old;
-					sigfillset(&full);
-					sigprocmask(SIG_BLOCK, &full, &old);
 					schedule();
-					sigprocmask(SIG_SETMASK, &old, NULL);
 					do_prepare = true;
 				}
 				goto retry;

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

* Re: [PATCH] md: use TASK_IDLE instead of blocking signals
  2017-10-18 23:01 [PATCH] md: use TASK_IDLE instead of blocking signals Mikulas Patocka
@ 2017-10-18 23:22 ` NeilBrown
  2017-10-19  3:48 ` Shaohua Li
  1 sibling, 0 replies; 3+ messages in thread
From: NeilBrown @ 2017-10-18 23:22 UTC (permalink / raw)
  To: Mikulas Patocka, Shaohua Li; +Cc: linux-raid

[-- Attachment #1: Type: text/plain, Size: 3051 bytes --]

On Wed, Oct 18 2017, Mikulas Patocka wrote:

> Hi - I submit this patch for the next merge window:
>
>
> Some times ago, I made a patch f9c79bc05a2a that blocks signals around the
> schedule() calls in MD. The MD subsystem needs to do an uninterruptible
> sleep that is not accounted in load average - so we block signals and use
> interruptible sleep.
>
> The kernel has a special TASK_IDLE state for this purpose, so we can use
> it instead of blocking signals. This patch doesn't fix any bug, it just
> makes the code simpler.
>
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

Thanks for doing this!  It has been on my mind to look at exactly this
possibility for a while but it never quite happened.  Now I don't need
to :-)

Acked-by: NeilBrown <neilb@suse.com>

Thanks,
NeilBrown


>
> ---
>  drivers/md/raid1.c |    7 +------
>  drivers/md/raid5.c |    7 +------
>  2 files changed, 2 insertions(+), 12 deletions(-)
>
> Index: linux-2.6/drivers/md/raid1.c
> ===================================================================
> --- linux-2.6.orig/drivers/md/raid1.c
> +++ linux-2.6/drivers/md/raid1.c
> @@ -37,7 +37,6 @@
>  #include <linux/module.h>
>  #include <linux/seq_file.h>
>  #include <linux/ratelimit.h>
> -#include <linux/sched/signal.h>
>  
>  #include <trace/events/block.h>
>  
> @@ -1322,9 +1321,8 @@ static void raid1_write_request(struct m
>  		 */
>  		DEFINE_WAIT(w);
>  		for (;;) {
> -			sigset_t full, old;
>  			prepare_to_wait(&conf->wait_barrier,
> -					&w, TASK_INTERRUPTIBLE);
> +					&w, TASK_IDLE);
>  			if (bio_end_sector(bio) <= mddev->suspend_lo ||
>  			    bio->bi_iter.bi_sector >= mddev->suspend_hi ||
>  			    (mddev_is_clustered(mddev) &&
> @@ -1332,10 +1330,7 @@ static void raid1_write_request(struct m
>  				     bio->bi_iter.bi_sector,
>  				     bio_end_sector(bio))))
>  				break;
> -			sigfillset(&full);
> -			sigprocmask(SIG_BLOCK, &full, &old);
>  			schedule();
> -			sigprocmask(SIG_SETMASK, &old, NULL);
>  		}
>  		finish_wait(&conf->wait_barrier, &w);
>  	}
> Index: linux-2.6/drivers/md/raid5.c
> ===================================================================
> --- linux-2.6.orig/drivers/md/raid5.c
> +++ linux-2.6/drivers/md/raid5.c
> @@ -55,7 +55,6 @@
>  #include <linux/ratelimit.h>
>  #include <linux/nodemask.h>
>  #include <linux/flex_array.h>
> -#include <linux/sched/signal.h>
>  
>  #include <trace/events/block.h>
>  #include <linux/list_sort.h>
> @@ -5691,14 +5690,10 @@ static bool raid5_make_request(struct md
>  				 * wait.
>  				 */
>  				prepare_to_wait(&conf->wait_for_overlap,
> -						&w, TASK_INTERRUPTIBLE);
> +						&w, TASK_IDLE);
>  				if (logical_sector >= mddev->suspend_lo &&
>  				    logical_sector < mddev->suspend_hi) {
> -					sigset_t full, old;
> -					sigfillset(&full);
> -					sigprocmask(SIG_BLOCK, &full, &old);
>  					schedule();
> -					sigprocmask(SIG_SETMASK, &old, NULL);
>  					do_prepare = true;
>  				}
>  				goto retry;

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

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

* Re: [PATCH] md: use TASK_IDLE instead of blocking signals
  2017-10-18 23:01 [PATCH] md: use TASK_IDLE instead of blocking signals Mikulas Patocka
  2017-10-18 23:22 ` NeilBrown
@ 2017-10-19  3:48 ` Shaohua Li
  1 sibling, 0 replies; 3+ messages in thread
From: Shaohua Li @ 2017-10-19  3:48 UTC (permalink / raw)
  To: Mikulas Patocka; +Cc: Shaohua Li, NeilBrown, linux-raid

On Wed, Oct 18, 2017 at 07:01:11PM -0400, Mikulas Patocka wrote:
> Hi - I submit this patch for the next merge window:
> 
> 
> Some times ago, I made a patch f9c79bc05a2a that blocks signals around the
> schedule() calls in MD. The MD subsystem needs to do an uninterruptible
> sleep that is not accounted in load average - so we block signals and use
> interruptible sleep.
> 
> The kernel has a special TASK_IDLE state for this purpose, so we can use
> it instead of blocking signals. This patch doesn't fix any bug, it just
> makes the code simpler.

thanks, applied! 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> 
> ---
>  drivers/md/raid1.c |    7 +------
>  drivers/md/raid5.c |    7 +------
>  2 files changed, 2 insertions(+), 12 deletions(-)
> 
> Index: linux-2.6/drivers/md/raid1.c
> ===================================================================
> --- linux-2.6.orig/drivers/md/raid1.c
> +++ linux-2.6/drivers/md/raid1.c
> @@ -37,7 +37,6 @@
>  #include <linux/module.h>
>  #include <linux/seq_file.h>
>  #include <linux/ratelimit.h>
> -#include <linux/sched/signal.h>
>  
>  #include <trace/events/block.h>
>  
> @@ -1322,9 +1321,8 @@ static void raid1_write_request(struct m
>  		 */
>  		DEFINE_WAIT(w);
>  		for (;;) {
> -			sigset_t full, old;
>  			prepare_to_wait(&conf->wait_barrier,
> -					&w, TASK_INTERRUPTIBLE);
> +					&w, TASK_IDLE);
>  			if (bio_end_sector(bio) <= mddev->suspend_lo ||
>  			    bio->bi_iter.bi_sector >= mddev->suspend_hi ||
>  			    (mddev_is_clustered(mddev) &&
> @@ -1332,10 +1330,7 @@ static void raid1_write_request(struct m
>  				     bio->bi_iter.bi_sector,
>  				     bio_end_sector(bio))))
>  				break;
> -			sigfillset(&full);
> -			sigprocmask(SIG_BLOCK, &full, &old);
>  			schedule();
> -			sigprocmask(SIG_SETMASK, &old, NULL);
>  		}
>  		finish_wait(&conf->wait_barrier, &w);
>  	}
> Index: linux-2.6/drivers/md/raid5.c
> ===================================================================
> --- linux-2.6.orig/drivers/md/raid5.c
> +++ linux-2.6/drivers/md/raid5.c
> @@ -55,7 +55,6 @@
>  #include <linux/ratelimit.h>
>  #include <linux/nodemask.h>
>  #include <linux/flex_array.h>
> -#include <linux/sched/signal.h>
>  
>  #include <trace/events/block.h>
>  #include <linux/list_sort.h>
> @@ -5691,14 +5690,10 @@ static bool raid5_make_request(struct md
>  				 * wait.
>  				 */
>  				prepare_to_wait(&conf->wait_for_overlap,
> -						&w, TASK_INTERRUPTIBLE);
> +						&w, TASK_IDLE);
>  				if (logical_sector >= mddev->suspend_lo &&
>  				    logical_sector < mddev->suspend_hi) {
> -					sigset_t full, old;
> -					sigfillset(&full);
> -					sigprocmask(SIG_BLOCK, &full, &old);
>  					schedule();
> -					sigprocmask(SIG_SETMASK, &old, NULL);
>  					do_prepare = true;
>  				}
>  				goto retry;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2017-10-19  3:48 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-18 23:01 [PATCH] md: use TASK_IDLE instead of blocking signals Mikulas Patocka
2017-10-18 23:22 ` NeilBrown
2017-10-19  3:48 ` Shaohua Li

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).