linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* mm/list_lru.c:514:3-8: WARNING: NULL check before some freeing functions is not needed.
@ 2024-12-29  1:59 kernel test robot
  2024-12-29  9:39 ` Kairui Song
  0 siblings, 1 reply; 2+ messages in thread
From: kernel test robot @ 2024-12-29  1:59 UTC (permalink / raw)
  To: Kairui Song
  Cc: oe-kbuild-all, linux-kernel, Andrew Morton, Linux Memory Management List

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head:   059dd502b263d8a4e2a84809cf1068d6a3905e6f
commit: 28e98022b31efdb8f1ba310d938cd9b97ededfe4 mm/list_lru: simplify reparenting and initial allocation
date:   7 weeks ago
config: i386-randconfig-054-20241229 (https://download.01.org/0day-ci/archive/20241229/202412290924.UTP7GH2Z-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412290924.UTP7GH2Z-lkp@intel.com/

cocci warnings: (new ones prefixed by >>)
>> mm/list_lru.c:514:3-8: WARNING: NULL check before some freeing functions is not needed.

vim +514 mm/list_lru.c

   470	
   471	int memcg_list_lru_alloc(struct mem_cgroup *memcg, struct list_lru *lru,
   472				 gfp_t gfp)
   473	{
   474		unsigned long flags;
   475		struct list_lru_memcg *mlru;
   476		struct mem_cgroup *pos, *parent;
   477		XA_STATE(xas, &lru->xa, 0);
   478	
   479		if (!list_lru_memcg_aware(lru) || memcg_list_lru_allocated(memcg, lru))
   480			return 0;
   481	
   482		gfp &= GFP_RECLAIM_MASK;
   483		/*
   484		 * Because the list_lru can be reparented to the parent cgroup's
   485		 * list_lru, we should make sure that this cgroup and all its
   486		 * ancestors have allocated list_lru_memcg.
   487		 */
   488		do {
   489			/*
   490			 * Keep finding the farest parent that wasn't populated
   491			 * until found memcg itself.
   492			 */
   493			pos = memcg;
   494			parent = parent_mem_cgroup(pos);
   495			while (!memcg_list_lru_allocated(parent, lru)) {
   496				pos = parent;
   497				parent = parent_mem_cgroup(pos);
   498			}
   499	
   500			mlru = memcg_init_list_lru_one(gfp);
   501			if (!mlru)
   502				return -ENOMEM;
   503			xas_set(&xas, pos->kmemcg_id);
   504			do {
   505				xas_lock_irqsave(&xas, flags);
   506				if (!xas_load(&xas) && !css_is_dying(&pos->css)) {
   507					xas_store(&xas, mlru);
   508					if (!xas_error(&xas))
   509						mlru = NULL;
   510				}
   511				xas_unlock_irqrestore(&xas, flags);
   512			} while (xas_nomem(&xas, gfp));
   513			if (mlru)
 > 514				kfree(mlru);
   515		} while (pos != memcg && !css_is_dying(&pos->css));
   516	
   517		return xas_error(&xas);
   518	}
   519	#else
   520	static inline void memcg_init_list_lru(struct list_lru *lru, bool memcg_aware)
   521	{
   522	}
   523	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: mm/list_lru.c:514:3-8: WARNING: NULL check before some freeing functions is not needed.
  2024-12-29  1:59 mm/list_lru.c:514:3-8: WARNING: NULL check before some freeing functions is not needed kernel test robot
@ 2024-12-29  9:39 ` Kairui Song
  0 siblings, 0 replies; 2+ messages in thread
From: Kairui Song @ 2024-12-29  9:39 UTC (permalink / raw)
  To: kernel test robot
  Cc: oe-kbuild-all, linux-kernel, Andrew Morton, Linux Memory Management List

On Sun, Dec 29, 2024 at 10:00 AM kernel test robot <lkp@intel.com> wrote:
>
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   059dd502b263d8a4e2a84809cf1068d6a3905e6f
> commit: 28e98022b31efdb8f1ba310d938cd9b97ededfe4 mm/list_lru: simplify reparenting and initial allocation
> date:   7 weeks ago
> config: i386-randconfig-054-20241229 (https://download.01.org/0day-ci/archive/20241229/202412290924.UTP7GH2Z-lkp@intel.com/config)
> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202412290924.UTP7GH2Z-lkp@intel.com/
>
> cocci warnings: (new ones prefixed by >>)
> >> mm/list_lru.c:514:3-8: WARNING: NULL check before some freeing functions is not needed.
>
> vim +514 mm/list_lru.c
>
>    470
>    471  int memcg_list_lru_alloc(struct mem_cgroup *memcg, struct list_lru *lru,
>    472                           gfp_t gfp)
>    473  {
>    474          unsigned long flags;
>    475          struct list_lru_memcg *mlru;
>    476          struct mem_cgroup *pos, *parent;
>    477          XA_STATE(xas, &lru->xa, 0);
>    478
>    479          if (!list_lru_memcg_aware(lru) || memcg_list_lru_allocated(memcg, lru))
>    480                  return 0;
>    481
>    482          gfp &= GFP_RECLAIM_MASK;
>    483          /*
>    484           * Because the list_lru can be reparented to the parent cgroup's
>    485           * list_lru, we should make sure that this cgroup and all its
>    486           * ancestors have allocated list_lru_memcg.
>    487           */
>    488          do {
>    489                  /*
>    490                   * Keep finding the farest parent that wasn't populated
>    491                   * until found memcg itself.
>    492                   */
>    493                  pos = memcg;
>    494                  parent = parent_mem_cgroup(pos);
>    495                  while (!memcg_list_lru_allocated(parent, lru)) {
>    496                          pos = parent;
>    497                          parent = parent_mem_cgroup(pos);
>    498                  }
>    499
>    500                  mlru = memcg_init_list_lru_one(gfp);
>    501                  if (!mlru)
>    502                          return -ENOMEM;
>    503                  xas_set(&xas, pos->kmemcg_id);
>    504                  do {
>    505                          xas_lock_irqsave(&xas, flags);
>    506                          if (!xas_load(&xas) && !css_is_dying(&pos->css)) {
>    507                                  xas_store(&xas, mlru);
>    508                                  if (!xas_error(&xas))
>    509                                          mlru = NULL;
>    510                          }
>    511                          xas_unlock_irqrestore(&xas, flags);
>    512                  } while (xas_nomem(&xas, gfp));
>    513                  if (mlru)
>  > 514                          kfree(mlru);

Hmm, does it need a fix? mlru here is most likely always NULL, so in
most cases this should save a function call, a really micro
optimization.

Perhaps a "if (unlikely(mlru))" is better, and silence the bot?

>    515          } while (pos != memcg && !css_is_dying(&pos->css));
>    516
>    517          return xas_error(&xas);
>    518  }
>    519  #else
>    520  static inline void memcg_init_list_lru(struct list_lru *lru, bool memcg_aware)
>    521  {
>    522  }
>    523
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
>


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-12-29  9:39 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-12-29  1:59 mm/list_lru.c:514:3-8: WARNING: NULL check before some freeing functions is not needed kernel test robot
2024-12-29  9:39 ` Kairui Song

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox