* [PATCH RESEND v8 0/3] Introduce RCU string API
@ 2015-01-09 20:35 Omar Sandoval
2015-01-09 20:35 ` [PATCH RESEND v8 1/3] Move BTRFS RCU string to common library Omar Sandoval
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Omar Sandoval @ 2015-01-09 20:35 UTC (permalink / raw)
To: Chris Mason, Josef Bacik, David Sterba, Paul E. McKenney,
Josh Triplett, Steven Rostedt, Mathieu Desnoyers, Lai Jiangshan,
linux-kernel, linux-btrfs
Cc: Omar Sandoval
Hi, everyone,
Now that the merge window and the holidays are over, I've rebased this on
v3.19-rc3 for the next merge window.
This patch series introduces the RCU string API and cleans up the wreckage of
sparse warnings which follow from it (shown here from when the patch was
briefly in the btrfs integration tree):
On Thu, Nov 27, 2014 at 06:45:20AM +0800, kbuild test robot wrote:
> tree: git://git.kernel.org/pub/scm/linux/kernel/git/mason/linux-btrfs.git integration
> head: c7a37618b60026121255c69e042d74ae5631470c
> commit: 37aad79d90a0cbf82a5eda62dfe3af4241f5aca3 [38/39] Move BTRFS RCU string to common library
> reproduce:
> # apt-get install sparse
> git checkout 37aad79d90a0cbf82a5eda62dfe3af4241f5aca3
> make ARCH=x86_64 allmodconfig
> make C=1 CF=-D__CHECK_ENDIAN__
>
>
> sparse warnings: (new ones prefixed by >>)
>
> >> fs/btrfs/check-integrity.c:848:25: sparse: incorrect type in argument 1 (different address spaces)
> fs/btrfs/check-integrity.c:848:25: expected struct rcu_string [noderef] <asn:4>*rcu_str
> fs/btrfs/check-integrity.c:848:25: got struct rcu_string *name
[snip]
Version 8 combines the original patch with another patch series I posted to fix
these warnings, which fixes the botched __rcu annotations that caused some of
the warnings and refactors the existing uses of rcustring to get rid of the
rest. There's also a patch to fix an RCU misuse.
Thanks!
v8: Get the __rcu annotations right, clean up RCU string-related sparse noise
v7: Add arguments to kernel doc for printk wrappers, use ##__VA_ARGS
v6: Add header dependencies to rcustring.h
v5: Rebase against v3.18-rc3
v4: Don't return anything from the printk wrappers on the assumption that
printk will return void someday
v3: Add __rcu annotation to relevant functions, add Paul's ack and Josh's
review
Omar Sandoval (3):
Move BTRFS RCU string to common library
btrfs: refactor btrfs_device->name updates
btrfs: fix suspicious RCU in BTRFS_IOC_DEV_INFO
fs/btrfs/check-integrity.c | 6 +--
fs/btrfs/dev-replace.c | 19 ++++----
fs/btrfs/disk-io.c | 6 +--
fs/btrfs/extent_io.c | 4 +-
fs/btrfs/ioctl.c | 14 +++---
fs/btrfs/raid56.c | 2 +-
fs/btrfs/rcu-string.h | 56 ------------------------
fs/btrfs/scrub.c | 15 ++++---
fs/btrfs/super.c | 2 +-
fs/btrfs/volumes.c | 107 ++++++++++++++++++++++++++++++---------------
fs/btrfs/volumes.h | 2 +-
include/linux/rcustring.h | 97 ++++++++++++++++++++++++++++++++++++++++
12 files changed, 204 insertions(+), 126 deletions(-)
delete mode 100644 fs/btrfs/rcu-string.h
create mode 100644 include/linux/rcustring.h
--
2.2.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH RESEND v8 1/3] Move BTRFS RCU string to common library
2015-01-09 20:35 [PATCH RESEND v8 0/3] Introduce RCU string API Omar Sandoval
@ 2015-01-09 20:35 ` Omar Sandoval
2015-01-09 20:35 ` [PATCH RESEND v8 2/3] btrfs: refactor btrfs_device->name updates Omar Sandoval
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Omar Sandoval @ 2015-01-09 20:35 UTC (permalink / raw)
To: Chris Mason, Josef Bacik, David Sterba, Paul E. McKenney,
Josh Triplett, Steven Rostedt, Mathieu Desnoyers, Lai Jiangshan,
linux-kernel, linux-btrfs
Cc: Omar Sandoval
The RCU-friendly string API used internally by BTRFS is generic enough
for common use. This doesn't add any new functionality but instead just
moves the code and documents the existing API.
Reviewed-by: Josh Triplett <josh@joshtriplett.org>
Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Omar Sandoval <osandov@osandov.com>
---
fs/btrfs/check-integrity.c | 6 +--
fs/btrfs/dev-replace.c | 19 ++++-----
fs/btrfs/disk-io.c | 6 +--
fs/btrfs/extent_io.c | 4 +-
fs/btrfs/ioctl.c | 4 +-
fs/btrfs/raid56.c | 2 +-
fs/btrfs/rcu-string.h | 56 --------------------------
fs/btrfs/scrub.c | 15 +++----
fs/btrfs/super.c | 2 +-
fs/btrfs/volumes.c | 14 +++----
include/linux/rcustring.h | 97 ++++++++++++++++++++++++++++++++++++++++++++++
11 files changed, 134 insertions(+), 91 deletions(-)
delete mode 100644 fs/btrfs/rcu-string.h
create mode 100644 include/linux/rcustring.h
diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
index d897ef8..9f4ac42 100644
--- a/fs/btrfs/check-integrity.c
+++ b/fs/btrfs/check-integrity.c
@@ -95,6 +95,7 @@
#include <linux/genhd.h>
#include <linux/blkdev.h>
#include <linux/vmalloc.h>
+#include <linux/rcustring.h>
#include "ctree.h"
#include "disk-io.h"
#include "hash.h"
@@ -104,7 +105,6 @@
#include "print-tree.h"
#include "locking.h"
#include "check-integrity.h"
-#include "rcu-string.h"
#define BTRFSIC_BLOCK_HASHTABLE_SIZE 0x10000
#define BTRFSIC_BLOCK_LINK_HASHTABLE_SIZE 0x10000
@@ -848,8 +848,8 @@ static int btrfsic_process_superblock_dev_mirror(
printk_in_rcu(KERN_INFO "New initial S-block (bdev %p, %s)"
" @%llu (%s/%llu/%d)\n",
superblock_bdev,
- rcu_str_deref(device->name), dev_bytenr,
- dev_state->name, dev_bytenr,
+ rcu_string_dereference(device->name),
+ dev_bytenr, dev_state->name, dev_bytenr,
superblock_mirror_num);
list_add(&superblock_tmp->all_blocks_node,
&state->all_blocks_list);
diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index ca6a3a3..51133ea 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -25,6 +25,7 @@
#include <linux/capability.h>
#include <linux/kthread.h>
#include <linux/math64.h>
+#include <linux/rcustring.h>
#include <asm/div64.h>
#include "ctree.h"
#include "extent_map.h"
@@ -34,7 +35,6 @@
#include "volumes.h"
#include "async-thread.h"
#include "check-integrity.h"
-#include "rcu-string.h"
#include "dev-replace.h"
#include "sysfs.h"
@@ -379,9 +379,9 @@ int btrfs_dev_replace_start(struct btrfs_root *root,
printk_in_rcu(KERN_INFO
"BTRFS: dev_replace from %s (devid %llu) to %s started\n",
src_device->missing ? "<missing disk>" :
- rcu_str_deref(src_device->name),
+ rcu_string_dereference(src_device->name),
src_device->devid,
- rcu_str_deref(tgt_device->name));
+ rcu_string_dereference(tgt_device->name));
/*
* from now on, the writes to the srcdev are all duplicated to
@@ -532,9 +532,10 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info,
printk_in_rcu(KERN_ERR
"BTRFS: btrfs_scrub_dev(%s, %llu, %s) failed %d\n",
src_device->missing ? "<missing disk>" :
- rcu_str_deref(src_device->name),
+ rcu_string_dereference(src_device->name),
src_device->devid,
- rcu_str_deref(tgt_device->name), scrub_ret);
+ rcu_string_dereference(tgt_device->name),
+ scrub_ret);
btrfs_dev_replace_unlock(dev_replace);
mutex_unlock(&root->fs_info->chunk_mutex);
mutex_unlock(&root->fs_info->fs_devices->device_list_mutex);
@@ -549,9 +550,9 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info,
printk_in_rcu(KERN_INFO
"BTRFS: dev_replace from %s (devid %llu) to %s finished\n",
src_device->missing ? "<missing disk>" :
- rcu_str_deref(src_device->name),
+ rcu_string_dereference(src_device->name),
src_device->devid,
- rcu_str_deref(tgt_device->name));
+ rcu_string_dereference(tgt_device->name));
tgt_device->is_tgtdev_for_dev_replace = 0;
tgt_device->devid = src_device->devid;
src_device->devid = BTRFS_DEV_REPLACE_DEVID;
@@ -819,10 +820,10 @@ static int btrfs_dev_replace_kthread(void *data)
printk_in_rcu(KERN_INFO
"BTRFS: continuing dev_replace from %s (devid %llu) to %s @%u%%\n",
dev_replace->srcdev->missing ? "<missing disk>" :
- rcu_str_deref(dev_replace->srcdev->name),
+ rcu_string_dereference(dev_replace->srcdev->name),
dev_replace->srcdev->devid,
dev_replace->tgtdev ?
- rcu_str_deref(dev_replace->tgtdev->name) :
+ rcu_string_dereference(dev_replace->tgtdev->name) :
"<missing target disk>",
(unsigned int)progress);
}
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 8c63419..a13b7fc 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -31,6 +31,7 @@
#include <linux/ratelimit.h>
#include <linux/uuid.h>
#include <linux/semaphore.h>
+#include <linux/rcustring.h>
#include <asm/unaligned.h>
#include "ctree.h"
#include "disk-io.h"
@@ -44,7 +45,6 @@
#include "free-space-cache.h"
#include "inode-map.h"
#include "check-integrity.h"
-#include "rcu-string.h"
#include "dev-replace.h"
#include "raid56.h"
#include "sysfs.h"
@@ -3062,7 +3062,7 @@ static void btrfs_end_buffer_write_sync(struct buffer_head *bh, int uptodate)
printk_ratelimited_in_rcu(KERN_WARNING "BTRFS: lost page write due to "
"I/O error on %s\n",
- rcu_str_deref(device->name));
+ rcu_string_dereference(device->name));
/* note, we dont' set_buffer_write_io_error because we have
* our own ways of dealing with the IO errors
*/
@@ -3251,7 +3251,7 @@ static int write_dev_flush(struct btrfs_device *device, int wait)
if (bio_flagged(bio, BIO_EOPNOTSUPP)) {
printk_in_rcu("BTRFS: disabling barriers on dev %s\n",
- rcu_str_deref(device->name));
+ rcu_string_dereference(device->name));
device->nobarriers = 1;
} else if (!bio_flagged(bio, BIO_UPTODATE)) {
ret = -EIO;
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 4ebabd2..57cd6a5 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -11,6 +11,7 @@
#include <linux/pagevec.h>
#include <linux/prefetch.h>
#include <linux/cleancache.h>
+#include <linux/rcustring.h>
#include "extent_io.h"
#include "extent_map.h"
#include "ctree.h"
@@ -18,7 +19,6 @@
#include "volumes.h"
#include "check-integrity.h"
#include "locking.h"
-#include "rcu-string.h"
#include "backref.h"
static struct kmem_cache *extent_state_cache;
@@ -2075,7 +2075,7 @@ int repair_io_failure(struct inode *inode, u64 start, u64 length, u64 logical,
printk_ratelimited_in_rcu(KERN_INFO
"BTRFS: read error corrected: ino %llu off %llu (dev %s sector %llu)\n",
btrfs_ino(inode), start,
- rcu_str_deref(dev->name), sector);
+ rcu_string_dereference(dev->name), sector);
bio_put(bio);
return 0;
}
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index d49fe8a..7ca1cad 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -44,6 +44,7 @@
#include <linux/uuid.h>
#include <linux/btrfs.h>
#include <linux/uaccess.h>
+#include <linux/rcustring.h>
#include "ctree.h"
#include "disk-io.h"
#include "transaction.h"
@@ -53,7 +54,6 @@
#include "locking.h"
#include "inode-map.h"
#include "backref.h"
-#include "rcu-string.h"
#include "send.h"
#include "dev-replace.h"
#include "props.h"
@@ -1568,7 +1568,7 @@ static noinline int btrfs_ioctl_resize(struct file *file,
new_size *= root->sectorsize;
printk_in_rcu(KERN_INFO "BTRFS: new size for %s is %llu\n",
- rcu_str_deref(device->name), new_size);
+ rcu_string_dereference(device->name), new_size);
if (new_size > old_size) {
trans = btrfs_start_transaction(root, 0);
diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
index 8ab2a17..44573bf 100644
--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -32,6 +32,7 @@
#include <linux/list_sort.h>
#include <linux/raid/xor.h>
#include <linux/vmalloc.h>
+#include <linux/rcustring.h>
#include <asm/div64.h>
#include "ctree.h"
#include "extent_map.h"
@@ -42,7 +43,6 @@
#include "raid56.h"
#include "async-thread.h"
#include "check-integrity.h"
-#include "rcu-string.h"
/* set when additional merges to this rbio are not allowed */
#define RBIO_RMW_LOCKED_BIT 1
diff --git a/fs/btrfs/rcu-string.h b/fs/btrfs/rcu-string.h
deleted file mode 100644
index 9e111e4..0000000
--- a/fs/btrfs/rcu-string.h
+++ /dev/null
@@ -1,56 +0,0 @@
-/*
- * Copyright (C) 2012 Red Hat. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public
- * License v2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * General Public License for more details.
- *
- * You should have received a copy of the GNU General Public
- * License along with this program; if not, write to the
- * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
- * Boston, MA 021110-1307, USA.
- */
-
-struct rcu_string {
- struct rcu_head rcu;
- char str[0];
-};
-
-static inline struct rcu_string *rcu_string_strdup(const char *src, gfp_t mask)
-{
- size_t len = strlen(src) + 1;
- struct rcu_string *ret = kzalloc(sizeof(struct rcu_string) +
- (len * sizeof(char)), mask);
- if (!ret)
- return ret;
- strncpy(ret->str, src, len);
- return ret;
-}
-
-static inline void rcu_string_free(struct rcu_string *str)
-{
- if (str)
- kfree_rcu(str, rcu);
-}
-
-#define printk_in_rcu(fmt, ...) do { \
- rcu_read_lock(); \
- printk(fmt, __VA_ARGS__); \
- rcu_read_unlock(); \
-} while (0)
-
-#define printk_ratelimited_in_rcu(fmt, ...) do { \
- rcu_read_lock(); \
- printk_ratelimited(fmt, __VA_ARGS__); \
- rcu_read_unlock(); \
-} while (0)
-
-#define rcu_str_deref(rcu_str) ({ \
- struct rcu_string *__str = rcu_dereference(rcu_str); \
- __str->str; \
-})
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index f2bb13a..1807101 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -18,6 +18,7 @@
#include <linux/blkdev.h>
#include <linux/ratelimit.h>
+#include <linux/rcustring.h>
#include "ctree.h"
#include "volumes.h"
#include "disk-io.h"
@@ -27,7 +28,6 @@
#include "extent_io.h"
#include "dev-replace.h"
#include "check-integrity.h"
-#include "rcu-string.h"
#include "raid56.h"
/*
@@ -562,7 +562,8 @@ static int scrub_print_warning_inode(u64 inum, u64 offset, u64 root,
printk_in_rcu(KERN_WARNING "BTRFS: %s at logical %llu on dev "
"%s, sector %llu, root %llu, inode %llu, offset %llu, "
"length %llu, links %u (path: %s)\n", swarn->errstr,
- swarn->logical, rcu_str_deref(swarn->dev->name),
+ swarn->logical,
+ rcu_string_dereference(swarn->dev->name),
(unsigned long long)swarn->sector, root, inum, offset,
min(isize - offset, (u64)PAGE_SIZE), nlink,
(char *)(unsigned long)ipath->fspath->val[i]);
@@ -574,7 +575,7 @@ err:
printk_in_rcu(KERN_WARNING "BTRFS: %s at logical %llu on dev "
"%s, sector %llu, root %llu, inode %llu, offset %llu: path "
"resolving failed with ret=%d\n", swarn->errstr,
- swarn->logical, rcu_str_deref(swarn->dev->name),
+ swarn->logical, rcu_string_dereference(swarn->dev->name),
(unsigned long long)swarn->sector, root, inum, offset, ret);
free_ipath(ipath);
@@ -632,7 +633,7 @@ static void scrub_print_warning(const char *errstr, struct scrub_block *sblock)
"BTRFS: %s at logical %llu on dev %s, "
"sector %llu: metadata %s (level %d) in tree "
"%llu\n", errstr, swarn.logical,
- rcu_str_deref(dev->name),
+ rcu_string_dereference(dev->name),
(unsigned long long)swarn.sector,
ref_level ? "node" : "leaf",
ret < 0 ? -1 : ref_level,
@@ -831,7 +832,7 @@ out:
num_uncorrectable_read_errors);
printk_ratelimited_in_rcu(KERN_ERR "BTRFS: "
"unable to fixup (nodatasum) error at logical %llu on dev %s\n",
- fixup->logical, rcu_str_deref(fixup->dev->name));
+ fixup->logical, rcu_string_dereference(fixup->dev->name));
}
btrfs_free_path(path);
@@ -1249,7 +1250,7 @@ corrected_error:
spin_unlock(&sctx->stat_lock);
printk_ratelimited_in_rcu(KERN_ERR
"BTRFS: fixed up error at logical %llu on dev %s\n",
- logical, rcu_str_deref(dev->name));
+ logical, rcu_string_dereference(dev->name));
}
} else {
did_not_correct_error:
@@ -1258,7 +1259,7 @@ did_not_correct_error:
spin_unlock(&sctx->stat_lock);
printk_ratelimited_in_rcu(KERN_ERR
"BTRFS: unable to fixup (regular) error at logical %llu on dev %s\n",
- logical, rcu_str_deref(dev->name));
+ logical, rcu_string_dereference(dev->name));
}
out:
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 60f7cbe..0481020 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -42,6 +42,7 @@
#include <linux/cleancache.h>
#include <linux/ratelimit.h>
#include <linux/btrfs.h>
+#include <linux/rcustring.h>
#include "delayed-inode.h"
#include "ctree.h"
#include "disk-io.h"
@@ -54,7 +55,6 @@
#include "volumes.h"
#include "export.h"
#include "compression.h"
-#include "rcu-string.h"
#include "dev-replace.h"
#include "free-space-cache.h"
#include "backref.h"
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 50c5a87..2231275 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -27,6 +27,7 @@
#include <linux/kthread.h>
#include <linux/raid/pq.h>
#include <linux/semaphore.h>
+#include <linux/rcustring.h>
#include <asm/div64.h>
#include "ctree.h"
#include "extent_map.h"
@@ -37,7 +38,6 @@
#include "raid56.h"
#include "async-thread.h"
#include "check-integrity.h"
-#include "rcu-string.h"
#include "math.h"
#include "dev-replace.h"
#include "sysfs.h"
@@ -6474,7 +6474,7 @@ static int update_dev_stat_item(struct btrfs_trans_handle *trans,
if (ret < 0) {
printk_in_rcu(KERN_WARNING "BTRFS: "
"error %d while searching for dev_stats item for device %s!\n",
- ret, rcu_str_deref(device->name));
+ ret, rcu_string_dereference(device->name));
goto out;
}
@@ -6485,7 +6485,7 @@ static int update_dev_stat_item(struct btrfs_trans_handle *trans,
if (ret != 0) {
printk_in_rcu(KERN_WARNING "BTRFS: "
"delete too small dev_stats item for device %s failed %d!\n",
- rcu_str_deref(device->name), ret);
+ rcu_string_dereference(device->name), ret);
goto out;
}
ret = 1;
@@ -6498,8 +6498,8 @@ static int update_dev_stat_item(struct btrfs_trans_handle *trans,
&key, sizeof(*ptr));
if (ret < 0) {
printk_in_rcu(KERN_WARNING "BTRFS: "
- "insert dev_stats item for device %s failed %d!\n",
- rcu_str_deref(device->name), ret);
+ "insert dev_stats item for device %s failed %d!\n",
+ rcu_string_dereference(device->name), ret);
goto out;
}
}
@@ -6555,7 +6555,7 @@ static void btrfs_dev_stat_print_on_error(struct btrfs_device *dev)
return;
printk_ratelimited_in_rcu(KERN_ERR "BTRFS: "
"bdev %s errs: wr %u, rd %u, flush %u, corrupt %u, gen %u\n",
- rcu_str_deref(dev->name),
+ rcu_string_dereference(dev->name),
btrfs_dev_stat_read(dev, BTRFS_DEV_STAT_WRITE_ERRS),
btrfs_dev_stat_read(dev, BTRFS_DEV_STAT_READ_ERRS),
btrfs_dev_stat_read(dev, BTRFS_DEV_STAT_FLUSH_ERRS),
@@ -6575,7 +6575,7 @@ static void btrfs_dev_stat_print_on_load(struct btrfs_device *dev)
printk_in_rcu(KERN_INFO "BTRFS: "
"bdev %s errs: wr %u, rd %u, flush %u, corrupt %u, gen %u\n",
- rcu_str_deref(dev->name),
+ rcu_string_dereference(dev->name),
btrfs_dev_stat_read(dev, BTRFS_DEV_STAT_WRITE_ERRS),
btrfs_dev_stat_read(dev, BTRFS_DEV_STAT_READ_ERRS),
btrfs_dev_stat_read(dev, BTRFS_DEV_STAT_FLUSH_ERRS),
diff --git a/include/linux/rcustring.h b/include/linux/rcustring.h
new file mode 100644
index 0000000..d73cfcd
--- /dev/null
+++ b/include/linux/rcustring.h
@@ -0,0 +1,97 @@
+/*
+ * RCU-friendly strings
+ *
+ * Copyright (C) 2012 Red Hat. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you can access it online at
+ * http://www.gnu.org/licenses/gpl-2.0.html.
+ */
+
+#ifndef _LINUX_RCUSTRING_H
+#define _LINUX_RCUSTRING_H
+
+#include <linux/types.h>
+#include <linux/compiler.h>
+#include <linux/printk.h>
+#include <linux/rcupdate.h>
+#include <linux/slab.h>
+
+struct rcu_string {
+ struct rcu_head rcu;
+ char str[0];
+};
+
+/**
+ * rcu_string_strdup() - create an RCU string from a string
+ * @src: The string to copy
+ * @flags: Flags for kmalloc
+ */
+static inline struct rcu_string *rcu_string_strdup(const char *src, gfp_t flags)
+{
+ struct rcu_string *ret;
+ size_t len = strlen(src) + 1;
+
+ ret = kmalloc(sizeof(*ret) + (len * sizeof(char)), flags);
+ if (ret)
+ memcpy(ret->str, src, len);
+ return ret;
+}
+
+/**
+ * rcu_string_free() - free an RCU string
+ * @str: The string
+ */
+static inline void rcu_string_free(struct rcu_string *str)
+{
+ if (str)
+ kfree_rcu(str, rcu);
+}
+
+/**
+ * rcu_string_dereference() - dereference an RCU string
+ * @str: The string
+ *
+ * Like rcu_dereference, this must be done in an RCU critical section.
+ */
+static inline char *rcu_string_dereference(struct rcu_string __rcu *rcu_str)
+{
+ return rcu_dereference(rcu_str)->str;
+}
+
+/**
+ * printk_in_rcu() - printk in an RCU read-side critical section
+ * @fmt: Format string
+ * @...: Values
+ */
+#define printk_in_rcu(fmt, ...) \
+ do { \
+ rcu_read_lock(); \
+ printk(fmt, ##__VA_ARGS__); \
+ rcu_read_unlock(); \
+ } while (0)
+
+/**
+ * printk_ratelimited_in_rcu() - printk_ratelimited in an RCU read-side critical
+ * section
+ * @fmt: Format string
+ * @...: Values
+ */
+#define printk_ratelimited_in_rcu(fmt, ...) \
+ do { \
+ rcu_read_lock(); \
+ printk_ratelimited(fmt, ##__VA_ARGS__); \
+ rcu_read_unlock(); \
+ } while (0)
+
+#endif
--
2.2.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH RESEND v8 2/3] btrfs: refactor btrfs_device->name updates
2015-01-09 20:35 [PATCH RESEND v8 0/3] Introduce RCU string API Omar Sandoval
2015-01-09 20:35 ` [PATCH RESEND v8 1/3] Move BTRFS RCU string to common library Omar Sandoval
@ 2015-01-09 20:35 ` Omar Sandoval
2015-01-09 20:36 ` [PATCH RESEND v8 3/3] btrfs: fix suspicious RCU in BTRFS_IOC_DEV_INFO Omar Sandoval
2015-01-10 19:56 ` [PATCH RESEND v8 0/3] Introduce RCU string API Paul E. McKenney
3 siblings, 0 replies; 6+ messages in thread
From: Omar Sandoval @ 2015-01-09 20:35 UTC (permalink / raw)
To: Chris Mason, Josef Bacik, David Sterba, Paul E. McKenney,
Josh Triplett, Steven Rostedt, Mathieu Desnoyers, Lai Jiangshan,
linux-kernel, linux-btrfs
Cc: Omar Sandoval
The rcu_string API introduced some new sparse errors but also revealed
existing ones. First of all, the name in struct btrfs_device should be
annotated as __rcu to prevent unsafe reads. Additionally, updates should
go through rcu_dereference_protected to make it clear what's going on.
This introduces some helper functions that factor out this
functionality.
Signed-off-by: Omar Sandoval <osandov@osandov.com>
---
fs/btrfs/volumes.c | 93 +++++++++++++++++++++++++++++++++++++-----------------
fs/btrfs/volumes.h | 2 +-
2 files changed, 65 insertions(+), 30 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 2231275..ddd7483 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -53,6 +53,45 @@ static void btrfs_dev_stat_print_on_load(struct btrfs_device *device);
DEFINE_MUTEX(uuid_mutex);
static LIST_HEAD(fs_uuids);
+/*
+ * Dereference the device name under the uuid_mutex.
+ */
+static inline struct rcu_string *
+btrfs_dev_rcu_protected_name(struct btrfs_device *dev)
+__must_hold(&uuid_mutex)
+{
+ return rcu_dereference_protected(dev->name,
+ lockdep_is_held(&uuid_mutex));
+}
+
+/*
+ * Use when the caller is the only possible updater.
+ */
+static inline struct rcu_string *
+btrfs_dev_rcu_only_name(struct btrfs_device *dev)
+{
+ return rcu_dereference_protected(dev->name, 1);
+}
+
+/*
+ * Rename a device under the uuid_mutex.
+ */
+static inline int btrfs_dev_rename(struct btrfs_device *dev, const char *name)
+__must_hold(&uuid_mutex)
+{
+ struct rcu_string *old_name, *new_name;
+
+ new_name = rcu_string_strdup(name, GFP_NOFS);
+ if (!new_name)
+ return -ENOMEM;
+
+ old_name = btrfs_dev_rcu_protected_name(dev);
+ rcu_assign_pointer(dev->name, new_name);
+ rcu_string_free(old_name);
+
+ return 0;
+}
+
static struct btrfs_fs_devices *__alloc_fs_devices(void)
{
struct btrfs_fs_devices *fs_devs;
@@ -104,7 +143,7 @@ static void free_fs_devices(struct btrfs_fs_devices *fs_devices)
device = list_entry(fs_devices->devices.next,
struct btrfs_device, dev_list);
list_del(&device->dev_list);
- rcu_string_free(device->name);
+ rcu_string_free(btrfs_dev_rcu_only_name(device));
kfree(device);
}
kfree(fs_devices);
@@ -485,12 +524,10 @@ static noinline int device_list_add(const char *path,
return PTR_ERR(device);
}
- name = rcu_string_strdup(path, GFP_NOFS);
- if (!name) {
+ if (btrfs_dev_rename(device, path)) {
kfree(device);
return -ENOMEM;
}
- rcu_assign_pointer(device->name, name);
mutex_lock(&fs_devices->device_list_mutex);
list_add_rcu(&device->dev_list, &fs_devices->devices);
@@ -499,7 +536,11 @@ static noinline int device_list_add(const char *path,
ret = 1;
device->fs_devices = fs_devices;
- } else if (!device->name || strcmp(device->name->str, path)) {
+ } else {
+ name = btrfs_dev_rcu_protected_name(device);
+ if (name && strcmp(name->str, path) == 0)
+ goto out;
+
/*
* When FS is already mounted.
* 1. If you are here and if the device->name is NULL that
@@ -537,17 +578,15 @@ static noinline int device_list_add(const char *path,
return -EEXIST;
}
- name = rcu_string_strdup(path, GFP_NOFS);
- if (!name)
+ if (btrfs_dev_rename(device, path))
return -ENOMEM;
- rcu_string_free(device->name);
- rcu_assign_pointer(device->name, name);
if (device->missing) {
fs_devices->missing_devices--;
device->missing = 0;
}
}
+out:
/*
* Unmount does not free the btrfs_device struct but would zero
* generation along with most of the other members. So just update
@@ -584,17 +623,12 @@ static struct btrfs_fs_devices *clone_fs_devices(struct btrfs_fs_devices *orig)
if (IS_ERR(device))
goto error;
- /*
- * This is ok to do without rcu read locked because we hold the
- * uuid mutex so nothing we touch in here is going to disappear.
- */
- if (orig_dev->name) {
- name = rcu_string_strdup(orig_dev->name->str, GFP_NOFS);
- if (!name) {
+ name = btrfs_dev_rcu_protected_name(orig_dev);
+ if (name) {
+ if (btrfs_dev_rename(device, name->str)) {
kfree(device);
goto error;
}
- rcu_assign_pointer(device->name, name);
}
list_add(&device->dev_list, &fs_devices->devices);
@@ -656,7 +690,7 @@ again:
}
list_del_init(&device->dev_list);
fs_devices->num_devices--;
- rcu_string_free(device->name);
+ rcu_string_free(btrfs_dev_rcu_only_name(device));
kfree(device);
}
@@ -679,7 +713,7 @@ static void __free_device(struct work_struct *work)
if (device->bdev)
blkdev_put(device->bdev, device->mode);
- rcu_string_free(device->name);
+ rcu_string_free(btrfs_dev_rcu_only_name(device));
kfree(device);
}
@@ -721,11 +755,10 @@ static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
device->uuid);
BUG_ON(IS_ERR(new_device)); /* -ENOMEM */
- /* Safe because we are under uuid_mutex */
- if (device->name) {
- name = rcu_string_strdup(device->name->str, GFP_NOFS);
- BUG_ON(!name); /* -ENOMEM */
- rcu_assign_pointer(new_device->name, name);
+ name = btrfs_dev_rcu_protected_name(device);
+ if (name) {
+ if (btrfs_dev_rename(new_device, name->str))
+ BUG_ON(1); /* -ENOMEM */
}
list_replace_rcu(&device->dev_list, &new_device->dev_list);
@@ -784,18 +817,20 @@ static int __btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
u64 devid;
int seeding = 1;
int ret = 0;
+ struct rcu_string *name;
flags |= FMODE_EXCL;
list_for_each_entry(device, head, dev_list) {
if (device->bdev)
continue;
- if (!device->name)
+ name = btrfs_dev_rcu_protected_name(device);
+ if (!name)
continue;
/* Just open everything we can; ignore failures here */
- if (btrfs_get_bdev_and_sb(device->name->str, flags, holder, 1,
- &bdev, &bh))
+ if (btrfs_get_bdev_and_sb(name->str, flags, holder, 1, &bdev,
+ &bh))
continue;
disk_super = (struct btrfs_super_block *)bh->b_data;
@@ -2142,7 +2177,7 @@ int btrfs_init_new_device(struct btrfs_root *root, char *device_path)
trans = btrfs_start_transaction(root, 0);
if (IS_ERR(trans)) {
- rcu_string_free(device->name);
+ rcu_string_free(btrfs_dev_rcu_only_name(device));
kfree(device);
ret = PTR_ERR(trans);
goto error;
@@ -2279,7 +2314,7 @@ int btrfs_init_new_device(struct btrfs_root *root, char *device_path)
error_trans:
btrfs_end_transaction(trans, root);
- rcu_string_free(device->name);
+ rcu_string_free(btrfs_dev_rcu_only_name(device));
btrfs_kobj_rm_device(root->fs_info, device);
kfree(device);
error:
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index d6fe73c..234fd20 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -54,7 +54,7 @@ struct btrfs_device {
struct btrfs_root *dev_root;
- struct rcu_string *name;
+ struct rcu_string __rcu *name;
u64 generation;
--
2.2.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH RESEND v8 3/3] btrfs: fix suspicious RCU in BTRFS_IOC_DEV_INFO
2015-01-09 20:35 [PATCH RESEND v8 0/3] Introduce RCU string API Omar Sandoval
2015-01-09 20:35 ` [PATCH RESEND v8 1/3] Move BTRFS RCU string to common library Omar Sandoval
2015-01-09 20:35 ` [PATCH RESEND v8 2/3] btrfs: refactor btrfs_device->name updates Omar Sandoval
@ 2015-01-09 20:36 ` Omar Sandoval
2015-01-10 19:56 ` [PATCH RESEND v8 0/3] Introduce RCU string API Paul E. McKenney
3 siblings, 0 replies; 6+ messages in thread
From: Omar Sandoval @ 2015-01-09 20:36 UTC (permalink / raw)
To: Chris Mason, Josef Bacik, David Sterba, Paul E. McKenney,
Josh Triplett, Steven Rostedt, Mathieu Desnoyers, Lai Jiangshan,
linux-kernel, linux-btrfs
Cc: Omar Sandoval
A naked read of the value of an RCU pointer isn't safe. Put the whole
access in an RCU critical section, not just the pointer dereference.
Signed-off-by: Omar Sandoval <osandov@osandov.com>
---
fs/btrfs/ioctl.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 7ca1cad..ee2528e 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2706,6 +2706,7 @@ static long btrfs_ioctl_dev_info(struct btrfs_root *root, void __user *arg)
struct btrfs_fs_devices *fs_devices = root->fs_info->fs_devices;
int ret = 0;
char *s_uuid = NULL;
+ struct rcu_string *name;
di_args = memdup_user(arg, sizeof(*di_args));
if (IS_ERR(di_args))
@@ -2726,17 +2727,16 @@ static long btrfs_ioctl_dev_info(struct btrfs_root *root, void __user *arg)
di_args->bytes_used = btrfs_device_get_bytes_used(dev);
di_args->total_bytes = btrfs_device_get_total_bytes(dev);
memcpy(di_args->uuid, dev->uuid, sizeof(di_args->uuid));
- if (dev->name) {
- struct rcu_string *name;
- rcu_read_lock();
- name = rcu_dereference(dev->name);
+ rcu_read_lock();
+ name = rcu_dereference(dev->name);
+ if (name) {
strncpy(di_args->path, name->str, sizeof(di_args->path));
- rcu_read_unlock();
di_args->path[sizeof(di_args->path) - 1] = 0;
} else {
di_args->path[0] = '\0';
}
+ rcu_read_unlock();
out:
mutex_unlock(&fs_devices->device_list_mutex);
--
2.2.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND v8 0/3] Introduce RCU string API
2015-01-09 20:35 [PATCH RESEND v8 0/3] Introduce RCU string API Omar Sandoval
` (2 preceding siblings ...)
2015-01-09 20:36 ` [PATCH RESEND v8 3/3] btrfs: fix suspicious RCU in BTRFS_IOC_DEV_INFO Omar Sandoval
@ 2015-01-10 19:56 ` Paul E. McKenney
2015-01-10 21:05 ` Omar Sandoval
3 siblings, 1 reply; 6+ messages in thread
From: Paul E. McKenney @ 2015-01-10 19:56 UTC (permalink / raw)
To: Omar Sandoval
Cc: Chris Mason, Josef Bacik, David Sterba, Josh Triplett,
Steven Rostedt, Mathieu Desnoyers, Lai Jiangshan, linux-kernel,
linux-btrfs
On Fri, Jan 09, 2015 at 12:35:57PM -0800, Omar Sandoval wrote:
> Hi, everyone,
>
> Now that the merge window and the holidays are over, I've rebased this on
> v3.19-rc3 for the next merge window.
Wasn't btrfs going to take this one?
If I am supposed to take it, please split out the RCU infrastructure
portion so that I can push it separately.
Thanx, Paul
> This patch series introduces the RCU string API and cleans up the wreckage of
> sparse warnings which follow from it (shown here from when the patch was
> briefly in the btrfs integration tree):
>
> On Thu, Nov 27, 2014 at 06:45:20AM +0800, kbuild test robot wrote:
> > tree: git://git.kernel.org/pub/scm/linux/kernel/git/mason/linux-btrfs.git integration
> > head: c7a37618b60026121255c69e042d74ae5631470c
> > commit: 37aad79d90a0cbf82a5eda62dfe3af4241f5aca3 [38/39] Move BTRFS RCU string to common library
> > reproduce:
> > # apt-get install sparse
> > git checkout 37aad79d90a0cbf82a5eda62dfe3af4241f5aca3
> > make ARCH=x86_64 allmodconfig
> > make C=1 CF=-D__CHECK_ENDIAN__
> >
> >
> > sparse warnings: (new ones prefixed by >>)
> >
> > >> fs/btrfs/check-integrity.c:848:25: sparse: incorrect type in argument 1 (different address spaces)
> > fs/btrfs/check-integrity.c:848:25: expected struct rcu_string [noderef] <asn:4>*rcu_str
> > fs/btrfs/check-integrity.c:848:25: got struct rcu_string *name
> [snip]
>
> Version 8 combines the original patch with another patch series I posted to fix
> these warnings, which fixes the botched __rcu annotations that caused some of
> the warnings and refactors the existing uses of rcustring to get rid of the
> rest. There's also a patch to fix an RCU misuse.
>
> Thanks!
>
> v8: Get the __rcu annotations right, clean up RCU string-related sparse noise
> v7: Add arguments to kernel doc for printk wrappers, use ##__VA_ARGS
> v6: Add header dependencies to rcustring.h
> v5: Rebase against v3.18-rc3
> v4: Don't return anything from the printk wrappers on the assumption that
> printk will return void someday
> v3: Add __rcu annotation to relevant functions, add Paul's ack and Josh's
> review
>
> Omar Sandoval (3):
> Move BTRFS RCU string to common library
> btrfs: refactor btrfs_device->name updates
> btrfs: fix suspicious RCU in BTRFS_IOC_DEV_INFO
>
> fs/btrfs/check-integrity.c | 6 +--
> fs/btrfs/dev-replace.c | 19 ++++----
> fs/btrfs/disk-io.c | 6 +--
> fs/btrfs/extent_io.c | 4 +-
> fs/btrfs/ioctl.c | 14 +++---
> fs/btrfs/raid56.c | 2 +-
> fs/btrfs/rcu-string.h | 56 ------------------------
> fs/btrfs/scrub.c | 15 ++++---
> fs/btrfs/super.c | 2 +-
> fs/btrfs/volumes.c | 107 ++++++++++++++++++++++++++++++---------------
> fs/btrfs/volumes.h | 2 +-
> include/linux/rcustring.h | 97 ++++++++++++++++++++++++++++++++++++++++
> 12 files changed, 204 insertions(+), 126 deletions(-)
> delete mode 100644 fs/btrfs/rcu-string.h
> create mode 100644 include/linux/rcustring.h
>
> --
> 2.2.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RESEND v8 0/3] Introduce RCU string API
2015-01-10 19:56 ` [PATCH RESEND v8 0/3] Introduce RCU string API Paul E. McKenney
@ 2015-01-10 21:05 ` Omar Sandoval
0 siblings, 0 replies; 6+ messages in thread
From: Omar Sandoval @ 2015-01-10 21:05 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Chris Mason, Josef Bacik, David Sterba, Josh Triplett,
Steven Rostedt, Mathieu Desnoyers, Lai Jiangshan, linux-kernel,
linux-btrfs
On Sat, Jan 10, 2015 at 11:56:48AM -0800, Paul E. McKenney wrote:
> On Fri, Jan 09, 2015 at 12:35:57PM -0800, Omar Sandoval wrote:
> > Hi, everyone,
> >
> > Now that the merge window and the holidays are over, I've rebased this on
> > v3.19-rc3 for the next merge window.
>
> Wasn't btrfs going to take this one?
>
> If I am supposed to take it, please split out the RCU infrastructure
> portion so that I can push it separately.
>
> Thanx, Paul
>
> > This patch series introduces the RCU string API and cleans up the wreckage of
> > sparse warnings which follow from it (shown here from when the patch was
> > briefly in the btrfs integration tree):
> >
> > On Thu, Nov 27, 2014 at 06:45:20AM +0800, kbuild test robot wrote:
> > > tree: git://git.kernel.org/pub/scm/linux/kernel/git/mason/linux-btrfs.git integration
> > > head: c7a37618b60026121255c69e042d74ae5631470c
> > > commit: 37aad79d90a0cbf82a5eda62dfe3af4241f5aca3 [38/39] Move BTRFS RCU string to common library
> > > reproduce:
> > > # apt-get install sparse
> > > git checkout 37aad79d90a0cbf82a5eda62dfe3af4241f5aca3
> > > make ARCH=x86_64 allmodconfig
> > > make C=1 CF=-D__CHECK_ENDIAN__
> > >
> > >
> > > sparse warnings: (new ones prefixed by >>)
> > >
> > > >> fs/btrfs/check-integrity.c:848:25: sparse: incorrect type in argument 1 (different address spaces)
> > > fs/btrfs/check-integrity.c:848:25: expected struct rcu_string [noderef] <asn:4>*rcu_str
> > > fs/btrfs/check-integrity.c:848:25: got struct rcu_string *name
> > [snip]
> >
> > Version 8 combines the original patch with another patch series I posted to fix
> > these warnings, which fixes the botched __rcu annotations that caused some of
> > the warnings and refactors the existing uses of rcustring to get rid of the
> > rest. There's also a patch to fix an RCU misuse.
> >
> > Thanks!
> >
> > v8: Get the __rcu annotations right, clean up RCU string-related sparse noise
> > v7: Add arguments to kernel doc for printk wrappers, use ##__VA_ARGS
> > v6: Add header dependencies to rcustring.h
> > v5: Rebase against v3.18-rc3
> > v4: Don't return anything from the printk wrappers on the assumption that
> > printk will return void someday
> > v3: Add __rcu annotation to relevant functions, add Paul's ack and Josh's
> > review
> >
> > Omar Sandoval (3):
> > Move BTRFS RCU string to common library
> > btrfs: refactor btrfs_device->name updates
> > btrfs: fix suspicious RCU in BTRFS_IOC_DEV_INFO
> >
> > fs/btrfs/check-integrity.c | 6 +--
> > fs/btrfs/dev-replace.c | 19 ++++----
> > fs/btrfs/disk-io.c | 6 +--
> > fs/btrfs/extent_io.c | 4 +-
> > fs/btrfs/ioctl.c | 14 +++---
> > fs/btrfs/raid56.c | 2 +-
> > fs/btrfs/rcu-string.h | 56 ------------------------
> > fs/btrfs/scrub.c | 15 ++++---
> > fs/btrfs/super.c | 2 +-
> > fs/btrfs/volumes.c | 107 ++++++++++++++++++++++++++++++---------------
> > fs/btrfs/volumes.h | 2 +-
> > include/linux/rcustring.h | 97 ++++++++++++++++++++++++++++++++++++++++
> > 12 files changed, 204 insertions(+), 126 deletions(-)
> > delete mode 100644 fs/btrfs/rcu-string.h
> > create mode 100644 include/linux/rcustring.h
> >
> > --
> > 2.2.1
> >
>
Hi, Paul,
I believe Chris is still going to take this one, just wanted to keep you
and the RCU reviewers in the loop.
Thanks!
--
Omar
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-01-10 21:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-09 20:35 [PATCH RESEND v8 0/3] Introduce RCU string API Omar Sandoval
2015-01-09 20:35 ` [PATCH RESEND v8 1/3] Move BTRFS RCU string to common library Omar Sandoval
2015-01-09 20:35 ` [PATCH RESEND v8 2/3] btrfs: refactor btrfs_device->name updates Omar Sandoval
2015-01-09 20:36 ` [PATCH RESEND v8 3/3] btrfs: fix suspicious RCU in BTRFS_IOC_DEV_INFO Omar Sandoval
2015-01-10 19:56 ` [PATCH RESEND v8 0/3] Introduce RCU string API Paul E. McKenney
2015-01-10 21:05 ` Omar Sandoval
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).