[PATCH] block/laptop_mode: Convert timers to use timer_setup()

[PATCH] block/laptop_mode: Convert timers to use timer_setup()

[Kees Cook]

In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Jens Axboe <axboe@kernel.dk>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: Matthew Wilcox <mawilcox@microsoft.com>
Cc: Jeff Layton <jlayton@redhat.com>
Cc: linux-block@vger.kernel.org
Cc: linux-mm@kvack.org
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
This requires commit 686fef928bba ("timer: Prepare to change timer
callback argument type") in v4.14-rc3, but should be otherwise
stand-alone.
---
 block/blk-core.c          | 10 +++++-----
 include/linux/writeback.h |  2 +-
 mm/page-writeback.c       |  9 +++++----
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 048be4aa6024..070913294cbb 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -803,9 +803,9 @@ static void blk_queue_usage_counter_release(struct percpu_ref *ref)
 	wake_up_all(&q->mq_freeze_wq);
 }
 
-static void blk_rq_timed_out_timer(unsigned long data)
+static void blk_rq_timed_out_timer(struct timer_list *t)
 {
-	struct request_queue *q = (struct request_queue *)data;
+	struct request_queue *q = from_timer(q, t, timeout);
 
 	kblockd_schedule_work(&q->timeout_work);
 }
@@ -841,9 +841,9 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
 	q->backing_dev_info->name = "block";
 	q->node = node_id;
 
-	setup_timer(&q->backing_dev_info->laptop_mode_wb_timer,
-		    laptop_mode_timer_fn, (unsigned long) q);
-	setup_timer(&q->timeout, blk_rq_timed_out_timer, (unsigned long) q);
+	timer_setup(&q->backing_dev_info->laptop_mode_wb_timer,
+		    laptop_mode_timer_fn, 0);
+	timer_setup(&q->timeout, blk_rq_timed_out_timer, 0);
 	INIT_LIST_HEAD(&q->queue_head);
 	INIT_LIST_HEAD(&q->timeout_list);
 	INIT_LIST_HEAD(&q->icq_list);
diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index d5815794416c..3bfb50e69a81 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -329,7 +329,7 @@ static inline void cgroup_writeback_umount(void)
 void laptop_io_completion(struct backing_dev_info *info);
 void laptop_sync_completion(void);
 void laptop_mode_sync(struct work_struct *work);
-void laptop_mode_timer_fn(unsigned long data);
+void laptop_mode_timer_fn(struct timer_list *t);
 #else
 static inline void laptop_sync_completion(void) { }
 #endif
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 0b9c5cbe8eba..1be0e3067f04 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -1977,9 +1977,10 @@ int dirty_writeback_centisecs_handler(struct ctl_table *table, int write,
 }
 
 #ifdef CONFIG_BLOCK
-void laptop_mode_timer_fn(unsigned long data)
+void laptop_mode_timer_fn(struct timer_list *t)
 {
-	struct request_queue *q = (struct request_queue *)data;
+	struct backing_dev_info *backing_dev_info =
+		from_timer(backing_dev_info, t, laptop_mode_wb_timer);
 	int nr_pages = global_node_page_state(NR_FILE_DIRTY) +
 		global_node_page_state(NR_UNSTABLE_NFS);
 	struct bdi_writeback *wb;
@@ -1988,11 +1989,11 @@ void laptop_mode_timer_fn(unsigned long data)
 	 * We want to write everything out, not just down to the dirty
 	 * threshold
 	 */
-	if (!bdi_has_dirty_io(q->backing_dev_info))
+	if (!bdi_has_dirty_io(backing_dev_info))
 		return;
 
 	rcu_read_lock();
-	list_for_each_entry_rcu(wb, &q->backing_dev_info->wb_list, bdi_node)
+	list_for_each_entry_rcu(wb, &backing_dev_info->wb_list, bdi_node)
 		if (wb_has_dirty_io(wb))
 			wb_start_writeback(wb, nr_pages, true,
 					   WB_REASON_LAPTOP_TIMER);
-- 
2.7.4


-- 
Kees Cook
Pixel Security

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] block/laptop_mode: Convert timers to use timer_setup()

[Jens Axboe]

On 10/04/2017 06:49 PM, Kees Cook wrote:
> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
> 
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Vladimir Davydov <vdavydov.dev@gmail.com>
> Cc: Matthew Wilcox <mawilcox@microsoft.com>
> Cc: Jeff Layton <jlayton@redhat.com>
> Cc: linux-block@vger.kernel.org
> Cc: linux-mm@kvack.org
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> This requires commit 686fef928bba ("timer: Prepare to change timer
> callback argument type") in v4.14-rc3, but should be otherwise
> stand-alone.

My only complaint about this is the use of a from_timer() macro instead
of just using container_of() at the call sites to actually show that is
happening. I'm generally opposed to obfuscation like that. It just means
you have to look up what is going on, instead of it being readily
apparent to the reader/reviewer.

I guess I do have a a second complaint as well - that it landed in -rc3,
which is rather late considering subsystem trees are usually forked
earlier than that. Had this been in -rc1, I would have had an easier
time applying the block bits for 4.15.

-- 
Jens Axboe

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] block/laptop_mode: Convert timers to use timer_setup()

[Kees Cook]

On Thu, Oct 5, 2017 at 7:56 AM, Jens Axboe <axboe@kernel.dk> wrote:
> On 10/04/2017 06:49 PM, Kees Cook wrote:
>> In preparation for unconditionally passing the struct timer_list pointer to
>> all timer callbacks, switch to using the new timer_setup() and from_timer()
>> to pass the timer pointer explicitly.
>>
>> Cc: Jens Axboe <axboe@kernel.dk>
>> Cc: Michal Hocko <mhocko@suse.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Jan Kara <jack@suse.cz>
>> Cc: Johannes Weiner <hannes@cmpxchg.org>
>> Cc: Nicholas Piggin <npiggin@gmail.com>
>> Cc: Vladimir Davydov <vdavydov.dev@gmail.com>
>> Cc: Matthew Wilcox <mawilcox@microsoft.com>
>> Cc: Jeff Layton <jlayton@redhat.com>
>> Cc: linux-block@vger.kernel.org
>> Cc: linux-mm@kvack.org
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> ---
>> This requires commit 686fef928bba ("timer: Prepare to change timer
>> callback argument type") in v4.14-rc3, but should be otherwise
>> stand-alone.
>
> My only complaint about this is the use of a from_timer() macro instead
> of just using container_of() at the call sites to actually show that is
> happening. I'm generally opposed to obfuscation like that. It just means
> you have to look up what is going on, instead of it being readily
> apparent to the reader/reviewer.

Yeah, this got discussed a bit with tglx and hch. Ultimately, this
seems to be the least bad of several options. Specifically with regard
to container_of(), it just gets to be huge, and makes things harder to
read (almost always requires a line break, needlessly repeats the
variable type definition, etc). Since there is precedent of both using
wrappers on container_of() and for adding from_foo() helpers, I chose
the resulting from_timer().

> I guess I do have a a second complaint as well - that it landed in -rc3,
> which is rather late considering subsystem trees are usually forked
> earlier than that. Had this been in -rc1, I would have had an easier
> time applying the block bits for 4.15.

Yes, totally true. tglx and I ended up meeting face-to-face at the
Kernel Recipes conference and we solved some outstanding design issues
with the conversion. The timing meant the new API went into -rc3,
which seemed better than missing an entire release cycle, or carrying
deltas against maintainer trees that would drift. (This is actually my
second massive refactoring of these changes...)

If you don't want to deal with the -rc3 issue, would you want these
changes to get carried in the timer tree instead?

Thanks!

-Kees

-- 
Kees Cook
Pixel Security

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] block/laptop_mode: Convert timers to use timer_setup()

[Jens Axboe]

On 10/05/2017 11:49 AM, Kees Cook wrote:
> On Thu, Oct 5, 2017 at 7:56 AM, Jens Axboe <axboe@kernel.dk> wrote:
>> On 10/04/2017 06:49 PM, Kees Cook wrote:
>>> In preparation for unconditionally passing the struct timer_list pointer to
>>> all timer callbacks, switch to using the new timer_setup() and from_timer()
>>> to pass the timer pointer explicitly.
>>>
>>> Cc: Jens Axboe <axboe@kernel.dk>
>>> Cc: Michal Hocko <mhocko@suse.com>
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> Cc: Jan Kara <jack@suse.cz>
>>> Cc: Johannes Weiner <hannes@cmpxchg.org>
>>> Cc: Nicholas Piggin <npiggin@gmail.com>
>>> Cc: Vladimir Davydov <vdavydov.dev@gmail.com>
>>> Cc: Matthew Wilcox <mawilcox@microsoft.com>
>>> Cc: Jeff Layton <jlayton@redhat.com>
>>> Cc: linux-block@vger.kernel.org
>>> Cc: linux-mm@kvack.org
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>> ---
>>> This requires commit 686fef928bba ("timer: Prepare to change timer
>>> callback argument type") in v4.14-rc3, but should be otherwise
>>> stand-alone.
>>
>> My only complaint about this is the use of a from_timer() macro instead
>> of just using container_of() at the call sites to actually show that is
>> happening. I'm generally opposed to obfuscation like that. It just means
>> you have to look up what is going on, instead of it being readily
>> apparent to the reader/reviewer.
> 
> Yeah, this got discussed a bit with tglx and hch. Ultimately, this
> seems to be the least bad of several options. Specifically with regard
> to container_of(), it just gets to be huge, and makes things harder to
> read (almost always requires a line break, needlessly repeats the
> variable type definition, etc). Since there is precedent of both using
> wrappers on container_of() and for adding from_foo() helpers, I chose
> the resulting from_timer().

It might make for a longer line, but at least it's a readable line.
What does from_timer() do? Nobody knows, you have to find it and check.
So I'd still argue that it's a hell of a lot more readable than some
random function name.

>> I guess I do have a a second complaint as well - that it landed in -rc3,
>> which is rather late considering subsystem trees are usually forked
>> earlier than that. Had this been in -rc1, I would have had an easier
>> time applying the block bits for 4.15.
> 
> Yes, totally true. tglx and I ended up meeting face-to-face at the
> Kernel Recipes conference and we solved some outstanding design issues
> with the conversion. The timing meant the new API went into -rc3,
> which seemed better than missing an entire release cycle, or carrying
> deltas against maintainer trees that would drift. (This is actually my
> second massive refactoring of these changes...)

Honestly, I think the change should have waited for 4.15 in that case.
Why the rush? It wasn't ready for the merge window.

> If you don't want to deal with the -rc3 issue, would you want these
> changes to get carried in the timer tree instead?

I can carry them, not a problem. Just a bit grumpy as this seems poorly
handled.

-- 
Jens Axboe

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] block/laptop_mode: Convert timers to use timer_setup()

[Kees Cook]

On Thu, Oct 5, 2017 at 11:26 AM, Jens Axboe <axboe@kernel.dk> wrote:
> Honestly, I think the change should have waited for 4.15 in that case.
> Why the rush? It wasn't ready for the merge window.

My understanding from my discussion with tglx was that if the API
change waiting for 4.15, then the conversions would have to wait until
4.16. With most conversions only depending on the new API, it seemed
silly to wait another whole release when the work is just waiting to
be merged.

But yes, timing was not ideal. I did try to get it in earlier, but I
think tglx was busy with other concerns.

-Kees

-- 
Kees Cook
Pixel Security

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] block/laptop_mode: Convert timers to use timer_setup()

[Jens Axboe]

On 10/05/2017 12:49 PM, Kees Cook wrote:
> On Thu, Oct 5, 2017 at 11:26 AM, Jens Axboe <axboe@kernel.dk> wrote:
>> Honestly, I think the change should have waited for 4.15 in that case.
>> Why the rush? It wasn't ready for the merge window.
> 
> My understanding from my discussion with tglx was that if the API
> change waiting for 4.15, then the conversions would have to wait until
> 4.16. With most conversions only depending on the new API, it seemed
> silly to wait another whole release when the work is just waiting to
> be merged.

Right, it would have shifted everything a release. But that's how we do
things! If something isn't ready _before_ the merge window, let alone
talking -rc3 time, then it gets to wait unless it's fixing a regression
introduced in the merge window. Or if you can argue that it's a really
critical fix, sure, it can be squeezed in.

I'm puzzled why anyone would think that this is any different. What I'm
hearing here is that "I didn't want to wait, and my change is more
important than others", as an argument for why this should be treated
any differently.

I'm not saying the change is bad, it looks trivial, from_timer()
discussion aside. But it's not a critical fix, by any stretch of the
imagination, and neither are the driver conversions. With this
additionally causing extra problems because of the timing, that's just
further proof that it should have waited.

> But yes, timing was not ideal. I did try to get it in earlier, but I
> think tglx was busy with other concerns.

So it should have waited...

-- 
Jens Axboe

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] block/laptop_mode: Convert timers to use timer_setup()

[Thomas Gleixner]

On Thu, 5 Oct 2017, Jens Axboe wrote:
> On 10/05/2017 11:49 AM, Kees Cook wrote:
> > Yes, totally true. tglx and I ended up meeting face-to-face at the
> > Kernel Recipes conference and we solved some outstanding design issues
> > with the conversion. The timing meant the new API went into -rc3,
> > which seemed better than missing an entire release cycle, or carrying
> > deltas against maintainer trees that would drift. (This is actually my
> > second massive refactoring of these changes...)
> 
> Honestly, I think the change should have waited for 4.15 in that case.
> Why the rush? It wasn't ready for the merge window.

Come on. You know very well that a prerequisite for global changes which is
not yet used in Linus tree can get merged post merge windew in order to
avoid massive inter maintainer tree dependencies. We've done that before.

There are only a few options we have for such global changes:

   - Delay everything for a full release and keep hunting the ever changing
     code and the new users of old interfaces, which is a pain in the
     butt. I know what I'm talking about.

   - Apply everything in a central tree, which is prone to merge conflicts
     in next when maintainer trees contain changes in the same area. So
     getting it through the maintainers is the best option for this kind of
     stuff.

   - Create a separate branch for other maintainers to pull, which I did
     often enough in the past to avoid merge dependencies.  I decided not
     to offer that branch this time, because it would be been necessary to
     pull it into a gazillion of trees. So we decided that Linus tree as a
     dependency is good enough.

     Just for the record:

     Last time I did this for block you did not complain, that you got
     something based on -rc3 from me because you wanted to have these
     changes in your tree so you could apply the depending multi-queue
     patches. tip irq/for-block exists for a reason.

     So what's the difference to pull -rc3 this time? Nothing, just the
     fact that you are not interested in the change.

     So please stop this hypocritical ranting right here.

Thanks,

	tglx






--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] block/laptop_mode: Convert timers to use timer_setup()

[Jens Axboe]

On 10/05/2017 01:23 PM, Thomas Gleixner wrote:
> On Thu, 5 Oct 2017, Jens Axboe wrote:
>> On 10/05/2017 11:49 AM, Kees Cook wrote:
>>> Yes, totally true. tglx and I ended up meeting face-to-face at the
>>> Kernel Recipes conference and we solved some outstanding design issues
>>> with the conversion. The timing meant the new API went into -rc3,
>>> which seemed better than missing an entire release cycle, or carrying
>>> deltas against maintainer trees that would drift. (This is actually my
>>> second massive refactoring of these changes...)
>>
>> Honestly, I think the change should have waited for 4.15 in that case.
>> Why the rush? It wasn't ready for the merge window.
> 
> Come on. You know very well that a prerequisite for global changes which is
> not yet used in Linus tree can get merged post merge windew in order to
> avoid massive inter maintainer tree dependencies. We've done that before.

My point is that doing it this late makes things harder than they should
have been. If this was in for -rc1, it would have made things a lot
easier. Or even -rc2. I try and wait to fork off the block tree for as
long as I can, -rc2 is generally where that happens.

> There are only a few options we have for such global changes:
> 
>    - Delay everything for a full release and keep hunting the ever changing
>      code and the new users of old interfaces, which is a pain in the
>      butt. I know what I'm talking about.

Not disagreeing with that it's a pain in the butt. The timing is the
main reason why that is the case, though.

>    - Apply everything in a central tree, which is prone to merge conflicts
>      in next when maintainer trees contain changes in the same area. So
>      getting it through the maintainers is the best option for this kind of
>      stuff.

Don't disagree with that either.

>    - Create a separate branch for other maintainers to pull, which I did
>      often enough in the past to avoid merge dependencies.  I decided not
>      to offer that branch this time, because it would be been necessary to
>      pull it into a gazillion of trees. So we decided that Linus tree as a
>      dependency is good enough.
> 
>      Just for the record:
> 
>      Last time I did this for block you did not complain, that you got
>      something based on -rc3 from me because you wanted to have these
>      changes in your tree so you could apply the depending multi-queue
>      patches. tip irq/for-block exists for a reason.
> 
>      So what's the difference to pull -rc3 this time? Nothing, just the
>      fact that you are not interested in the change.
> 
>      So please stop this hypocritical ranting right here.

I'm not judging this based on whether I find it interesting or not, but
rather if it's something that's generally important to get in. Maybe it
is, but I don't see any justification for that at all. So just looking
at the isolated change, it does not strike me as something that's
important enough to warrant special treatment (and the pain associated
with that). I don't care about the core change, it's obviously trivial.
Expecting maintainers to pick up this dependency asap mid cycle is what
sucks.

Please stop accusing me of being hypocritical. I'm questionning the
timing of the change, that should be possible without someone resorting
to ad hominem attacks.

-- 
Jens Axboe

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] block/laptop_mode: Convert timers to use timer_setup()

[Jens Axboe]

On 10/05/2017 01:41 PM, Jens Axboe wrote:
> On 10/05/2017 01:23 PM, Thomas Gleixner wrote:
>> On Thu, 5 Oct 2017, Jens Axboe wrote:
>>> On 10/05/2017 11:49 AM, Kees Cook wrote:
>>>> Yes, totally true. tglx and I ended up meeting face-to-face at the
>>>> Kernel Recipes conference and we solved some outstanding design issues
>>>> with the conversion. The timing meant the new API went into -rc3,
>>>> which seemed better than missing an entire release cycle, or carrying
>>>> deltas against maintainer trees that would drift. (This is actually my
>>>> second massive refactoring of these changes...)
>>>
>>> Honestly, I think the change should have waited for 4.15 in that case.
>>> Why the rush? It wasn't ready for the merge window.
>>
>> Come on. You know very well that a prerequisite for global changes which is
>> not yet used in Linus tree can get merged post merge windew in order to
>> avoid massive inter maintainer tree dependencies. We've done that before.
> 
> My point is that doing it this late makes things harder than they should
> have been. If this was in for -rc1, it would have made things a lot
> easier. Or even -rc2. I try and wait to fork off the block tree for as
> long as I can, -rc2 is generally where that happens.

Timing of the change aside, this patch doesn't even apply to the 4.15
block tree:

checking file block/blk-core.c
checking file include/linux/writeback.h
Hunk #1 succeeded at 309 (offset -20 lines).
checking file mm/page-writeback.c
Hunk #1 FAILED at 1977.
Hunk #2 FAILED at 1988.
2 out of 2 hunks FAILED

-- 
Jens Axboe

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] block/laptop_mode: Convert timers to use timer_setup()

[Thomas Gleixner]

Jens,

On Thu, 5 Oct 2017, Jens Axboe wrote:
> On 10/05/2017 01:23 PM, Thomas Gleixner wrote:
> > Come on. You know very well that a prerequisite for global changes which is
> > not yet used in Linus tree can get merged post merge windew in order to
> > avoid massive inter maintainer tree dependencies. We've done that before.
> 
> My point is that doing it this late makes things harder than they should
> have been. If this was in for -rc1, it would have made things a lot
> easier. Or even -rc2. I try and wait to fork off the block tree for as
> long as I can, -rc2 is generally where that happens.

Well, yes. I know it's about habits. There is no real technical reason not
to merge -rc3 or later into your devel/next branch. I actually do that for
various reasons, one being that I prefer to have halfways testable
branches, which is often not the case when they are based of rc1, which is
especially true in this 4.14 cycle. The other is to pick up stuff which
went into Linus tree via a urgent branch or even got applied from mail
directly.

> I'm not judging this based on whether I find it interesting or not, but
> rather if it's something that's generally important to get in. Maybe it
> is, but I don't see any justification for that at all. So just looking
> at the isolated change, it does not strike me as something that's
> important enough to warrant special treatment (and the pain associated
> with that). I don't care about the core change, it's obviously trivial.
> Expecting maintainers to pick up this dependency asap mid cycle is what
> sucks.

I'm really not getting the 'pain' point.

'git merge linus' is not really a pain and it does not break workflows
assumed that you do that on a branch which has immutable state. If you want
to keep your branches open for rebasing due to some wreckage in the middle
of it, that's a different story.

> Please stop accusing me of being hypocritical. I'm questionning the
> timing of the change, that should be possible without someone resorting
> to ad hominem attacks.

Well, it seemed hypocritical to me for a hopefully understandable reason. I
didn't want to attack or offend you in any way.

I just know from repeated experience how painful it is to do full tree
overhauls and sit on large patch queues for a long time. There is some
point where you need to get things going and I really appreciate the work
of people doing that. Refactoring the kernel to get rid of legacy burdens
and in this case to address a popular attack vector is definitely useful
for everybody and should be supported. We tried to make it easy by pushing
this to Linus and I really did not expect that merging Linus -rc3 into a
devel/next branch is a painful work to do.

As Kees said already, we can set that particular patch aside and push it
along with the rest of ignored ones around 15-rc1 time so we can remove the
old interfaces. Though we hopefully wont end up with a gazillion of ignored
or considered too painful ones.

Thanks,

	tglx

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] block/laptop_mode: Convert timers to use timer_setup()

[Jens Axboe]

On 10/05/2017 03:53 PM, Thomas Gleixner wrote:
> Jens,
> 
> On Thu, 5 Oct 2017, Jens Axboe wrote:
>> On 10/05/2017 01:23 PM, Thomas Gleixner wrote:
>>> Come on. You know very well that a prerequisite for global changes which is
>>> not yet used in Linus tree can get merged post merge windew in order to
>>> avoid massive inter maintainer tree dependencies. We've done that before.
>>
>> My point is that doing it this late makes things harder than they should
>> have been. If this was in for -rc1, it would have made things a lot
>> easier. Or even -rc2. I try and wait to fork off the block tree for as
>> long as I can, -rc2 is generally where that happens.
> 
> Well, yes. I know it's about habits. There is no real technical reason not
> to merge -rc3 or later into your devel/next branch. I actually do that for
> various reasons, one being that I prefer to have halfways testable
> branches, which is often not the case when they are based of rc1, which is
> especially true in this 4.14 cycle. The other is to pick up stuff which
> went into Linus tree via a urgent branch or even got applied from mail
> directly.

Yes, it's not impossible, I just usually prefer not to. For this case, I
just setup a for-4.15/timer, that is the current block branch with -rc3
pulled in. I applied the two patches for floppy and amiflop, I'm
assuming Kees will respin the writeback/laptop version and I can shove
that in there too.

>> I'm not judging this based on whether I find it interesting or not, but
>> rather if it's something that's generally important to get in. Maybe it
>> is, but I don't see any justification for that at all. So just looking
>> at the isolated change, it does not strike me as something that's
>> important enough to warrant special treatment (and the pain associated
>> with that). I don't care about the core change, it's obviously trivial.
>> Expecting maintainers to pick up this dependency asap mid cycle is what
>> sucks.
> 
> I'm really not getting the 'pain' point.
> 
> 'git merge linus' is not really a pain and it does not break workflows
> assumed that you do that on a branch which has immutable state. If you want
> to keep your branches open for rebasing due to some wreckage in the middle
> of it, that's a different story.

I try never to rebase the development branches. It'll happen very
rarely, but only for cases where the screwup has been short and I'm
assuming/hoping no one pulled it yet.

>> Please stop accusing me of being hypocritical. I'm questionning the
>> timing of the change, that should be possible without someone resorting
>> to ad hominem attacks.
> 
> Well, it seemed hypocritical to me for a hopefully understandable reason. I
> didn't want to attack or offend you in any way.
> 
> I just know from repeated experience how painful it is to do full tree
> overhauls and sit on large patch queues for a long time. There is some
> point where you need to get things going and I really appreciate the work
> of people doing that. Refactoring the kernel to get rid of legacy burdens
> and in this case to address a popular attack vector is definitely useful
> for everybody and should be supported. We tried to make it easy by pushing
> this to Linus and I really did not expect that merging Linus -rc3 into a
> devel/next branch is a painful work to do.
> 
> As Kees said already, we can set that particular patch aside and push it
> along with the rest of ignored ones around 15-rc1 time so we can remove the
> old interfaces. Though we hopefully wont end up with a gazillion of ignored
> or considered too painful ones.

No worries, we'll get over it. The new branch is setup, so as soon as
the patches are funneled in, hopefully it can be ignored/forgotten until
the merge window opens.

-- 
Jens Axboe

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] block/laptop_mode: Convert timers to use timer_setup()

[Kees Cook]

On Thu, Oct 5, 2017 at 3:07 PM, Jens Axboe <axboe@kernel.dk> wrote:
> Yes, it's not impossible, I just usually prefer not to. For this case, I
> just setup a for-4.15/timer, that is the current block branch with -rc3
> pulled in. I applied the two patches for floppy and amiflop, I'm
> assuming Kees will respin the writeback/laptop version and I can shove
> that in there too.

Thanks for setting this up! I'll respin and send it out again.

-Kees

-- 
Kees Cook
Pixel Security

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>