Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Peter Zijlstra]

On Fri, Sep 23, 2016 at 08:42:51AM -0400, Mike Snitzer wrote:
> On Fri, Sep 23 2016 at  8:26am -0400,
> Peter Zijlstra  wrote:
> 
> > On Fri, Sep 23, 2016 at 02:17:10PM +0200, Mike Galbraith wrote:
> > > On Fri, 2016-09-23 at 10:00 +0200, Thomas Gleixner wrote:
> > > > On Fri, 23 Sep 2016, Peter Zijlstra wrote:
> > > 
> > > > > Is anybody still using PREEMPT_NONE? Most workloads also care about
> > > > > latency to some extend. Lots of code has explicit cond_resched() and
> > > > > doesn't worry.
> > > > 
> > > > Dunno. But I bet there are workloads which love it.
> > > 
> > > SUSE definitely uses it.  I had presumed that was enterprise standard.
> > 
> > Hmm, I thought most distros defaulted to PREEMPT_VOLUNTARY.
> 
> So what is the concensus on this?  Switch dm-bufio's cond_resched calls
> (in peter's patch) to might_sleep()?  Or continue using cond_resched but
> fix cond_resched to do the might_sleep() equivalent if PREEMPT_NONE?

I'd go with the one I posted and look again if ever a performance issue
shows up.

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Peter Zijlstra]

On Fri, Sep 23, 2016 at 08:42:51AM -0400, Mike Snitzer wrote:
> On Fri, Sep 23 2016 at  8:26am -0400,
> Peter Zijlstra  wrote:
> 
> > On Fri, Sep 23, 2016 at 02:17:10PM +0200, Mike Galbraith wrote:
> > > On Fri, 2016-09-23 at 10:00 +0200, Thomas Gleixner wrote:
> > > > On Fri, 23 Sep 2016, Peter Zijlstra wrote:
> > > 
> > > > > Is anybody still using PREEMPT_NONE? Most workloads also care about
> > > > > latency to some extend. Lots of code has explicit cond_resched() and
> > > > > doesn't worry.
> > > > 
> > > > Dunno. But I bet there are workloads which love it.
> > > 
> > > SUSE definitely uses it.  I had presumed that was enterprise standard.
> > 
> > Hmm, I thought most distros defaulted to PREEMPT_VOLUNTARY.
> 
> So what is the concensus on this?  Switch dm-bufio's cond_resched calls
> (in peter's patch) to might_sleep()?  Or continue using cond_resched but
> fix cond_resched to do the might_sleep() equivalent if PREEMPT_NONE?

I'd go with the one I posted and look again if ever a performance issue
shows up.

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Mike Snitzer]

On Fri, Sep 23 2016 at  8:26am -0400,
Peter Zijlstra  wrote:

> On Fri, Sep 23, 2016 at 02:17:10PM +0200, Mike Galbraith wrote:
> > On Fri, 2016-09-23 at 10:00 +0200, Thomas Gleixner wrote:
> > > On Fri, 23 Sep 2016, Peter Zijlstra wrote:
> > 
> > > > Is anybody still using PREEMPT_NONE? Most workloads also care about
> > > > latency to some extend. Lots of code has explicit cond_resched() and
> > > > doesn't worry.
> > > 
> > > Dunno. But I bet there are workloads which love it.
> > 
> > SUSE definitely uses it.  I had presumed that was enterprise standard.
> 
> Hmm, I thought most distros defaulted to PREEMPT_VOLUNTARY.

So what is the concensus on this?  Switch dm-bufio's cond_resched calls
(in peter's patch) to might_sleep()?  Or continue using cond_resched but
fix cond_resched to do the might_sleep() equivalent if PREEMPT_NONE?

Mike

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Mike Snitzer]

On Fri, Sep 23 2016 at  8:26am -0400,
Peter Zijlstra  wrote:

> On Fri, Sep 23, 2016 at 02:17:10PM +0200, Mike Galbraith wrote:
> > On Fri, 2016-09-23 at 10:00 +0200, Thomas Gleixner wrote:
> > > On Fri, 23 Sep 2016, Peter Zijlstra wrote:
> > 
> > > > Is anybody still using PREEMPT_NONE? Most workloads also care about
> > > > latency to some extend. Lots of code has explicit cond_resched() and
> > > > doesn't worry.
> > > 
> > > Dunno. But I bet there are workloads which love it.
> > 
> > SUSE definitely uses it.  I had presumed that was enterprise standard.
> 
> Hmm, I thought most distros defaulted to PREEMPT_VOLUNTARY.

So what is the concensus on this?  Switch dm-bufio's cond_resched calls
(in peter's patch) to might_sleep()?  Or continue using cond_resched but
fix cond_resched to do the might_sleep() equivalent if PREEMPT_NONE?

Mike

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Mike Galbraith]

On Fri, 2016-09-23 at 14:26 +0200, Peter Zijlstra wrote:
> On Fri, Sep 23, 2016 at 02:17:10PM +0200, Mike Galbraith wrote:
> > On Fri, 2016-09-23 at 10:00 +0200, Thomas Gleixner wrote:
> > > On Fri, 23 Sep 2016, Peter Zijlstra wrote:
> > 
> > > > Is anybody still using PREEMPT_NONE? Most workloads also care
> > > > about
> > > > latency to some extend. Lots of code has explicit
> > > > cond_resched() and
> > > > doesn't worry.
> > > 
> > > Dunno. But I bet there are workloads which love it.
> > 
> > SUSE definitely uses it.  I had presumed that was enterprise
> > standard.
> 
> Hmm, I thought most distros defaulted to PREEMPT_VOLUNTARY.

I use PREEMPT_VOLUNTARY for my desktop, that offering much better
performance than the PREEMPT desktop targeted kernels (ick), but
workhorses run PREEMPT_NONE for maximum throughput.

-Mike

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Mike Galbraith]

On Fri, 2016-09-23 at 14:26 +0200, Peter Zijlstra wrote:
> On Fri, Sep 23, 2016 at 02:17:10PM +0200, Mike Galbraith wrote:
> > On Fri, 2016-09-23 at 10:00 +0200, Thomas Gleixner wrote:
> > > On Fri, 23 Sep 2016, Peter Zijlstra wrote:
> > 
> > > > Is anybody still using PREEMPT_NONE? Most workloads also care
> > > > about
> > > > latency to some extend. Lots of code has explicit
> > > > cond_resched() and
> > > > doesn't worry.
> > > 
> > > Dunno. But I bet there are workloads which love it.
> > 
> > SUSE definitely uses it.  I had presumed that was enterprise
> > standard.
> 
> Hmm, I thought most distros defaulted to PREEMPT_VOLUNTARY.

I use PREEMPT_VOLUNTARY for my desktop, that offering much better
performance than the PREEMPT desktop targeted kernels (ick), but
workhorses run PREEMPT_NONE for maximum throughput.

-Mike

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Peter Zijlstra]

On Fri, Sep 23, 2016 at 02:17:10PM +0200, Mike Galbraith wrote:
> On Fri, 2016-09-23 at 10:00 +0200, Thomas Gleixner wrote:
> > On Fri, 23 Sep 2016, Peter Zijlstra wrote:
> 
> > > Is anybody still using PREEMPT_NONE? Most workloads also care about
> > > latency to some extend. Lots of code has explicit cond_resched() and
> > > doesn't worry.
> > 
> > Dunno. But I bet there are workloads which love it.
> 
> SUSE definitely uses it.  I had presumed that was enterprise standard.

Hmm, I thought most distros defaulted to PREEMPT_VOLUNTARY.

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Peter Zijlstra]

On Fri, Sep 23, 2016 at 02:17:10PM +0200, Mike Galbraith wrote:
> On Fri, 2016-09-23 at 10:00 +0200, Thomas Gleixner wrote:
> > On Fri, 23 Sep 2016, Peter Zijlstra wrote:
> 
> > > Is anybody still using PREEMPT_NONE? Most workloads also care about
> > > latency to some extend. Lots of code has explicit cond_resched() and
> > > doesn't worry.
> > 
> > Dunno. But I bet there are workloads which love it.
> 
> SUSE definitely uses it.  I had presumed that was enterprise standard.

Hmm, I thought most distros defaulted to PREEMPT_VOLUNTARY.

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Mike Galbraith]

On Fri, 2016-09-23 at 10:00 +0200, Thomas Gleixner wrote:
> On Fri, 23 Sep 2016, Peter Zijlstra wrote:

> > Is anybody still using PREEMPT_NONE? Most workloads also care about
> > latency to some extend. Lots of code has explicit cond_resched() and
> > doesn't worry.
> 
> Dunno. But I bet there are workloads which love it.

SUSE definitely uses it.  I had presumed that was enterprise standard.

-Mike

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Mike Galbraith]

On Fri, 2016-09-23 at 10:00 +0200, Thomas Gleixner wrote:
> On Fri, 23 Sep 2016, Peter Zijlstra wrote:

> > Is anybody still using PREEMPT_NONE? Most workloads also care about
> > latency to some extend. Lots of code has explicit cond_resched() and
> > doesn't worry.
> 
> Dunno. But I bet there are workloads which love it.

SUSE definitely uses it.  I had presumed that was enterprise standard.

-Mike

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Ingo Molnar]

* Thomas Gleixner  wrote:

> On Fri, 23 Sep 2016, Peter Zijlstra wrote:
> > On Fri, Sep 23, 2016 at 10:00:37AM +0200, Thomas Gleixner wrote:
> > > On Fri, 23 Sep 2016, Peter Zijlstra wrote:
> > > > It is, might_sleep() implies might_resched(). In fact, that's all what
> > > > PREEMPT_VOLUNTARY is, make the might_sleep() debug test imply a resched
> > > > point.
> > > 
> > > Grr, how intuitive - NOT!
> > 
> > No, it actually makes sense. Because you 'obviously' only call
> > might_sleep() in contexts that should be able to sleep (if not, it'll
> > holler). So they're already placed right for preemption.
> 
> I disagree. might_sleep() is commonly known as a debug mechanism and it
> existed before the preemption stuff went in. So the easy way to sprinkle
> preemption points into the kernel was to hijack might_sleep(). I know it's
> historical, but that doesnt make it any more intuitive.

If we rename it to might_as_well_sleep() it becomes more intuitive! ;-)

Thanks,

Ingo

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Ingo Molnar]

* Thomas Gleixner  wrote:

> On Fri, 23 Sep 2016, Peter Zijlstra wrote:
> > On Fri, Sep 23, 2016 at 10:00:37AM +0200, Thomas Gleixner wrote:
> > > On Fri, 23 Sep 2016, Peter Zijlstra wrote:
> > > > It is, might_sleep() implies might_resched(). In fact, that's all what
> > > > PREEMPT_VOLUNTARY is, make the might_sleep() debug test imply a resched
> > > > point.
> > > 
> > > Grr, how intuitive - NOT!
> > 
> > No, it actually makes sense. Because you 'obviously' only call
> > might_sleep() in contexts that should be able to sleep (if not, it'll
> > holler). So they're already placed right for preemption.
> 
> I disagree. might_sleep() is commonly known as a debug mechanism and it
> existed before the preemption stuff went in. So the easy way to sprinkle
> preemption points into the kernel was to hijack might_sleep(). I know it's
> historical, but that doesnt make it any more intuitive.

If we rename it to might_as_well_sleep() it becomes more intuitive! ;-)

Thanks,

Ingo

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Thomas Gleixner]

On Fri, 23 Sep 2016, Peter Zijlstra wrote:
> On Fri, Sep 23, 2016 at 10:00:37AM +0200, Thomas Gleixner wrote:
> > On Fri, 23 Sep 2016, Peter Zijlstra wrote:
> > > It is, might_sleep() implies might_resched(). In fact, that's all what
> > > PREEMPT_VOLUNTARY is, make the might_sleep() debug test imply a resched
> > > point.
> > 
> > Grr, how intuitive - NOT!
> 
> No, it actually makes sense. Because you 'obviously' only call
> might_sleep() in contexts that should be able to sleep (if not, it'll
> holler). So they're already placed right for preemption.

I disagree. might_sleep() is commonly known as a debug mechanism and it
existed before the preemption stuff went in. So the easy way to sprinkle
preemption points into the kernel was to hijack might_sleep(). I know it's
historical, but that doesnt make it any more intuitive.

Thanks,

tglx

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Thomas Gleixner]

On Fri, 23 Sep 2016, Peter Zijlstra wrote:
> On Fri, Sep 23, 2016 at 10:00:37AM +0200, Thomas Gleixner wrote:
> > On Fri, 23 Sep 2016, Peter Zijlstra wrote:
> > > It is, might_sleep() implies might_resched(). In fact, that's all what
> > > PREEMPT_VOLUNTARY is, make the might_sleep() debug test imply a resched
> > > point.
> > 
> > Grr, how intuitive - NOT!
> 
> No, it actually makes sense. Because you 'obviously' only call
> might_sleep() in contexts that should be able to sleep (if not, it'll
> holler). So they're already placed right for preemption.

I disagree. might_sleep() is commonly known as a debug mechanism and it
existed before the preemption stuff went in. So the easy way to sprinkle
preemption points into the kernel was to hijack might_sleep(). I know it's
historical, but that doesnt make it any more intuitive.

Thanks,

tglx

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Peter Zijlstra]

On Fri, Sep 23, 2016 at 10:00:37AM +0200, Thomas Gleixner wrote:
> On Fri, 23 Sep 2016, Peter Zijlstra wrote:
> > It is, might_sleep() implies might_resched(). In fact, that's all what
> > PREEMPT_VOLUNTARY is, make the might_sleep() debug test imply a resched
> > point.
> 
> Grr, how intuitive - NOT!

No, it actually makes sense. Because you 'obviously' only call
might_sleep() in contexts that should be able to sleep (if not, it'll
holler). So they're already placed right for preemption.

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Peter Zijlstra]

On Fri, Sep 23, 2016 at 10:00:37AM +0200, Thomas Gleixner wrote:
> On Fri, 23 Sep 2016, Peter Zijlstra wrote:
> > It is, might_sleep() implies might_resched(). In fact, that's all what
> > PREEMPT_VOLUNTARY is, make the might_sleep() debug test imply a resched
> > point.
> 
> Grr, how intuitive - NOT!

No, it actually makes sense. Because you 'obviously' only call
might_sleep() in contexts that should be able to sleep (if not, it'll
holler). So they're already placed right for preemption.

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Thomas Gleixner]

On Fri, 23 Sep 2016, Peter Zijlstra wrote:
> On Thu, Sep 22, 2016 at 10:59:30PM +0200, Thomas Gleixner wrote:
> > On Thu, 22 Sep 2016, Mikulas Patocka wrote:
> > > On Mon, 19 Sep 2016, Peter Zijlstra wrote:
> > > 
> > > > On Tue, Sep 13, 2016 at 09:39:59AM -0400, Mike Snitzer wrote:
> > > > > So I'm not sure how this dm-bufio local cond_resched() wrapper still 
> > > > > got
> > > > > in... happy to take your patch.
> > > > > 
> > > > > Please respond with whatever SOB you'd like applied to the patch 
> > > > > header.
> > > > 
> > > > Sorry, for the delay, here goes.
> > > 
> > > Why not change it to might_sleep()? - that would be almost equivalent to 
> > 
> > You mean might_resched(). might_sleep() is not even remotely equivalent.
> 
> It is, might_sleep() implies might_resched(). In fact, that's all what
> PREEMPT_VOLUNTARY is, make the might_sleep() debug test imply a resched
> point.

Grr, how intuitive - NOT!
 
> > > If we call the cond_resched() function in tight loops such as walking all 
> > > buffers in a list, there may be performance penalty due to the call, so 
> > > the call should be done only if it is really needed (i.e. in 
> > > CONFIG_PREEMPT_VOLUNTARY case).
> > 
> > Makes sense.
> 
> Is anybody still using PREEMPT_NONE? Most workloads also care about
> latency to some extend. Lots of code has explicit cond_resched() and
> doesn't worry.

Dunno. But I bet there are workloads which love it.

Thanks,

tglx

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Thomas Gleixner]

On Fri, 23 Sep 2016, Peter Zijlstra wrote:
> On Thu, Sep 22, 2016 at 10:59:30PM +0200, Thomas Gleixner wrote:
> > On Thu, 22 Sep 2016, Mikulas Patocka wrote:
> > > On Mon, 19 Sep 2016, Peter Zijlstra wrote:
> > > 
> > > > On Tue, Sep 13, 2016 at 09:39:59AM -0400, Mike Snitzer wrote:
> > > > > So I'm not sure how this dm-bufio local cond_resched() wrapper still 
> > > > > got
> > > > > in... happy to take your patch.
> > > > > 
> > > > > Please respond with whatever SOB you'd like applied to the patch 
> > > > > header.
> > > > 
> > > > Sorry, for the delay, here goes.
> > > 
> > > Why not change it to might_sleep()? - that would be almost equivalent to 
> > 
> > You mean might_resched(). might_sleep() is not even remotely equivalent.
> 
> It is, might_sleep() implies might_resched(). In fact, that's all what
> PREEMPT_VOLUNTARY is, make the might_sleep() debug test imply a resched
> point.

Grr, how intuitive - NOT!
 
> > > If we call the cond_resched() function in tight loops such as walking all 
> > > buffers in a list, there may be performance penalty due to the call, so 
> > > the call should be done only if it is really needed (i.e. in 
> > > CONFIG_PREEMPT_VOLUNTARY case).
> > 
> > Makes sense.
> 
> Is anybody still using PREEMPT_NONE? Most workloads also care about
> latency to some extend. Lots of code has explicit cond_resched() and
> doesn't worry.

Dunno. But I bet there are workloads which love it.

Thanks,

tglx

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Peter Zijlstra]

On Thu, Sep 22, 2016 at 10:59:30PM +0200, Thomas Gleixner wrote:
> On Thu, 22 Sep 2016, Mikulas Patocka wrote:
> > On Mon, 19 Sep 2016, Peter Zijlstra wrote:
> > 
> > > On Tue, Sep 13, 2016 at 09:39:59AM -0400, Mike Snitzer wrote:
> > > > So I'm not sure how this dm-bufio local cond_resched() wrapper still got
> > > > in... happy to take your patch.
> > > > 
> > > > Please respond with whatever SOB you'd like applied to the patch header.
> > > 
> > > Sorry, for the delay, here goes.
> > 
> > Why not change it to might_sleep()? - that would be almost equivalent to 
> 
> You mean might_resched(). might_sleep() is not even remotely equivalent.

It is, might_sleep() implies might_resched(). In fact, that's all what
PREEMPT_VOLUNTARY is, make the might_sleep() debug test imply a resched
point.

> > If we call the cond_resched() function in tight loops such as walking all 
> > buffers in a list, there may be performance penalty due to the call, so 
> > the call should be done only if it is really needed (i.e. in 
> > CONFIG_PREEMPT_VOLUNTARY case).
> 
> Makes sense.

Is anybody still using PREEMPT_NONE? Most workloads also care about
latency to some extend. Lots of code has explicit cond_resched() and
doesn't worry.

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Peter Zijlstra]

On Thu, Sep 22, 2016 at 10:59:30PM +0200, Thomas Gleixner wrote:
> On Thu, 22 Sep 2016, Mikulas Patocka wrote:
> > On Mon, 19 Sep 2016, Peter Zijlstra wrote:
> > 
> > > On Tue, Sep 13, 2016 at 09:39:59AM -0400, Mike Snitzer wrote:
> > > > So I'm not sure how this dm-bufio local cond_resched() wrapper still got
> > > > in... happy to take your patch.
> > > > 
> > > > Please respond with whatever SOB you'd like applied to the patch header.
> > > 
> > > Sorry, for the delay, here goes.
> > 
> > Why not change it to might_sleep()? - that would be almost equivalent to 
> 
> You mean might_resched(). might_sleep() is not even remotely equivalent.

It is, might_sleep() implies might_resched(). In fact, that's all what
PREEMPT_VOLUNTARY is, make the might_sleep() debug test imply a resched
point.

> > If we call the cond_resched() function in tight loops such as walking all 
> > buffers in a list, there may be performance penalty due to the call, so 
> > the call should be done only if it is really needed (i.e. in 
> > CONFIG_PREEMPT_VOLUNTARY case).
> 
> Makes sense.

Is anybody still using PREEMPT_NONE? Most workloads also care about
latency to some extend. Lots of code has explicit cond_resched() and
doesn't worry.

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Thomas Gleixner]

On Thu, 22 Sep 2016, Mikulas Patocka wrote:
> On Mon, 19 Sep 2016, Peter Zijlstra wrote:
> 
> > On Tue, Sep 13, 2016 at 09:39:59AM -0400, Mike Snitzer wrote:
> > > So I'm not sure how this dm-bufio local cond_resched() wrapper still got
> > > in... happy to take your patch.
> > > 
> > > Please respond with whatever SOB you'd like applied to the patch header.
> > 
> > Sorry, for the delay, here goes.
> 
> Why not change it to might_sleep()? - that would be almost equivalent to 

You mean might_resched(). might_sleep() is not even remotely equivalent.

> If we call the cond_resched() function in tight loops such as walking all 
> buffers in a list, there may be performance penalty due to the call, so 
> the call should be done only if it is really needed (i.e. in 
> CONFIG_PREEMPT_VOLUNTARY case).

Makes sense.

Thanks,

tglx

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Thomas Gleixner]

On Thu, 22 Sep 2016, Mikulas Patocka wrote:
> On Mon, 19 Sep 2016, Peter Zijlstra wrote:
> 
> > On Tue, Sep 13, 2016 at 09:39:59AM -0400, Mike Snitzer wrote:
> > > So I'm not sure how this dm-bufio local cond_resched() wrapper still got
> > > in... happy to take your patch.
> > > 
> > > Please respond with whatever SOB you'd like applied to the patch header.
> > 
> > Sorry, for the delay, here goes.
> 
> Why not change it to might_sleep()? - that would be almost equivalent to 

You mean might_resched(). might_sleep() is not even remotely equivalent.

> If we call the cond_resched() function in tight loops such as walking all 
> buffers in a list, there may be performance penalty due to the call, so 
> the call should be done only if it is really needed (i.e. in 
> CONFIG_PREEMPT_VOLUNTARY case).

Makes sense.

Thanks,

tglx

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Mikulas Patocka]

On Mon, 19 Sep 2016, Peter Zijlstra wrote:

> On Tue, Sep 13, 2016 at 09:39:59AM -0400, Mike Snitzer wrote:
> > So I'm not sure how this dm-bufio local cond_resched() wrapper still got
> > in... happy to take your patch.
> > 
> > Please respond with whatever SOB you'd like applied to the patch header.
> 
> Sorry, for the delay, here goes.

Why not change it to might_sleep()? - that would be almost equivalent to 
the code that was there before (i.e. reschedule only if 
CONFIG_PREEMPT_VOLUNTARY is set).

If we call the cond_resched() function in tight loops such as walking all 
buffers in a list, there may be performance penalty due to the call, so 
the call should be done only if it is really needed (i.e. in 
CONFIG_PREEMPT_VOLUNTARY case).

Mikulas

> ---
> Subject: dm: Remove dm_bufio_cond_resched()
> From: Peter Zijlstra 
> Date: Tue, 13 Sep 2016 10:45:20 +0200
> 
> Remove pointless local wrappery. Use cond_resched() like everybody else.
> 
> Cc: Ingo Molnar 
> Cc: Mikulas Patocka 
> Cc: Mike Snitzer 
> Cc: Alasdair Kergon 
> Acked-by: Thomas Gleixner 
> Signed-off-by: Peter Zijlstra (Intel) 
> ---
>  drivers/md/dm-bufio.c |   31 +--
>  1 file changed, 9 insertions(+), 22 deletions(-)
> 
> --- a/drivers/md/dm-bufio.c
> +++ b/drivers/md/dm-bufio.c
> @@ -191,19 +191,6 @@ static void dm_bufio_unlock(struct dm_bu
>   mutex_unlock(>lock);
>  }
>  
> -/*
> - * FIXME Move to sched.h?
> - */
> -#ifdef CONFIG_PREEMPT_VOLUNTARY
> -#  define dm_bufio_cond_resched()\
> -do { \
> - if (unlikely(need_resched()))   \
> - _cond_resched();\
> -} while (0)
> -#else
> -#  define dm_bufio_cond_resched()do { } while (0)
> -#endif
> -
>  /**/
>  
>  /*
> @@ -741,7 +728,7 @@ static void __flush_write_list(struct li
>   list_entry(write_list->next, struct dm_buffer, 
> write_list);
>   list_del(>write_list);
>   submit_io(b, WRITE, b->block, write_endio);
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>   blk_finish_plug();
>  }
> @@ -780,7 +767,7 @@ static struct dm_buffer *__get_unclaimed
>   __unlink_buffer(b);
>   return b;
>   }
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>  
>   list_for_each_entry_reverse(b, >lru[LIST_DIRTY], lru_list) {
> @@ -791,7 +778,7 @@ static struct dm_buffer *__get_unclaimed
>   __unlink_buffer(b);
>   return b;
>   }
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>  
>   return NULL;
> @@ -923,7 +910,7 @@ static void __write_dirty_buffers_async(
>   return;
>  
>   __write_dirty_buffer(b, write_list);
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>  }
>  
> @@ -973,7 +960,7 @@ static void __check_watermark(struct dm_
>   return;
>  
>   __free_buffer_wake(b);
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>  
>   if (c->n_buffers[LIST_DIRTY] > threshold_buffers)
> @@ -1170,7 +1157,7 @@ void dm_bufio_prefetch(struct dm_bufio_c
>   submit_io(b, READ, b->block, read_endio);
>   dm_bufio_release(b);
>  
> - dm_bufio_cond_resched();
> + cond_resched();
>  
>   if (!n_blocks)
>   goto flush_plug;
> @@ -1291,7 +1278,7 @@ int dm_bufio_write_dirty_buffers(struct
>   !test_bit(B_WRITING, >state))
>   __relink_lru(b, LIST_CLEAN);
>  
> - dm_bufio_cond_resched();
> + cond_resched();
>  
>   /*
>* If we dropped the lock, the list is no longer consistent,
> @@ -1574,7 +1561,7 @@ static unsigned long __scan(struct dm_bu
>   freed++;
>   if (!--nr_to_scan || ((count - freed) <= retain_target))
>   return freed;
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>   }
>   return freed;
> @@ -1808,7 +1795,7 @@ static void __evict_old_buffers(struct d
>   if (__try_evict_buffer(b, 0))
>   count--;
>  
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>  
>   dm_bufio_unlock(c);
> 

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Mikulas Patocka]

On Mon, 19 Sep 2016, Peter Zijlstra wrote:

> On Tue, Sep 13, 2016 at 09:39:59AM -0400, Mike Snitzer wrote:
> > So I'm not sure how this dm-bufio local cond_resched() wrapper still got
> > in... happy to take your patch.
> > 
> > Please respond with whatever SOB you'd like applied to the patch header.
> 
> Sorry, for the delay, here goes.

Why not change it to might_sleep()? - that would be almost equivalent to 
the code that was there before (i.e. reschedule only if 
CONFIG_PREEMPT_VOLUNTARY is set).

If we call the cond_resched() function in tight loops such as walking all 
buffers in a list, there may be performance penalty due to the call, so 
the call should be done only if it is really needed (i.e. in 
CONFIG_PREEMPT_VOLUNTARY case).

Mikulas

> ---
> Subject: dm: Remove dm_bufio_cond_resched()
> From: Peter Zijlstra 
> Date: Tue, 13 Sep 2016 10:45:20 +0200
> 
> Remove pointless local wrappery. Use cond_resched() like everybody else.
> 
> Cc: Ingo Molnar 
> Cc: Mikulas Patocka 
> Cc: Mike Snitzer 
> Cc: Alasdair Kergon 
> Acked-by: Thomas Gleixner 
> Signed-off-by: Peter Zijlstra (Intel) 
> ---
>  drivers/md/dm-bufio.c |   31 +--
>  1 file changed, 9 insertions(+), 22 deletions(-)
> 
> --- a/drivers/md/dm-bufio.c
> +++ b/drivers/md/dm-bufio.c
> @@ -191,19 +191,6 @@ static void dm_bufio_unlock(struct dm_bu
>   mutex_unlock(>lock);
>  }
>  
> -/*
> - * FIXME Move to sched.h?
> - */
> -#ifdef CONFIG_PREEMPT_VOLUNTARY
> -#  define dm_bufio_cond_resched()\
> -do { \
> - if (unlikely(need_resched()))   \
> - _cond_resched();\
> -} while (0)
> -#else
> -#  define dm_bufio_cond_resched()do { } while (0)
> -#endif
> -
>  /**/
>  
>  /*
> @@ -741,7 +728,7 @@ static void __flush_write_list(struct li
>   list_entry(write_list->next, struct dm_buffer, 
> write_list);
>   list_del(>write_list);
>   submit_io(b, WRITE, b->block, write_endio);
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>   blk_finish_plug();
>  }
> @@ -780,7 +767,7 @@ static struct dm_buffer *__get_unclaimed
>   __unlink_buffer(b);
>   return b;
>   }
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>  
>   list_for_each_entry_reverse(b, >lru[LIST_DIRTY], lru_list) {
> @@ -791,7 +778,7 @@ static struct dm_buffer *__get_unclaimed
>   __unlink_buffer(b);
>   return b;
>   }
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>  
>   return NULL;
> @@ -923,7 +910,7 @@ static void __write_dirty_buffers_async(
>   return;
>  
>   __write_dirty_buffer(b, write_list);
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>  }
>  
> @@ -973,7 +960,7 @@ static void __check_watermark(struct dm_
>   return;
>  
>   __free_buffer_wake(b);
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>  
>   if (c->n_buffers[LIST_DIRTY] > threshold_buffers)
> @@ -1170,7 +1157,7 @@ void dm_bufio_prefetch(struct dm_bufio_c
>   submit_io(b, READ, b->block, read_endio);
>   dm_bufio_release(b);
>  
> - dm_bufio_cond_resched();
> + cond_resched();
>  
>   if (!n_blocks)
>   goto flush_plug;
> @@ -1291,7 +1278,7 @@ int dm_bufio_write_dirty_buffers(struct
>   !test_bit(B_WRITING, >state))
>   __relink_lru(b, LIST_CLEAN);
>  
> - dm_bufio_cond_resched();
> + cond_resched();
>  
>   /*
>* If we dropped the lock, the list is no longer consistent,
> @@ -1574,7 +1561,7 @@ static unsigned long __scan(struct dm_bu
>   freed++;
>   if (!--nr_to_scan || ((count - freed) <= retain_target))
>   return freed;
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>   }
>   return freed;
> @@ -1808,7 +1795,7 @@ static void __evict_old_buffers(struct d
>   if (__try_evict_buffer(b, 0))
>   count--;
>  
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>  
>   dm_bufio_unlock(c);
> 

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Peter Zijlstra]

On Tue, Sep 13, 2016 at 09:39:59AM -0400, Mike Snitzer wrote:
> So I'm not sure how this dm-bufio local cond_resched() wrapper still got
> in... happy to take your patch.
> 
> Please respond with whatever SOB you'd like applied to the patch header.

Sorry, for the delay, here goes.

---
Subject: dm: Remove dm_bufio_cond_resched()
From: Peter Zijlstra 
Date: Tue, 13 Sep 2016 10:45:20 +0200

Remove pointless local wrappery. Use cond_resched() like everybody else.

Cc: Ingo Molnar 
Cc: Mikulas Patocka 
Cc: Mike Snitzer 
Cc: Alasdair Kergon 
Acked-by: Thomas Gleixner 
Signed-off-by: Peter Zijlstra (Intel) 
---
 drivers/md/dm-bufio.c |   31 +--
 1 file changed, 9 insertions(+), 22 deletions(-)

--- a/drivers/md/dm-bufio.c
+++ b/drivers/md/dm-bufio.c
@@ -191,19 +191,6 @@ static void dm_bufio_unlock(struct dm_bu
mutex_unlock(>lock);
 }
 
-/*
- * FIXME Move to sched.h?
- */
-#ifdef CONFIG_PREEMPT_VOLUNTARY
-#  define dm_bufio_cond_resched()  \
-do {   \
-   if (unlikely(need_resched()))   \
-   _cond_resched();\
-} while (0)
-#else
-#  define dm_bufio_cond_resched()do { } while (0)
-#endif
-
 /**/
 
 /*
@@ -741,7 +728,7 @@ static void __flush_write_list(struct li
list_entry(write_list->next, struct dm_buffer, 
write_list);
list_del(>write_list);
submit_io(b, WRITE, b->block, write_endio);
-   dm_bufio_cond_resched();
+   cond_resched();
}
blk_finish_plug();
 }
@@ -780,7 +767,7 @@ static struct dm_buffer *__get_unclaimed
__unlink_buffer(b);
return b;
}
-   dm_bufio_cond_resched();
+   cond_resched();
}
 
list_for_each_entry_reverse(b, >lru[LIST_DIRTY], lru_list) {
@@ -791,7 +778,7 @@ static struct dm_buffer *__get_unclaimed
__unlink_buffer(b);
return b;
}
-   dm_bufio_cond_resched();
+   cond_resched();
}
 
return NULL;
@@ -923,7 +910,7 @@ static void __write_dirty_buffers_async(
return;
 
__write_dirty_buffer(b, write_list);
-   dm_bufio_cond_resched();
+   cond_resched();
}
 }
 
@@ -973,7 +960,7 @@ static void __check_watermark(struct dm_
return;
 
__free_buffer_wake(b);
-   dm_bufio_cond_resched();
+   cond_resched();
}
 
if (c->n_buffers[LIST_DIRTY] > threshold_buffers)
@@ -1170,7 +1157,7 @@ void dm_bufio_prefetch(struct dm_bufio_c
submit_io(b, READ, b->block, read_endio);
dm_bufio_release(b);
 
-   dm_bufio_cond_resched();
+   cond_resched();
 
if (!n_blocks)
goto flush_plug;
@@ -1291,7 +1278,7 @@ int dm_bufio_write_dirty_buffers(struct
!test_bit(B_WRITING, >state))
__relink_lru(b, LIST_CLEAN);
 
-   dm_bufio_cond_resched();
+   cond_resched();
 
/*
 * If we dropped the lock, the list is no longer consistent,
@@ -1574,7 +1561,7 @@ static unsigned long __scan(struct dm_bu
freed++;
if (!--nr_to_scan || ((count - freed) <= retain_target))
return freed;
-   dm_bufio_cond_resched();
+   cond_resched();
}
}
return freed;
@@ -1808,7 +1795,7 @@ static void __evict_old_buffers(struct d
if (__try_evict_buffer(b, 0))
count--;
 
-   dm_bufio_cond_resched();
+   cond_resched();
}
 
dm_bufio_unlock(c);

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Peter Zijlstra]

On Tue, Sep 13, 2016 at 09:39:59AM -0400, Mike Snitzer wrote:
> So I'm not sure how this dm-bufio local cond_resched() wrapper still got
> in... happy to take your patch.
> 
> Please respond with whatever SOB you'd like applied to the patch header.

Sorry, for the delay, here goes.

---
Subject: dm: Remove dm_bufio_cond_resched()
From: Peter Zijlstra 
Date: Tue, 13 Sep 2016 10:45:20 +0200

Remove pointless local wrappery. Use cond_resched() like everybody else.

Cc: Ingo Molnar 
Cc: Mikulas Patocka 
Cc: Mike Snitzer 
Cc: Alasdair Kergon 
Acked-by: Thomas Gleixner 
Signed-off-by: Peter Zijlstra (Intel) 
---
 drivers/md/dm-bufio.c |   31 +--
 1 file changed, 9 insertions(+), 22 deletions(-)

--- a/drivers/md/dm-bufio.c
+++ b/drivers/md/dm-bufio.c
@@ -191,19 +191,6 @@ static void dm_bufio_unlock(struct dm_bu
mutex_unlock(>lock);
 }
 
-/*
- * FIXME Move to sched.h?
- */
-#ifdef CONFIG_PREEMPT_VOLUNTARY
-#  define dm_bufio_cond_resched()  \
-do {   \
-   if (unlikely(need_resched()))   \
-   _cond_resched();\
-} while (0)
-#else
-#  define dm_bufio_cond_resched()do { } while (0)
-#endif
-
 /**/
 
 /*
@@ -741,7 +728,7 @@ static void __flush_write_list(struct li
list_entry(write_list->next, struct dm_buffer, 
write_list);
list_del(>write_list);
submit_io(b, WRITE, b->block, write_endio);
-   dm_bufio_cond_resched();
+   cond_resched();
}
blk_finish_plug();
 }
@@ -780,7 +767,7 @@ static struct dm_buffer *__get_unclaimed
__unlink_buffer(b);
return b;
}
-   dm_bufio_cond_resched();
+   cond_resched();
}
 
list_for_each_entry_reverse(b, >lru[LIST_DIRTY], lru_list) {
@@ -791,7 +778,7 @@ static struct dm_buffer *__get_unclaimed
__unlink_buffer(b);
return b;
}
-   dm_bufio_cond_resched();
+   cond_resched();
}
 
return NULL;
@@ -923,7 +910,7 @@ static void __write_dirty_buffers_async(
return;
 
__write_dirty_buffer(b, write_list);
-   dm_bufio_cond_resched();
+   cond_resched();
}
 }
 
@@ -973,7 +960,7 @@ static void __check_watermark(struct dm_
return;
 
__free_buffer_wake(b);
-   dm_bufio_cond_resched();
+   cond_resched();
}
 
if (c->n_buffers[LIST_DIRTY] > threshold_buffers)
@@ -1170,7 +1157,7 @@ void dm_bufio_prefetch(struct dm_bufio_c
submit_io(b, READ, b->block, read_endio);
dm_bufio_release(b);
 
-   dm_bufio_cond_resched();
+   cond_resched();
 
if (!n_blocks)
goto flush_plug;
@@ -1291,7 +1278,7 @@ int dm_bufio_write_dirty_buffers(struct
!test_bit(B_WRITING, >state))
__relink_lru(b, LIST_CLEAN);
 
-   dm_bufio_cond_resched();
+   cond_resched();
 
/*
 * If we dropped the lock, the list is no longer consistent,
@@ -1574,7 +1561,7 @@ static unsigned long __scan(struct dm_bu
freed++;
if (!--nr_to_scan || ((count - freed) <= retain_target))
return freed;
-   dm_bufio_cond_resched();
+   cond_resched();
}
}
return freed;
@@ -1808,7 +1795,7 @@ static void __evict_old_buffers(struct d
if (__try_evict_buffer(b, 0))
count--;
 
-   dm_bufio_cond_resched();
+   cond_resched();
}
 
dm_bufio_unlock(c);

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Peter Zijlstra]

On Mon, Sep 19, 2016 at 05:49:07AM -0400, Mikulas Patocka wrote:
> 
> 
> On Tue, 13 Sep 2016, Peter Zijlstra wrote:
> 
> > Hi all,
> > 
> > While grepping for PREEMPT_VOLUNTARY I ran into dm_bufio_cond_resched()
> > and wondered WTH it was about.
> 
> cond_resched() calls _cond_resched() even if when we have a preemptive 
> kernel - with preemptive kernel, calling cond_resched is pointless because 
> rescheduling is done peemtively.
> 
> So, I added that dm_bufio_cond_resched(), that does nothing on peemptive 
> kernels (and also on PREEMPT_NONE kernels where the user doesn't care 
> about latency).
> 
> What is the reason why cond_resched() tests for rescheduling with 
> preemptive kernel? Why should I use cond_resched() in that case?

Because every body else does too. 'Fixing' something like that in the dm
code is entirely the wrong place. Also, you loose out on the
might_sleep() warning implied in it.

As it happens, I have a patch fixing that somewhere, let me try and get
it merged.

But thanks for the reminder, I'll go write a Changelog for this so that
people can commit.

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Peter Zijlstra]

On Mon, Sep 19, 2016 at 05:49:07AM -0400, Mikulas Patocka wrote:
> 
> 
> On Tue, 13 Sep 2016, Peter Zijlstra wrote:
> 
> > Hi all,
> > 
> > While grepping for PREEMPT_VOLUNTARY I ran into dm_bufio_cond_resched()
> > and wondered WTH it was about.
> 
> cond_resched() calls _cond_resched() even if when we have a preemptive 
> kernel - with preemptive kernel, calling cond_resched is pointless because 
> rescheduling is done peemtively.
> 
> So, I added that dm_bufio_cond_resched(), that does nothing on peemptive 
> kernels (and also on PREEMPT_NONE kernels where the user doesn't care 
> about latency).
> 
> What is the reason why cond_resched() tests for rescheduling with 
> preemptive kernel? Why should I use cond_resched() in that case?

Because every body else does too. 'Fixing' something like that in the dm
code is entirely the wrong place. Also, you loose out on the
might_sleep() warning implied in it.

As it happens, I have a patch fixing that somewhere, let me try and get
it merged.

But thanks for the reminder, I'll go write a Changelog for this so that
people can commit.

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Mikulas Patocka]

On Tue, 13 Sep 2016, Peter Zijlstra wrote:

> Hi all,
> 
> While grepping for PREEMPT_VOLUNTARY I ran into dm_bufio_cond_resched()
> and wondered WTH it was about.

cond_resched() calls _cond_resched() even if when we have a preemptive 
kernel - with preemptive kernel, calling cond_resched is pointless because 
rescheduling is done peemtively.

So, I added that dm_bufio_cond_resched(), that does nothing on peemptive 
kernels (and also on PREEMPT_NONE kernels where the user doesn't care 
about latency).

What is the reason why cond_resched() tests for rescheduling with 
preemptive kernel? Why should I use cond_resched() in that case?

Mikulas

> Is there anything wrong with the below patch?
> 
> ---
> diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c
> index 8625040bae92..125aedc3875f 100644
> --- a/drivers/md/dm-bufio.c
> +++ b/drivers/md/dm-bufio.c
> @@ -191,19 +191,6 @@ static void dm_bufio_unlock(struct dm_bufio_client *c)
>   mutex_unlock(>lock);
>  }
>  
> -/*
> - * FIXME Move to sched.h?
> - */
> -#ifdef CONFIG_PREEMPT_VOLUNTARY
> -#  define dm_bufio_cond_resched()\
> -do { \
> - if (unlikely(need_resched()))   \
> - _cond_resched();\
> -} while (0)
> -#else
> -#  define dm_bufio_cond_resched()do { } while (0)
> -#endif
> -
>  /**/
>  
>  /*
> @@ -741,7 +728,7 @@ static void __flush_write_list(struct list_head 
> *write_list)
>   list_entry(write_list->next, struct dm_buffer, 
> write_list);
>   list_del(>write_list);
>   submit_io(b, WRITE, b->block, write_endio);
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>   blk_finish_plug();
>  }
> @@ -780,7 +767,7 @@ static struct dm_buffer *__get_unclaimed_buffer(struct 
> dm_bufio_client *c)
>   __unlink_buffer(b);
>   return b;
>   }
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>  
>   list_for_each_entry_reverse(b, >lru[LIST_DIRTY], lru_list) {
> @@ -791,7 +778,7 @@ static struct dm_buffer *__get_unclaimed_buffer(struct 
> dm_bufio_client *c)
>   __unlink_buffer(b);
>   return b;
>   }
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>  
>   return NULL;
> @@ -923,7 +910,7 @@ static void __write_dirty_buffers_async(struct 
> dm_bufio_client *c, int no_wait,
>   return;
>  
>   __write_dirty_buffer(b, write_list);
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>  }
>  
> @@ -973,7 +960,7 @@ static void __check_watermark(struct dm_bufio_client *c,
>   return;
>  
>   __free_buffer_wake(b);
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>  
>   if (c->n_buffers[LIST_DIRTY] > threshold_buffers)
> @@ -1170,7 +1157,7 @@ void dm_bufio_prefetch(struct dm_bufio_client *c,
>   submit_io(b, READ, b->block, read_endio);
>   dm_bufio_release(b);
>  
> - dm_bufio_cond_resched();
> + cond_resched();
>  
>   if (!n_blocks)
>   goto flush_plug;
> @@ -1291,7 +1278,7 @@ int dm_bufio_write_dirty_buffers(struct dm_bufio_client 
> *c)
>   !test_bit(B_WRITING, >state))
>   __relink_lru(b, LIST_CLEAN);
>  
> - dm_bufio_cond_resched();
> + cond_resched();
>  
>   /*
>* If we dropped the lock, the list is no longer consistent,
> @@ -1574,7 +1561,7 @@ static unsigned long __scan(struct dm_bufio_client *c, 
> unsigned long nr_to_scan,
>   freed++;
>   if (!--nr_to_scan || ((count - freed) <= retain_target))
>   return freed;
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>   }
>   return freed;
> @@ -1808,7 +1795,7 @@ static void __evict_old_buffers(struct dm_bufio_client 
> *c, unsigned long age_hz)
>   if (__try_evict_buffer(b, 0))
>   count--;
>  
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>  
>   dm_bufio_unlock(c);
> 

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Mikulas Patocka]

On Tue, 13 Sep 2016, Peter Zijlstra wrote:

> Hi all,
> 
> While grepping for PREEMPT_VOLUNTARY I ran into dm_bufio_cond_resched()
> and wondered WTH it was about.

cond_resched() calls _cond_resched() even if when we have a preemptive 
kernel - with preemptive kernel, calling cond_resched is pointless because 
rescheduling is done peemtively.

So, I added that dm_bufio_cond_resched(), that does nothing on peemptive 
kernels (and also on PREEMPT_NONE kernels where the user doesn't care 
about latency).

What is the reason why cond_resched() tests for rescheduling with 
preemptive kernel? Why should I use cond_resched() in that case?

Mikulas

> Is there anything wrong with the below patch?
> 
> ---
> diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c
> index 8625040bae92..125aedc3875f 100644
> --- a/drivers/md/dm-bufio.c
> +++ b/drivers/md/dm-bufio.c
> @@ -191,19 +191,6 @@ static void dm_bufio_unlock(struct dm_bufio_client *c)
>   mutex_unlock(>lock);
>  }
>  
> -/*
> - * FIXME Move to sched.h?
> - */
> -#ifdef CONFIG_PREEMPT_VOLUNTARY
> -#  define dm_bufio_cond_resched()\
> -do { \
> - if (unlikely(need_resched()))   \
> - _cond_resched();\
> -} while (0)
> -#else
> -#  define dm_bufio_cond_resched()do { } while (0)
> -#endif
> -
>  /**/
>  
>  /*
> @@ -741,7 +728,7 @@ static void __flush_write_list(struct list_head 
> *write_list)
>   list_entry(write_list->next, struct dm_buffer, 
> write_list);
>   list_del(>write_list);
>   submit_io(b, WRITE, b->block, write_endio);
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>   blk_finish_plug();
>  }
> @@ -780,7 +767,7 @@ static struct dm_buffer *__get_unclaimed_buffer(struct 
> dm_bufio_client *c)
>   __unlink_buffer(b);
>   return b;
>   }
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>  
>   list_for_each_entry_reverse(b, >lru[LIST_DIRTY], lru_list) {
> @@ -791,7 +778,7 @@ static struct dm_buffer *__get_unclaimed_buffer(struct 
> dm_bufio_client *c)
>   __unlink_buffer(b);
>   return b;
>   }
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>  
>   return NULL;
> @@ -923,7 +910,7 @@ static void __write_dirty_buffers_async(struct 
> dm_bufio_client *c, int no_wait,
>   return;
>  
>   __write_dirty_buffer(b, write_list);
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>  }
>  
> @@ -973,7 +960,7 @@ static void __check_watermark(struct dm_bufio_client *c,
>   return;
>  
>   __free_buffer_wake(b);
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>  
>   if (c->n_buffers[LIST_DIRTY] > threshold_buffers)
> @@ -1170,7 +1157,7 @@ void dm_bufio_prefetch(struct dm_bufio_client *c,
>   submit_io(b, READ, b->block, read_endio);
>   dm_bufio_release(b);
>  
> - dm_bufio_cond_resched();
> + cond_resched();
>  
>   if (!n_blocks)
>   goto flush_plug;
> @@ -1291,7 +1278,7 @@ int dm_bufio_write_dirty_buffers(struct dm_bufio_client 
> *c)
>   !test_bit(B_WRITING, >state))
>   __relink_lru(b, LIST_CLEAN);
>  
> - dm_bufio_cond_resched();
> + cond_resched();
>  
>   /*
>* If we dropped the lock, the list is no longer consistent,
> @@ -1574,7 +1561,7 @@ static unsigned long __scan(struct dm_bufio_client *c, 
> unsigned long nr_to_scan,
>   freed++;
>   if (!--nr_to_scan || ((count - freed) <= retain_target))
>   return freed;
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>   }
>   return freed;
> @@ -1808,7 +1795,7 @@ static void __evict_old_buffers(struct dm_bufio_client 
> *c, unsigned long age_hz)
>   if (__try_evict_buffer(b, 0))
>   count--;
>  
> - dm_bufio_cond_resched();
> + cond_resched();
>   }
>  
>   dm_bufio_unlock(c);
> 

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Mike Snitzer]

On Tue, Sep 13 2016 at  4:45am -0400,
Peter Zijlstra  wrote:

> Hi all,
> 
> While grepping for PREEMPT_VOLUNTARY I ran into dm_bufio_cond_resched()
> and wondered WTH it was about.
> 
> Is there anything wrong with the below patch?

No, I'll pick it up for 4.9 merge.  Mikulas added it for sparc or
something.  I cannot recall _the_ reason (I wasn't maintaining DM back
then) but at the time both Alasdair and Joe Thornber reasoned that it
needed to go -- and that if it was really needed that it should be done
in terms of a proper patch to sched.h, etc.

So I'm not sure how this dm-bufio local cond_resched() wrapper still got
in... happy to take your patch.

Please respond with whatever SOB you'd like applied to the patch header.

Thanks,
Mike

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Mike Snitzer]

On Tue, Sep 13 2016 at  4:45am -0400,
Peter Zijlstra  wrote:

> Hi all,
> 
> While grepping for PREEMPT_VOLUNTARY I ran into dm_bufio_cond_resched()
> and wondered WTH it was about.
> 
> Is there anything wrong with the below patch?

No, I'll pick it up for 4.9 merge.  Mikulas added it for sparc or
something.  I cannot recall _the_ reason (I wasn't maintaining DM back
then) but at the time both Alasdair and Joe Thornber reasoned that it
needed to go -- and that if it was really needed that it should be done
in terms of a proper patch to sched.h, etc.

So I'm not sure how this dm-bufio local cond_resched() wrapper still got
in... happy to take your patch.

Please respond with whatever SOB you'd like applied to the patch header.

Thanks,
Mike

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Thomas Gleixner]

On Tue, 13 Sep 2016, Peter Zijlstra wrote:

> Hi all,
> 
> While grepping for PREEMPT_VOLUNTARY I ran into dm_bufio_cond_resched()
> and wondered WTH it was about.
> 
> Is there anything wrong with the below patch?

Not at all, except that you forgot to add your SOB to it :)

Acked-by: Thomas Gleixner 

Re: [RFC][PATCH] dm: Remove dm_bufio_cond_resched()

[Thomas Gleixner]

On Tue, 13 Sep 2016, Peter Zijlstra wrote:

> Hi all,
> 
> While grepping for PREEMPT_VOLUNTARY I ran into dm_bufio_cond_resched()
> and wondered WTH it was about.
> 
> Is there anything wrong with the below patch?

Not at all, except that you forgot to add your SOB to it :)

Acked-by: Thomas Gleixner