| Message ID | 1505850787-18311-2-git-send-email-axboe@kernel.dk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Sep 19, 2017 at 01:53:02PM -0600, Jens Axboe wrote: > This whole function is... interesting. Change the wakeup call > to the flusher threads to pass in nr_pages == 0, instead of > some random number of pages. This matches more closely what > similar cases do for memory shortage/reclaim. > > Signed-off-by: Jens Axboe <axboe@kernel.dk> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
On Tue 19-09-17 13:53:02, Jens Axboe wrote: > This whole function is... interesting. Change the wakeup call > to the flusher threads to pass in nr_pages == 0, instead of > some random number of pages. This matches more closely what > similar cases do for memory shortage/reclaim. > > Signed-off-by: Jens Axboe <axboe@kernel.dk> Ok, probably makes sense. You can add: Reviewed-by: Jan Kara <jack@suse.cz> BTW, after this nobody seems to use the number of pages for wakeup_flusher_threads() so can you just delete the argument for the function? After all system-wide wakeup is useful only for system wide sync(2) or memory reclaim so number of pages isn't very useful... Honza > --- > fs/buffer.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/buffer.c b/fs/buffer.c > index 170df856bdb9..9471a445e370 100644 > --- a/fs/buffer.c > +++ b/fs/buffer.c > @@ -260,7 +260,7 @@ static void free_more_memory(void) > struct zoneref *z; > int nid; > > - wakeup_flusher_threads(1024, WB_REASON_FREE_MORE_MEM); > + wakeup_flusher_threads(0, WB_REASON_FREE_MORE_MEM); > yield(); > > for_each_online_node(nid) { > -- > 2.7.4 >
On 09/20/2017 08:17 AM, Jan Kara wrote: > On Tue 19-09-17 13:53:02, Jens Axboe wrote: >> This whole function is... interesting. Change the wakeup call >> to the flusher threads to pass in nr_pages == 0, instead of >> some random number of pages. This matches more closely what >> similar cases do for memory shortage/reclaim. >> >> Signed-off-by: Jens Axboe <axboe@kernel.dk> > > Ok, probably makes sense. You can add: > > Reviewed-by: Jan Kara <jack@suse.cz> > > BTW, after this nobody seems to use the number of pages for > wakeup_flusher_threads() so can you just delete the argument for the > function? After all system-wide wakeup is useful only for system wide > sync(2) or memory reclaim so number of pages isn't very useful... Great observation! That's true, and if we kill that, it enables further cleanups down the line for patch 5 and 6. I have incorporated that.
diff --git a/fs/buffer.c b/fs/buffer.c index 170df856bdb9..9471a445e370 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -260,7 +260,7 @@ static void free_more_memory(void) struct zoneref *z; int nid; - wakeup_flusher_threads(1024, WB_REASON_FREE_MORE_MEM); + wakeup_flusher_threads(0, WB_REASON_FREE_MORE_MEM); yield(); for_each_online_node(nid) {
This whole function is... interesting. Change the wakeup call to the flusher threads to pass in nr_pages == 0, instead of some random number of pages. This matches more closely what similar cases do for memory shortage/reclaim. Signed-off-by: Jens Axboe <axboe@kernel.dk> --- fs/buffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)