* [PATCH 0/2] writeback: start-all allocation elimination @ 2017-10-03 15:16 Jens Axboe 2017-10-03 15:16 ` [PATCH 1/2] writeback: eliminate work item allocation in bd_start_writeback() Jens Axboe 2017-10-03 15:16 ` [PATCH 2/2] sysctl: remove /proc/sys/vm/nr_pdflush_threads Jens Axboe 0 siblings, 2 replies; 8+ messages in thread From: Jens Axboe @ 2017-10-03 15:16 UTC (permalink / raw) To: linux-kernel, linux-fsdevel; +Cc: hannes, jack This is on top of the parts of the previous series that have been queued up. I've posted the first patch in a reply to Jan last week, this is a more official posting. No changes, it works for me. Last patch is just a cleanup, finally killing off nr_pdflush_threads, 8 years after we deprecated it. Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads | 5 b/fs/fs-writeback.c | 71 ++++++-------- b/include/linux/backing-dev-defs.h | 1 b/include/trace/events/writeback.h | 1 b/kernel/sysctl.c | 5 5 files changed, 35 insertions(+), 48 deletions(-) -- Jens Axboe ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] writeback: eliminate work item allocation in bd_start_writeback() 2017-10-03 15:16 [PATCH 0/2] writeback: start-all allocation elimination Jens Axboe @ 2017-10-03 15:16 ` Jens Axboe 2017-10-04 7:26 ` Jan Kara 2017-10-03 15:16 ` [PATCH 2/2] sysctl: remove /proc/sys/vm/nr_pdflush_threads Jens Axboe 1 sibling, 1 reply; 8+ messages in thread From: Jens Axboe @ 2017-10-03 15:16 UTC (permalink / raw) To: linux-kernel, linux-fsdevel; +Cc: hannes, jack, Jens Axboe Handle start-all writeback like we do periodic or kupdate style writeback - by marking the bdi_writeback as needing a full flush, and simply waking the thread. This eliminates the need to allocate and queue a specific work item just for this purpose. After this change, we truly only ever have one of them running at any point in time. We mark the need to start all flushes, and the writeback thread will clear it once it has processed the request. Signed-off-by: Jens Axboe <axboe@kernel.dk> --- fs/fs-writeback.c | 71 +++++++++++++++++++--------------------- include/linux/backing-dev-defs.h | 1 + include/trace/events/writeback.h | 1 - 3 files changed, 35 insertions(+), 38 deletions(-) diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c index 399619c97567..9e24d604c59c 100644 --- a/fs/fs-writeback.c +++ b/fs/fs-writeback.c @@ -53,7 +53,6 @@ struct wb_writeback_work { unsigned int for_background:1; unsigned int for_sync:1; /* sync(2) WB_SYNC_ALL writeback */ unsigned int auto_free:1; /* free on completion */ - unsigned int start_all:1; /* nr_pages == 0 (all) writeback */ enum wb_reason reason; /* why was writeback initiated? */ struct list_head list; /* pending work list */ @@ -947,8 +946,6 @@ static unsigned long get_nr_dirty_pages(void) static void wb_start_writeback(struct bdi_writeback *wb, enum wb_reason reason) { - struct wb_writeback_work *work; - if (!wb_has_dirty_io(wb)) return; @@ -958,35 +955,14 @@ static void wb_start_writeback(struct bdi_writeback *wb, enum wb_reason reason) * high frequency, causing pointless allocations of tons of * work items and keeping the flusher threads busy retrieving * that work. Ensure that we only allow one of them pending and - * inflight at the time. It doesn't matter if we race a little - * bit on this, so use the faster separate test/set bit variants. + * inflight at the time. */ - if (test_bit(WB_start_all, &wb->state)) + if (test_bit(WB_start_all, &wb->state) || + test_and_set_bit(WB_start_all, &wb->state)) return; - set_bit(WB_start_all, &wb->state); - - /* - * This is WB_SYNC_NONE writeback, so if allocation fails just - * wakeup the thread for old dirty data writeback - */ - work = kzalloc(sizeof(*work), - GFP_NOWAIT | __GFP_NOMEMALLOC | __GFP_NOWARN); - if (!work) { - clear_bit(WB_start_all, &wb->state); - trace_writeback_nowork(wb); - wb_wakeup(wb); - return; - } - - work->sync_mode = WB_SYNC_NONE; - work->nr_pages = wb_split_bdi_pages(wb, get_nr_dirty_pages()); - work->range_cyclic = 1; - work->reason = reason; - work->auto_free = 1; - work->start_all = 1; - - wb_queue_work(wb, work); + wb->start_all_reason = reason; + wb_wakeup(wb); } /** @@ -1838,14 +1814,6 @@ static struct wb_writeback_work *get_next_work_item(struct bdi_writeback *wb) list_del_init(&work->list); } spin_unlock_bh(&wb->work_lock); - - /* - * Once we start processing a work item that had !nr_pages, - * clear the wb state bit for that so we can allow more. - */ - if (work && work->start_all) - clear_bit(WB_start_all, &wb->state); - return work; } @@ -1901,6 +1869,30 @@ static long wb_check_old_data_flush(struct bdi_writeback *wb) return 0; } +static long wb_check_start_all(struct bdi_writeback *wb) +{ + long nr_pages; + + if (!test_bit(WB_start_all, &wb->state)) + return 0; + + nr_pages = get_nr_dirty_pages(); + if (nr_pages) { + struct wb_writeback_work work = { + .nr_pages = wb_split_bdi_pages(wb, nr_pages), + .sync_mode = WB_SYNC_NONE, + .range_cyclic = 1, + .reason = wb->start_all_reason, + }; + + nr_pages = wb_writeback(wb, &work); + } + + clear_bit(WB_start_all, &wb->state); + return nr_pages; +} + + /* * Retrieve work items and do the writeback they describe */ @@ -1917,6 +1909,11 @@ static long wb_do_writeback(struct bdi_writeback *wb) } /* + * Check for a flush-everything request + */ + wrote += wb_check_start_all(wb); + + /* * Check for periodic writeback, kupdated() style */ wrote += wb_check_old_data_flush(wb); diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h index 420de5c7c7f9..f0f1df29d6b8 100644 --- a/include/linux/backing-dev-defs.h +++ b/include/linux/backing-dev-defs.h @@ -116,6 +116,7 @@ struct bdi_writeback { struct fprop_local_percpu completions; int dirty_exceeded; + int start_all_reason; spinlock_t work_lock; /* protects work_list & dwork scheduling */ struct list_head work_list; diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h index 9b57f014d79d..19a0ea08e098 100644 --- a/include/trace/events/writeback.h +++ b/include/trace/events/writeback.h @@ -286,7 +286,6 @@ DEFINE_EVENT(writeback_class, name, \ TP_PROTO(struct bdi_writeback *wb), \ TP_ARGS(wb)) -DEFINE_WRITEBACK_EVENT(writeback_nowork); DEFINE_WRITEBACK_EVENT(writeback_wake_background); TRACE_EVENT(writeback_bdi_register, -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] writeback: eliminate work item allocation in bd_start_writeback() 2017-10-03 15:16 ` [PATCH 1/2] writeback: eliminate work item allocation in bd_start_writeback() Jens Axboe @ 2017-10-04 7:26 ` Jan Kara 2017-10-04 14:42 ` Jens Axboe 0 siblings, 1 reply; 8+ messages in thread From: Jan Kara @ 2017-10-04 7:26 UTC (permalink / raw) To: Jens Axboe; +Cc: linux-kernel, linux-fsdevel, hannes, jack On Tue 03-10-17 09:16:20, Jens Axboe wrote: > Handle start-all writeback like we do periodic or kupdate > style writeback - by marking the bdi_writeback as needing a full > flush, and simply waking the thread. This eliminates the need to > allocate and queue a specific work item just for this purpose. > > After this change, we truly only ever have one of them running at > any point in time. We mark the need to start all flushes, and the > writeback thread will clear it once it has processed the request. > > Signed-off-by: Jens Axboe <axboe@kernel.dk> Just one nit below. You can add: Reviewed-by: Jan Kara <jack@suse.cz> > diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h > index 420de5c7c7f9..f0f1df29d6b8 100644 > --- a/include/linux/backing-dev-defs.h > +++ b/include/linux/backing-dev-defs.h > @@ -116,6 +116,7 @@ struct bdi_writeback { > > struct fprop_local_percpu completions; > int dirty_exceeded; > + int start_all_reason; This should be 'enum wb_reason' instead of 'int'. Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] writeback: eliminate work item allocation in bd_start_writeback() 2017-10-04 7:26 ` Jan Kara @ 2017-10-04 14:42 ` Jens Axboe 0 siblings, 0 replies; 8+ messages in thread From: Jens Axboe @ 2017-10-04 14:42 UTC (permalink / raw) To: Jan Kara; +Cc: linux-kernel, linux-fsdevel, hannes On 10/04/2017 01:26 AM, Jan Kara wrote: >> diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h >> index 420de5c7c7f9..f0f1df29d6b8 100644 >> --- a/include/linux/backing-dev-defs.h >> +++ b/include/linux/backing-dev-defs.h >> @@ -116,6 +116,7 @@ struct bdi_writeback { >> >> struct fprop_local_percpu completions; >> int dirty_exceeded; >> + int start_all_reason; > > This should be 'enum wb_reason' instead of 'int'. Yes good point, I'll move the enum and change 'start_all_reason' to be of type enum wb_reason. Thanks for the review! -- Jens Axboe ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] sysctl: remove /proc/sys/vm/nr_pdflush_threads 2017-10-03 15:16 [PATCH 0/2] writeback: start-all allocation elimination Jens Axboe 2017-10-03 15:16 ` [PATCH 1/2] writeback: eliminate work item allocation in bd_start_writeback() Jens Axboe @ 2017-10-03 15:16 ` Jens Axboe 2017-10-04 7:20 ` Jan Kara 2017-10-06 13:49 ` Rakesh Pandit 1 sibling, 2 replies; 8+ messages in thread From: Jens Axboe @ 2017-10-03 15:16 UTC (permalink / raw) To: linux-kernel, linux-fsdevel; +Cc: hannes, jack, Jens Axboe This tunable has been obsolete since 2.6.32, and writes to the file have been failing and complaining in dmesg since then: nr_pdflush_threads exported in /proc is scheduled for removal That was 8 years ago. Remove the file ABI obsolete notice, and the sysfs file. Signed-off-by: Jens Axboe <axboe@kernel.dk> --- Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads | 5 ----- kernel/sysctl.c | 5 ----- 2 files changed, 10 deletions(-) delete mode 100644 Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads diff --git a/Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads b/Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads deleted file mode 100644 index b0b0eeb20fe3..000000000000 --- a/Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads +++ /dev/null @@ -1,5 +0,0 @@ -What: /proc/sys/vm/nr_pdflush_threads -Date: June 2012 -Contact: Wanpeng Li <liwp@linux.vnet.ibm.com> -Description: Since pdflush is replaced by per-BDI flusher, the interface of old pdflush - exported in /proc/sys/vm/ should be removed. diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 6648fbbb8157..a5dd8d82c253 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -1345,11 +1345,6 @@ static struct ctl_table vm_table[] = { .extra1 = &zero, }, { - .procname = "nr_pdflush_threads", - .mode = 0444 /* read-only */, - .proc_handler = pdflush_proc_obsolete, - }, - { .procname = "swappiness", .data = &vm_swappiness, .maxlen = sizeof(vm_swappiness), -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] sysctl: remove /proc/sys/vm/nr_pdflush_threads 2017-10-03 15:16 ` [PATCH 2/2] sysctl: remove /proc/sys/vm/nr_pdflush_threads Jens Axboe @ 2017-10-04 7:20 ` Jan Kara 2017-10-06 13:49 ` Rakesh Pandit 1 sibling, 0 replies; 8+ messages in thread From: Jan Kara @ 2017-10-04 7:20 UTC (permalink / raw) To: Jens Axboe; +Cc: linux-kernel, linux-fsdevel, hannes, jack On Tue 03-10-17 09:16:21, Jens Axboe wrote: > This tunable has been obsolete since 2.6.32, and writes to the > file have been failing and complaining in dmesg since then: > > nr_pdflush_threads exported in /proc is scheduled for removal > > That was 8 years ago. Remove the file ABI obsolete notice, and > the sysfs file. > > Signed-off-by: Jens Axboe <axboe@kernel.dk> Agreed. You can add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads | 5 ----- > kernel/sysctl.c | 5 ----- > 2 files changed, 10 deletions(-) > delete mode 100644 Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads > > diff --git a/Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads b/Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads > deleted file mode 100644 > index b0b0eeb20fe3..000000000000 > --- a/Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads > +++ /dev/null > @@ -1,5 +0,0 @@ > -What: /proc/sys/vm/nr_pdflush_threads > -Date: June 2012 > -Contact: Wanpeng Li <liwp@linux.vnet.ibm.com> > -Description: Since pdflush is replaced by per-BDI flusher, the interface of old pdflush > - exported in /proc/sys/vm/ should be removed. > diff --git a/kernel/sysctl.c b/kernel/sysctl.c > index 6648fbbb8157..a5dd8d82c253 100644 > --- a/kernel/sysctl.c > +++ b/kernel/sysctl.c > @@ -1345,11 +1345,6 @@ static struct ctl_table vm_table[] = { > .extra1 = &zero, > }, > { > - .procname = "nr_pdflush_threads", > - .mode = 0444 /* read-only */, > - .proc_handler = pdflush_proc_obsolete, > - }, > - { > .procname = "swappiness", > .data = &vm_swappiness, > .maxlen = sizeof(vm_swappiness), > -- > 2.7.4 > -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] sysctl: remove /proc/sys/vm/nr_pdflush_threads 2017-10-03 15:16 ` [PATCH 2/2] sysctl: remove /proc/sys/vm/nr_pdflush_threads Jens Axboe 2017-10-04 7:20 ` Jan Kara @ 2017-10-06 13:49 ` Rakesh Pandit 2017-10-06 14:17 ` Jens Axboe 1 sibling, 1 reply; 8+ messages in thread From: Rakesh Pandit @ 2017-10-06 13:49 UTC (permalink / raw) To: Jens Axboe; +Cc: linux-kernel, linux-fsdevel, hannes, jack Hi Jens, On Tue, Oct 03, 2017 at 09:16:21AM -0600, Jens Axboe wrote: > This tunable has been obsolete since 2.6.32, and writes to the > file have been failing and complaining in dmesg since then: > > nr_pdflush_threads exported in /proc is scheduled for removal > > That was 8 years ago. Remove the file ABI obsolete notice, and > the sysfs file. > > Signed-off-by: Jens Axboe <axboe@kernel.dk> > --- > Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads | 5 ----- > kernel/sysctl.c | 5 ----- > 2 files changed, 10 deletions(-) > delete mode 100644 Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads > > diff --git a/Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads b/Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads > deleted file mode 100644 > index b0b0eeb20fe3..000000000000 > --- a/Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads > +++ /dev/null > @@ -1,5 +0,0 @@ > -What: /proc/sys/vm/nr_pdflush_threads > -Date: June 2012 > -Contact: Wanpeng Li <liwp@linux.vnet.ibm.com> > -Description: Since pdflush is replaced by per-BDI flusher, the interface of old pdflush > - exported in /proc/sys/vm/ should be removed. > diff --git a/kernel/sysctl.c b/kernel/sysctl.c > index 6648fbbb8157..a5dd8d82c253 100644 > --- a/kernel/sysctl.c > +++ b/kernel/sysctl.c > @@ -1345,11 +1345,6 @@ static struct ctl_table vm_table[] = { > .extra1 = &zero, > }, > { > - .procname = "nr_pdflush_threads", > - .mode = 0444 /* read-only */, > - .proc_handler = pdflush_proc_obsolete, It seems you forgot to remove pdflush_proc_obsolete from: ./include/linux/backing-dev.h ./mm/backing-dev.c Best regards, Rakesh Pandit ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] sysctl: remove /proc/sys/vm/nr_pdflush_threads 2017-10-06 13:49 ` Rakesh Pandit @ 2017-10-06 14:17 ` Jens Axboe 0 siblings, 0 replies; 8+ messages in thread From: Jens Axboe @ 2017-10-06 14:17 UTC (permalink / raw) To: Rakesh Pandit; +Cc: linux-kernel, linux-fsdevel, hannes, jack On 10/06/2017 07:49 AM, Rakesh Pandit wrote: > Hi Jens, > > On Tue, Oct 03, 2017 at 09:16:21AM -0600, Jens Axboe wrote: >> This tunable has been obsolete since 2.6.32, and writes to the >> file have been failing and complaining in dmesg since then: >> >> nr_pdflush_threads exported in /proc is scheduled for removal >> >> That was 8 years ago. Remove the file ABI obsolete notice, and >> the sysfs file. >> >> Signed-off-by: Jens Axboe <axboe@kernel.dk> >> --- >> Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads | 5 ----- >> kernel/sysctl.c | 5 ----- >> 2 files changed, 10 deletions(-) >> delete mode 100644 Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads >> >> diff --git a/Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads b/Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads >> deleted file mode 100644 >> index b0b0eeb20fe3..000000000000 >> --- a/Documentation/ABI/obsolete/proc-sys-vm-nr_pdflush_threads >> +++ /dev/null >> @@ -1,5 +0,0 @@ >> -What: /proc/sys/vm/nr_pdflush_threads >> -Date: June 2012 >> -Contact: Wanpeng Li <liwp@linux.vnet.ibm.com> >> -Description: Since pdflush is replaced by per-BDI flusher, the interface of old pdflush >> - exported in /proc/sys/vm/ should be removed. >> diff --git a/kernel/sysctl.c b/kernel/sysctl.c >> index 6648fbbb8157..a5dd8d82c253 100644 >> --- a/kernel/sysctl.c >> +++ b/kernel/sysctl.c >> @@ -1345,11 +1345,6 @@ static struct ctl_table vm_table[] = { >> .extra1 = &zero, >> }, >> { >> - .procname = "nr_pdflush_threads", >> - .mode = 0444 /* read-only */, >> - .proc_handler = pdflush_proc_obsolete, > > It seems you forgot to remove pdflush_proc_obsolete from: > ./include/linux/backing-dev.h > ./mm/backing-dev.c Yeah, I missed that, thanks for letting me know. I've now removed them. -- Jens Axboe ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-10-06 14:17 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-10-03 15:16 [PATCH 0/2] writeback: start-all allocation elimination Jens Axboe 2017-10-03 15:16 ` [PATCH 1/2] writeback: eliminate work item allocation in bd_start_writeback() Jens Axboe 2017-10-04 7:26 ` Jan Kara 2017-10-04 14:42 ` Jens Axboe 2017-10-03 15:16 ` [PATCH 2/2] sysctl: remove /proc/sys/vm/nr_pdflush_threads Jens Axboe 2017-10-04 7:20 ` Jan Kara 2017-10-06 13:49 ` Rakesh Pandit 2017-10-06 14:17 ` Jens Axboe
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.