Re: [PATCH] mm: Move maxable seq_file logic into a single place

From: Chris Down
Date: Thu Jan 24 2019 - 11:56:39 EST


Johannes Weiner writes:
I think this increases complexity more than it saves LOC,
unfortunately.

The current situation is a bit repetitive, but much more obviously
correct. And we're not planning on adding many more of those memcg
interface files, so I this doesn't seem to be an improvement re:
maintainability and future extensibility of the code.

Hmm, my main goal in the patch was not really reduction of LOC, but more around centralising logic so that it's clear where these functions are unique and where they are completely the same. My initial reaction upon reading these was mostly to feel like I'm missing something due to the large amount of similarity between them, wondering if the change in mem_cgroup/page_counter access is really the only thing to look at, so my goal was primarily to reduce confusion (although of course this is subjective, I can also see your point about how having "memory.low" and the like without context can also be confusing).

I think using a macro is not ideal, but unfortunately without it a lot of the complexity can't really be abstracted easily.

If not this, what would you think about a patch that adds two new functions:

1. mem_cgroup_from_seq
2. mem_cgroup_write_max_or_val

This won't be able to reduce as much of the overlap as the macro, but it still will centralise a lot of the logic.