On Wed, Sep 27, 2017 at 5:41 AM, Jens Axboe <axboe@xxxxxxxxx> wrote: > > So I reworked the series, to include three prep patches that end up > killing off free_more_memory(). This means that we don't have to do the > 1024 -> 0 change in there. On top of that, I added a separate bit to > manage range cyclic vs non range cyclic flush all work. This means that > we don't have to worry about the laptop case either. > > I think that should quell any of the concerns in the patchset, you can > find the new series here: > > http://git.kernel.dk/cgit/linux-block/log/?h=wb-start-all Yeah, this I feel more confident about. It still has some subtle changes (nr_pages is slightly different for laptop mode, and the whole "we rely on the VM to not return NULL") but I cannot for the life of me imagine that those changes are noticeable or meaningful. So now that last commit that limits the pending flushes is the only one that seems to matter, and the one you wanted to build up to. It's not a subtle change, but that's the one that fixes the actual bug you see, so now the rest of the series really looks like "prep-work for the bug fix". Of course, with the change to support range_cycling for the laptop mode case, there's actually possibly _two_ pending full flushes (the range-cyclic and the "all" one), so that last changelog may not be entirely accurate. Long-term, I would prefer to maybe make the "range_whole" case to always be range-cyclic, because I suspect that's the better behavior, but I think that series is the one that changes existing semantics the least for the existing cases, so I think your approach is better for now. As far as I can tell, the cases that do not set range_cyclic right now are: - wakeup_flusher_threads() - sync_inodes_sb() and in neither case does that lack of range_cyclic really seem to make any sense. It might be a purely historical artifact (an _old_ one: that range_cyclic logic goes back to 2006, as far as I can tell). But there might also be something I'm missing. Anyway, your series looks fine to me. Linus
