On Fri, Apr 1, 2022 at 9:11 AM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote: > > On Thu, 31 Mar 2022 09:46:52 +0200 Michal Hocko <mhocko@xxxxxxxx> wrote: > > > On Thu 31-03-22 06:39:56, Shakeel Butt wrote: > > > On Wed, Mar 30, 2022 at 07:48:45PM -0700, Roman Gushchin wrote: > > > > > > > > > > > [...] > > > > > > > > > > > > But honestly, I’d drop the original optimization together with > > > > the fix, if only there is no _real world_ data on the problem and > > > > the improvement. It seems like it has started as a nice simple > > > > improvement, but the race makes it complex and probably not worth > > > > the added complexity and fragility. > > > > > > I agree with dropping the original optimization as it is not really > > > fixing an observed issue which may justify adding some complexity. > > > > Completely agreed. The patch as it is proposed is not really acceptable > > IMHO and I have to say I am worried that this is not the first time we > > are in a situation when a follow up fixes or unrelated patches are > > growing in complexity to fit on top of a performance optimizations which > > do not refer to any actual numbers. > > Yup. I did this: > > From: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > Subject: mm/list_lru.c: revert "mm/list_lru: optimize memcg_reparent_list_lru_node()" > > 405cc51fc1049c73 ("mm/list_lru: optimize memcg_reparent_list_lru_node()") > has subtle races which are proving ugly to fix. Revert the original > optimization. If quantitative testing indicates that we have a > significant problem here then other implementations can be looked at. > > Fixes: 405cc51fc1049c73 ("mm/list_lru: optimize memcg_reparent_list_lru_node()") > Cc: Waiman Long <longman@xxxxxxxxxx> > Cc: Roman Gushchin <roman.gushchin@xxxxxxxxx> > Cc: Muchun Song <songmuchun@xxxxxxxxxxxxx> > Cc: Michal Hocko <mhocko@xxxxxxxx> > Cc: Johannes Weiner <hannes@xxxxxxxxxxx> > Cc: Shakeel Butt <shakeelb@xxxxxxxxxx> > Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> Reviewed-by: Muchun Song <songmuchun@xxxxxxxxxxxxx>