All of lore.kernel.org
 help / color / mirror / Atom feed
From: kbuild test robot <lkp@intel.com>
To: Waiman Long <longman@redhat.com>
Cc: kbuild-all@lists.01.org, Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>, Will Deacon <will@kernel.org>,
	Christoph Lameter <cl@linux.com>,
	Pekka Enberg <penberg@kernel.org>,
	David Rientjes <rientjes@google.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Waiman Long <longman@redhat.com>
Subject: Re: [PATCH 3/3] mm/slub: Fix potential deadlock problem in slab_attr_store()
Date: Fri, 14 Feb 2020 00:48:44 +0800	[thread overview]
Message-ID: <202002140001.e1zVjKdc%lkp@intel.com> (raw)
In-Reply-To: <20200210204651.21674-4-longman@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 3348 bytes --]

Hi Waiman,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/locking/core]
[also build test ERROR on tip/auto-latest linus/master v5.6-rc1 next-20200213]
[cannot apply to arm-perf/for-next/perf]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Waiman-Long/locking-mutex-Add-mutex_timed_lock-to-solve-potential-deadlock-problems/20200213-063401
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 41f0e29190ac9e38099a37abd1a8a4cb1dc21233
config: x86_64-randconfig-s2-20200213 (attached as .config)
compiler: gcc-6 (Debian 6.3.0-18+deb9u1) 6.3.0 20170516
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   mm/slub.c: In function 'slab_attr_store':
>> mm/slub.c:5542:7: error: implicit declaration of function 'mutex_timed_lock' [-Werror=implicit-function-declaration]
      if (mutex_timed_lock(&slab_mutex, 100) < 0)
          ^~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/mutex_timed_lock +5542 mm/slub.c

  5519	
  5520	static ssize_t slab_attr_store(struct kobject *kobj,
  5521					struct attribute *attr,
  5522					const char *buf, size_t len)
  5523	{
  5524		struct slab_attribute *attribute;
  5525		struct kmem_cache *s;
  5526		int err;
  5527	
  5528		attribute = to_slab_attr(attr);
  5529		s = to_slab(kobj);
  5530	
  5531		if (!attribute->store)
  5532			return -EIO;
  5533	
  5534		err = attribute->store(s, buf, len);
  5535	#ifdef CONFIG_MEMCG
  5536		if (slab_state >= FULL && err >= 0 && is_root_cache(s)) {
  5537			struct kmem_cache *c;
  5538	
  5539			/*
  5540			 * Timeout after 100ms
  5541			 */
> 5542			if (mutex_timed_lock(&slab_mutex, 100) < 0)
  5543				return -EBUSY;
  5544	
  5545			if (s->max_attr_size < len)
  5546				s->max_attr_size = len;
  5547	
  5548			/*
  5549			 * This is a best effort propagation, so this function's return
  5550			 * value will be determined by the parent cache only. This is
  5551			 * basically because not all attributes will have a well
  5552			 * defined semantics for rollbacks - most of the actions will
  5553			 * have permanent effects.
  5554			 *
  5555			 * Returning the error value of any of the children that fail
  5556			 * is not 100 % defined, in the sense that users seeing the
  5557			 * error code won't be able to know anything about the state of
  5558			 * the cache.
  5559			 *
  5560			 * Only returning the error code for the parent cache at least
  5561			 * has well defined semantics. The cache being written to
  5562			 * directly either failed or succeeded, in which case we loop
  5563			 * through the descendants with best-effort propagation.
  5564			 */
  5565			for_each_memcg_cache(c, s)
  5566				attribute->store(c, buf, len);
  5567			mutex_unlock(&slab_mutex);
  5568		}
  5569	#endif
  5570		return err;
  5571	}
  5572	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36070 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: kbuild test robot <lkp@intel.com>
To: kbuild-all@lists.01.org
Subject: Re: [PATCH 3/3] mm/slub: Fix potential deadlock problem in slab_attr_store()
Date: Fri, 14 Feb 2020 00:48:44 +0800	[thread overview]
Message-ID: <202002140001.e1zVjKdc%lkp@intel.com> (raw)
In-Reply-To: <20200210204651.21674-4-longman@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 3440 bytes --]

Hi Waiman,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/locking/core]
[also build test ERROR on tip/auto-latest linus/master v5.6-rc1 next-20200213]
[cannot apply to arm-perf/for-next/perf]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Waiman-Long/locking-mutex-Add-mutex_timed_lock-to-solve-potential-deadlock-problems/20200213-063401
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 41f0e29190ac9e38099a37abd1a8a4cb1dc21233
config: x86_64-randconfig-s2-20200213 (attached as .config)
compiler: gcc-6 (Debian 6.3.0-18+deb9u1) 6.3.0 20170516
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   mm/slub.c: In function 'slab_attr_store':
>> mm/slub.c:5542:7: error: implicit declaration of function 'mutex_timed_lock' [-Werror=implicit-function-declaration]
      if (mutex_timed_lock(&slab_mutex, 100) < 0)
          ^~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/mutex_timed_lock +5542 mm/slub.c

  5519	
  5520	static ssize_t slab_attr_store(struct kobject *kobj,
  5521					struct attribute *attr,
  5522					const char *buf, size_t len)
  5523	{
  5524		struct slab_attribute *attribute;
  5525		struct kmem_cache *s;
  5526		int err;
  5527	
  5528		attribute = to_slab_attr(attr);
  5529		s = to_slab(kobj);
  5530	
  5531		if (!attribute->store)
  5532			return -EIO;
  5533	
  5534		err = attribute->store(s, buf, len);
  5535	#ifdef CONFIG_MEMCG
  5536		if (slab_state >= FULL && err >= 0 && is_root_cache(s)) {
  5537			struct kmem_cache *c;
  5538	
  5539			/*
  5540			 * Timeout after 100ms
  5541			 */
> 5542			if (mutex_timed_lock(&slab_mutex, 100) < 0)
  5543				return -EBUSY;
  5544	
  5545			if (s->max_attr_size < len)
  5546				s->max_attr_size = len;
  5547	
  5548			/*
  5549			 * This is a best effort propagation, so this function's return
  5550			 * value will be determined by the parent cache only. This is
  5551			 * basically because not all attributes will have a well
  5552			 * defined semantics for rollbacks - most of the actions will
  5553			 * have permanent effects.
  5554			 *
  5555			 * Returning the error value of any of the children that fail
  5556			 * is not 100 % defined, in the sense that users seeing the
  5557			 * error code won't be able to know anything about the state of
  5558			 * the cache.
  5559			 *
  5560			 * Only returning the error code for the parent cache at least
  5561			 * has well defined semantics. The cache being written to
  5562			 * directly either failed or succeeded, in which case we loop
  5563			 * through the descendants with best-effort propagation.
  5564			 */
  5565			for_each_memcg_cache(c, s)
  5566				attribute->store(c, buf, len);
  5567			mutex_unlock(&slab_mutex);
  5568		}
  5569	#endif
  5570		return err;
  5571	}
  5572	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 36070 bytes --]

  parent reply	other threads:[~2020-02-13 16:49 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-10 20:46 [PATCH 0/3] locking/mutex: Add mutex_timed_lock() to solve potential deadlock problems Waiman Long
2020-02-10 20:46 ` [PATCH 1/3] locking/mutex: Add mutex_timed_lock() Waiman Long
2020-02-10 20:46 ` [PATCH 2/3] locking/mutex: Enable some lock event counters Waiman Long
2020-02-10 20:46 ` [PATCH 3/3] mm/slub: Fix potential deadlock problem in slab_attr_store() Waiman Long
2020-02-10 22:03   ` Andrew Morton
2020-02-10 22:14     ` Waiman Long
2020-02-10 23:10       ` Andrew Morton
2020-02-11 23:30         ` Waiman Long
2020-02-12 20:40           ` Waiman Long
2020-02-13 12:22   ` kbuild test robot
2020-02-13 12:22     ` kbuild test robot
2020-02-13 16:48   ` kbuild test robot [this message]
2020-02-13 16:48     ` kbuild test robot
2020-02-11 12:31 ` [PATCH 0/3] locking/mutex: Add mutex_timed_lock() to solve potential deadlock problems Peter Zijlstra
2020-02-11 23:31   ` Waiman Long

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=202002140001.e1zVjKdc%lkp@intel.com \
    --to=lkp@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=kbuild-all@lists.01.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=longman@redhat.com \
    --cc=mingo@redhat.com \
    --cc=penberg@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rientjes@google.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.