Re: Possible race in obj_stock_flush_required() vs drain_obj_stock()

[Michal Hocko <mhocko@suse.com>]

On Mon 03-10-22 17:09:15, Alexander Fedorov wrote:
> On 03.10.2022 16:32, Michal Hocko wrote:
> > On Mon 03-10-22 15:47:10, Alexander Fedorov wrote:
> >> @@ -3197,17 +3197,30 @@ static void drain_obj_stock(struct memcg_stock_pcp *stock)
> >>  		stock->nr_bytes = 0;
> >>  	}
> >>  
> >> -	obj_cgroup_put(old);
> >> +	/*
> >> +	 * Clear pointer before freeing memory so that
> >> +	 * drain_all_stock() -> obj_stock_flush_required()
> >> +	 * does not see a freed pointer.
> >> +	 */
> >>  	stock->cached_objcg = NULL;
> >> +	obj_cgroup_put(old);
> > 
> > Do we need barrier() or something else to ensure there is no reordering?
> > I am not reallyu sure what kind of barriers are implied by the pcp ref
> > counting.
> 
> obj_cgroup_put() -> kfree_rcu() -> synchronize_rcu() should take care
> of this:

This is a very subtle guarantee. Also it would only apply if this is the
last reference, right? Is there any reason to not use
	WRITE_ONCE(stock->cached_objcg, NULL);
	obj_cgroup_put(old);

IIRC this should prevent any reordering. 
-- 
Michal Hocko
SUSE Labs