Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1584506 > unrolled thread
| Started by | Elena Reshetova <elena.reshetova@intel.com> |
|---|---|
| First post | 2017-02-20 11:50 +0100 |
| Last post | 2017-02-20 12:40 +0100 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 1/5] mm: convert bdi_writeback_congested.refcnt from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-02-20 11:50 +0100
Re: [PATCH 1/5] mm: convert bdi_writeback_congested.refcnt from atomic_t to refcount_t kbuild test robot <lkp@intel.com> - 2017-02-20 12:40 +0100
| From | Elena Reshetova <elena.reshetova@intel.com> |
|---|---|
| Date | 2017-02-20 11:50 +0100 |
| Subject | [PATCH 1/5] mm: convert bdi_writeback_congested.refcnt from atomic_t to refcount_t |
| Message-ID | <tcTHz-3xn-3@gated-at.bofh.it> |
refcount_t type and corresponding API should be used instead of
atomic_t when the variable is used as a reference counter.
This allows to avoid accidental refcounter overflows that might
lead to use-after-free situations.
Switch bdi_writeback_congested.refcnt from atomic_t to refcount_t and
increment initial value by 1. The incrementation affecs the function
wb_congested_get_create which previously incremented both found and
created objects. After this patch the function will increment only found
objects and instead set the refcount of new objects to 1. Note that
new_congested is initially NULL, and will be discarded unless exiting
via the 'if (new_congested)' section.
Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: David Windsor <dwindsor@gmail.com>
---
include/linux/backing-dev-defs.h | 3 ++-
include/linux/backing-dev.h | 4 ++--
mm/backing-dev.c | 11 ++++++-----
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h
index ad95581..609ee6f 100644
--- a/include/linux/backing-dev-defs.h
+++ b/include/linux/backing-dev-defs.h
@@ -4,6 +4,7 @@
#include <linux/list.h>
#include <linux/radix-tree.h>
#include <linux/rbtree.h>
+#include <linux/refcount.h>
#include <linux/spinlock.h>
#include <linux/percpu_counter.h>
#include <linux/percpu-refcount.h>
@@ -51,7 +52,7 @@ enum wb_stat_item {
*/
struct bdi_writeback_congested {
unsigned long state; /* WB_[a]sync_congested flags */
- atomic_t refcnt; /* nr of attached wb's and blkg */
+ refcount_t refcnt; /* nr of attached wb's and blkg */
#ifdef CONFIG_CGROUP_WRITEBACK
struct backing_dev_info *bdi; /* the associated bdi */
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index c52a48c..4726d81 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -430,13 +430,13 @@ static inline bool inode_cgwb_enabled(struct inode *inode)
static inline struct bdi_writeback_congested *
wb_congested_get_create(struct backing_dev_info *bdi, int blkcg_id, gfp_t gfp)
{
- atomic_inc(&bdi->wb_congested->refcnt);
+ refcount_inc(&bdi->wb_congested->refcnt);
return bdi->wb_congested;
}
static inline void wb_congested_put(struct bdi_writeback_congested *congested)
{
- if (atomic_dec_and_test(&congested->refcnt))
+ if (refcount_dec_and_test(&congested->refcnt))
kfree(congested);
}
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 6d861d0..2df3ed7 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -417,14 +417,17 @@ wb_congested_get_create(struct backing_dev_info *bdi, int blkcg_id, gfp_t gfp)
node = &parent->rb_left;
else if (congested->blkcg_id > blkcg_id)
node = &parent->rb_right;
- else
- goto found;
+ else {
+ refcount_inc(&congested->refcnt);
+ goto found;
+ }
}
if (new_congested) {
/* !found and storage for new one already allocated, insert */
congested = new_congested;
new_congested = NULL;
+ refcount_set(&congested->refcnt, 1);
rb_link_node(&congested->rb_node, parent, node);
rb_insert_color(&congested->rb_node, &bdi->cgwb_congested_tree);
goto found;
@@ -437,13 +440,11 @@ wb_congested_get_create(struct backing_dev_info *bdi, int blkcg_id, gfp_t gfp)
if (!new_congested)
return NULL;
- atomic_set(&new_congested->refcnt, 0);
new_congested->bdi = bdi;
new_congested->blkcg_id = blkcg_id;
goto retry;
found:
- atomic_inc(&congested->refcnt);
spin_unlock_irqrestore(&cgwb_lock, flags);
kfree(new_congested);
return congested;
@@ -460,7 +461,7 @@ void wb_congested_put(struct bdi_writeback_congested *congested)
unsigned long flags;
local_irq_save(flags);
- if (!atomic_dec_and_lock(&congested->refcnt, &cgwb_lock)) {
+ if (!refcount_dec_and_lock(&congested->refcnt, &cgwb_lock)) {
local_irq_restore(flags);
return;
}
--
2.7.4
[toc] | [next] | [standalone]
| From | kbuild test robot <lkp@intel.com> |
|---|---|
| Date | 2017-02-20 12:40 +0100 |
| Subject | Re: [PATCH 1/5] mm: convert bdi_writeback_congested.refcnt from atomic_t to refcount_t |
| Message-ID | <tcUtX-453-13@gated-at.bofh.it> |
| In reply to | #1584506 |
[Multipart message — attachments visible in raw view] — view raw
Hi Elena,
[auto build test ERROR on mmotm/master]
[also build test ERROR on next-20170220]
[cannot apply to linus/master linux/master]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Elena-Reshetova/mm-subsystem-refcounter-conversions/20170220-190351
base: git://git.cmpxchg.org/linux-mmotm.git master
config: i386-tinyconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
mm/backing-dev.c: In function 'cgwb_bdi_init':
>> mm/backing-dev.c:798:13: error: passing argument 1 of 'atomic_set' from incompatible pointer type [-Werror=incompatible-pointer-types]
atomic_set(&bdi->wb_congested->refcnt, 1);
^
In file included from include/linux/atomic.h:4:0,
from arch/x86/include/asm/thread_info.h:53,
from include/linux/thread_info.h:25,
from arch/x86/include/asm/preempt.h:6,
from include/linux/preempt.h:59,
from include/linux/spinlock.h:50,
from include/linux/wait.h:8,
from mm/backing-dev.c:2:
arch/x86/include/asm/atomic.h:36:29: note: expected 'atomic_t * {aka struct <anonymous> *}' but argument is of type 'refcount_t * {aka struct refcount_struct *}'
static __always_inline void atomic_set(atomic_t *v, int i)
^~~~~~~~~~
cc1: some warnings being treated as errors
vim +/atomic_set +798 mm/backing-dev.c
a13f35e8 Tejun Heo 2015-07-02 792 int err;
a13f35e8 Tejun Heo 2015-07-02 793
a13f35e8 Tejun Heo 2015-07-02 794 bdi->wb_congested = kzalloc(sizeof(*bdi->wb_congested), GFP_KERNEL);
a13f35e8 Tejun Heo 2015-07-02 795 if (!bdi->wb_congested)
a13f35e8 Tejun Heo 2015-07-02 796 return -ENOMEM;
a13f35e8 Tejun Heo 2015-07-02 797
d3036542 mmotm auto import 2017-02-18 @798 atomic_set(&bdi->wb_congested->refcnt, 1);
d3036542 mmotm auto import 2017-02-18 799
a13f35e8 Tejun Heo 2015-07-02 800 err = wb_init(&bdi->wb, bdi, 1, GFP_KERNEL);
a13f35e8 Tejun Heo 2015-07-02 801 if (err) {
:::::: The code at line 798 was first introduced by commit
:::::: d3036542f0baefd61fb18ce9023c2cd96c89349c linux-next
:::::: TO: mmotm auto import <mm-commits@vger.kernel.org>
:::::: CC: Johannes Weiner <hannes@cmpxchg.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web