From patchwork Fri Oct 6 08:20:20 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoph Hellwig X-Patchwork-Id: 9988423 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id C00266029B for ; Fri, 6 Oct 2017 08:20:25 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id AC6E228D65 for ; Fri, 6 Oct 2017 08:20:25 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9FAEC28D8B; Fri, 6 Oct 2017 08:20:25 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5682928D65 for ; Fri, 6 Oct 2017 08:20:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751705AbdJFIUX (ORCPT ); Fri, 6 Oct 2017 04:20:23 -0400 Received: from bombadil.infradead.org ([65.50.211.133]:47792 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750868AbdJFIUW (ORCPT ); Fri, 6 Oct 2017 04:20:22 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20170209; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=7tmiWCW1BrG13vV1xMljgnHdCfER4bwLpJ4YsLEiDAY=; b=s5gGnpfpRdlRbGftNvSJ/vVGy IfhkTQJcgt4pKAg9lzDKuo+erQToJ/kp34G+e6ifBFCucmaVseYi/3DY+SYbj5an8TJ8ah0jD45ds G0ye5CzM2XV9V4227u/iRQT2FL4tpRbZamz4q5hihrl/AjcezcOF1dNoEHhjKdrN92wDVZUoInvet yn3f+gh25esC3NwcoLKQvA40WeNJ8iUYytBh7aeFfkQ1hw/L/8Z1pyKDy2mvTJZQAXINC98HpfKyp ouC9Iu8L43lV3NCJtXt0/aKE60Iy2hB7IrjAItIsy7tNehmj/DBTB3VBDKmyZMuc95LseXLZm/HT9 7UqjR9Mlg==; Received: from hch by bombadil.infradead.org with local (Exim 4.87 #1 (Red Hat Linux)) id 1e0Nro-0001hw-23; Fri, 06 Oct 2017 08:20:20 +0000 Date: Fri, 6 Oct 2017 01:20:20 -0700 From: Christoph Hellwig To: Kees Cook Cc: Jens Axboe , linux-kernel@vger.kernel.org, Michal Hocko , Andrew Morton , Jan Kara , Johannes Weiner , Nicholas Piggin , Vladimir Davydov , Matthew Wilcox , Jeff Layton , linux-block@vger.kernel.org, linux-mm@kvack.org, Thomas Gleixner Subject: Re: [PATCH v2] block/laptop_mode: Convert timers to use timer_setup() Message-ID: <20171006082020.GA12192@infradead.org> References: <20171005231623.GA109154@beast> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20171005231623.GA109154@beast> User-Agent: Mutt/1.8.3 (2017-05-23) X-SRS-Rewrite: SMTP reverse-path rewritten from by bombadil.infradead.org. See http://www.infradead.org/rpr.html Sender: linux-block-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-block@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP > -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); > } This isn't the laptop_mode timer, although the change itself looks fine. > + timer_setup(&q->backing_dev_info->laptop_mode_wb_timer, > + laptop_mode_timer_fn, 0); And I already pointed out to Jens when he did the previous changes to this one that it has no business being in the block code, it really should move to mm/page-writeback.c with the rest of the handling of this timer. Once that is fixed up your automated script should pick it up, so we wouldn't need the manual change. Untested patch for that below: --- From 77881bd72b5fb1219fc74625b0380930f9c580df Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Fri, 6 Oct 2017 10:18:53 +0200 Subject: mm: move all laptop_mode handling to backing-dev.c It isn't block-device specific and oddly spread over multiple files at the moment: TODO: audit that the unregistration changes are fine Signed-off-by: Christoph Hellwig --- block/blk-core.c | 3 --- include/linux/writeback.h | 6 ------ mm/backing-dev.c | 36 ++++++++++++++++++++++++++++++++++++ mm/page-writeback.c | 36 ------------------------------------ 4 files changed, 36 insertions(+), 45 deletions(-) diff --git a/block/blk-core.c b/block/blk-core.c index 14f7674fa0b1..f5f916b28c40 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -662,7 +662,6 @@ void blk_cleanup_queue(struct request_queue *q) blk_flush_integrity(); /* @q won't process any more request, flush async actions */ - del_timer_sync(&q->backing_dev_info->laptop_mode_wb_timer); blk_sync_queue(q); if (q->mq_ops) @@ -841,8 +840,6 @@ 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); INIT_LIST_HEAD(&q->queue_head); INIT_LIST_HEAD(&q->timeout_list); diff --git a/include/linux/writeback.h b/include/linux/writeback.h index 9c0091678af4..e6ba35a5e1f7 100644 --- a/include/linux/writeback.h +++ b/include/linux/writeback.h @@ -327,14 +327,8 @@ static inline void cgroup_writeback_umount(void) /* * mm/page-writeback.c */ -#ifdef CONFIG_BLOCK 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); -#else -static inline void laptop_sync_completion(void) { } -#endif bool node_dirty_ok(struct pglist_data *pgdat); int wb_domain_init(struct wb_domain *dom, gfp_t gfp); #ifdef CONFIG_CGROUP_WRITEBACK diff --git a/mm/backing-dev.c b/mm/backing-dev.c index e19606bb41a0..cb36f07f2af2 100644 --- a/mm/backing-dev.c +++ b/mm/backing-dev.c @@ -822,6 +822,38 @@ static void cgwb_remove_from_bdi_list(struct bdi_writeback *wb) #endif /* CONFIG_CGROUP_WRITEBACK */ +static void laptop_mode_timer_fn(unsigned long data) +{ + struct backing_dev_info *bdi = (struct backing_dev_info *)data; + + wakeup_flusher_threads_bdi(bdi, WB_REASON_LAPTOP_TIMER); +} + +/* + * We've spun up the disk and we're in laptop mode: schedule writeback + * of all dirty data a few seconds from now. If the flush is already scheduled + * then push it back - the user is still using the disk. + */ +void laptop_io_completion(struct backing_dev_info *bdi) +{ + mod_timer(&bdi->laptop_mode_wb_timer, jiffies + laptop_mode); +} + +/* + * We're in laptop mode and we've just synced. The sync's writes will have + * caused another writeback to be scheduled by laptop_io_completion. + * Nothing needs to be written back anymore, so we unschedule the writeback. + */ +void laptop_sync_completion(void) +{ + struct backing_dev_info *bdi; + + rcu_read_lock(); + list_for_each_entry_rcu(bdi, &bdi_list, bdi_list) + del_timer(&bdi->laptop_mode_wb_timer); + rcu_read_unlock(); +} + static int bdi_init(struct backing_dev_info *bdi) { int ret; @@ -835,6 +867,8 @@ static int bdi_init(struct backing_dev_info *bdi) INIT_LIST_HEAD(&bdi->bdi_list); INIT_LIST_HEAD(&bdi->wb_list); init_waitqueue_head(&bdi->wb_waitq); + setup_timer(&bdi->laptop_mode_wb_timer, + laptop_mode_timer_fn, (unsigned long)bdi); ret = cgwb_bdi_init(bdi); @@ -916,6 +950,8 @@ EXPORT_SYMBOL(bdi_register_owner); */ static void bdi_remove_from_list(struct backing_dev_info *bdi) { + del_timer_sync(&bdi->laptop_mode_wb_timer); + spin_lock_bh(&bdi_lock); list_del_rcu(&bdi->bdi_list); spin_unlock_bh(&bdi_lock); diff --git a/mm/page-writeback.c b/mm/page-writeback.c index 8d1fc593bce8..f8fe90dc529d 100644 --- a/mm/page-writeback.c +++ b/mm/page-writeback.c @@ -1976,42 +1976,6 @@ int dirty_writeback_centisecs_handler(struct ctl_table *table, int write, return 0; } -#ifdef CONFIG_BLOCK -void laptop_mode_timer_fn(unsigned long data) -{ - struct request_queue *q = (struct request_queue *)data; - - wakeup_flusher_threads_bdi(q->backing_dev_info, WB_REASON_LAPTOP_TIMER); -} - -/* - * We've spun up the disk and we're in laptop mode: schedule writeback - * of all dirty data a few seconds from now. If the flush is already scheduled - * then push it back - the user is still using the disk. - */ -void laptop_io_completion(struct backing_dev_info *info) -{ - mod_timer(&info->laptop_mode_wb_timer, jiffies + laptop_mode); -} - -/* - * We're in laptop mode and we've just synced. The sync's writes will have - * caused another writeback to be scheduled by laptop_io_completion. - * Nothing needs to be written back anymore, so we unschedule the writeback. - */ -void laptop_sync_completion(void) -{ - struct backing_dev_info *bdi; - - rcu_read_lock(); - - list_for_each_entry_rcu(bdi, &bdi_list, bdi_list) - del_timer(&bdi->laptop_mode_wb_timer); - - rcu_read_unlock(); -} -#endif - /* * If ratelimit_pages is too high then we can get into dirty-data overload * if a large number of processes all perform writes at the same time.