On Thu, Nov 30, 2023 at 11:57 AM Matthew Wilcox willy@infradead.org wrote:
On Thu, Nov 30, 2023 at 11:40:18AM -0800, Nhat Pham wrote:
This patch changes list_lru interface so that the caller must explicitly specify numa node and memcg when adding and removing objects. The old list_lru_add() and list_lru_del() are renamed to list_lru_add_obj() and list_lru_del_obj(), respectively.
Wouldn't it be better to add list_lru_add_memcg() and list_lru_del_memcg() and have:
+bool list_lru_del(struct list_lru *lru, struct list_head *item) +{
int nid = page_to_nid(virt_to_page(item));
struct mem_cgroup *memcg = list_lru_memcg_aware(lru) ?
mem_cgroup_from_slab_obj(item) : NULL;
return list_lru_del_memcg(lru, item, nid, memcg);
+}
Seems like _most_ callers will want the original versions and only a few will want the explicit memcg/nid versions. No?
I actually did something along that line in earlier iterations of this patch series (albeit with poorer naming - __list_lru_add() instead of list_lru_add_memcg()). The consensus after some back and forth was that the original list_lru_add() was not a very good design (the better one was this new version that allows for explicit numa/memcg selection). So I agreed to fix it everywhere as a prep patch.
I don't have strong opinions here to be completely honest, but I do think this new API makes more sense (at the cost of quite a bit of elbow grease to fix every callsites and extra reviewing).