| Message ID | 1459350477-16404-5-git-send-email-axboe@fb.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed 30-03-16 09:07:52, Jens Axboe wrote: > Note in the bdi_writeback structure if a task is currently being > limited in balance_dirty_pages(), waiting for writeback to > proceed. ... > diff --git a/mm/page-writeback.c b/mm/page-writeback.c > index 11ff8f758631..15e696bc5d14 100644 > --- a/mm/page-writeback.c > +++ b/mm/page-writeback.c > @@ -1746,7 +1746,9 @@ pause: > pause, > start_time); > __set_current_state(TASK_KILLABLE); > + wb->dirty_sleeping = 1; > io_schedule_timeout(pause); > + wb->dirty_sleeping = 0; Huh, but wb->dirty_sleeping is shared by all the processes in the system. So this is seriously racy, isn't it? You rather need a counter for this to work. Honza
On 04/13/2016 07:08 AM, Jan Kara wrote: > On Wed 30-03-16 09:07:52, Jens Axboe wrote: >> Note in the bdi_writeback structure if a task is currently being >> limited in balance_dirty_pages(), waiting for writeback to >> proceed. > ... >> diff --git a/mm/page-writeback.c b/mm/page-writeback.c >> index 11ff8f758631..15e696bc5d14 100644 >> --- a/mm/page-writeback.c >> +++ b/mm/page-writeback.c >> @@ -1746,7 +1746,9 @@ pause: >> pause, >> start_time); >> __set_current_state(TASK_KILLABLE); >> + wb->dirty_sleeping = 1; >> io_schedule_timeout(pause); >> + wb->dirty_sleeping = 0; > > Huh, but wb->dirty_sleeping is shared by all the processes in the system. > So this is seriously racy, isn't it? You rather need a counter for this to > work. Sure, but it's not _that_ important. It's like wb->dirty_exceeded, we have an equally relaxed relationship. I don't mind making it more solid, but I can't make it a counter without making it atomic. Which is why I left it as just a basic assignment. But I guess since we only fiddle with it when going to sleep, we can make it an atomic and not have to worry about the potential impact.
diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h index 1b4d69f68c33..f702309216b4 100644 --- a/include/linux/backing-dev-defs.h +++ b/include/linux/backing-dev-defs.h @@ -116,6 +116,8 @@ struct bdi_writeback { struct list_head work_list; struct delayed_work dwork; /* work item used for writeback */ + int dirty_sleeping; /* waiting on dirty limit exceeded */ + struct list_head bdi_node; /* anchored at bdi->wb_list */ #ifdef CONFIG_CGROUP_WRITEBACK diff --git a/mm/page-writeback.c b/mm/page-writeback.c index 11ff8f758631..15e696bc5d14 100644 --- a/mm/page-writeback.c +++ b/mm/page-writeback.c @@ -1746,7 +1746,9 @@ pause: pause, start_time); __set_current_state(TASK_KILLABLE); + wb->dirty_sleeping = 1; io_schedule_timeout(pause); + wb->dirty_sleeping = 0; current->dirty_paused_when = now + pause; current->nr_dirtied = 0;
Note in the bdi_writeback structure if a task is currently being limited in balance_dirty_pages(), waiting for writeback to proceed. Signed-off-by: Jens Axboe <axboe@fb.com> --- include/linux/backing-dev-defs.h | 2 ++ mm/page-writeback.c | 2 ++ 2 files changed, 4 insertions(+)