On 09/25/2017 03:35 AM, Jan Kara wrote: > On Thu 21-09-17 10:00:25, Jens Axboe wrote: >> On 09/21/2017 09:36 AM, Jens Axboe wrote: >>>> But more importantly once we are not guaranteed that we only have >>>> a single global wb_writeback_work per bdi_writeback we should just >>>> embedd that into struct bdi_writeback instead of dynamically >>>> allocating it. >>> >>> We could do this as a followup. But right now the logic is that we >>> can have on started (inflight), and still have one new queued. >> >> Something like the below would fit on top to do that. Gets rid of the >> allocation and embeds the work item for global start-all in the >> bdi_writeback structure. > > Hum, so when we consider stuff like embedded work item, I would somewhat > prefer to handle this like we do for for_background and for_kupdate style > writeback so that we don't have another special case. For these don't queue > any item, we just queue writeback work into the workqueue (via > wb_wakeup()). When flusher work gets processed wb_do_writeback() checks > (after processing all normal writeback requests) whether conditions for > these special writeback styles are met and if yes, it creates on-stack work > item and processes it (see wb_check_old_data_flush() and > wb_check_background_flush()). Thanks Jan, I think that's a really good suggestion and kills the special case completely. I'll rework the patch as a small series for 4.15. > So in this case we would just set some flag in bdi_writeback when memory > reclaim needs help and wb_do_writeback() would check for this flag and > create and process writeback-all style writeback work. Granted this does > not preserve ordering of requests (basically any specific request gets > priority over writeback-whole-world request) but memory gets cleaned in > either case so flusher should be doing what is needed. I don't think that matters, and we're already mostly there since we reject a request if one is pending. And at this point they are all identical "start all writeback" requests. -- Jens Axboe
