Move BTRFS RCU string to common library

PATCH Superseded Omar Sandoval 2 participants 6 messages Sep 27, 2014

Subsystems (3)

Btrfs file system RCU Printk

lore

Share & tools

lore

Omar Sandoval

author

2014-09-27 05:35 UTC #0 lore raw

v3 cover Move BTRFS RCU string to common library

This patch series makes the generic RCU string library used internally by BTRFS accessible by anyone.

The first patch makes printk_ratelimited pass on the return value from printk. Version 3 gives the temporary return variable a unique name to avoid name clashes.

The second patch moves the RCU string header and updates the BTRFS code that depends on it. Version 3 adds the __rcu annotation to the relevant functions.

Version 3 also adds Paul's ack and Josh's review.

Omar Sandoval (2):
Return a value from printk_ratelimited
Move BTRFS RCU string to common library

 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/printk.h     |  4 +-
 include/linux/rcustring.h  | 92 ++++++++++++++++++++++++++++++++++++++++++++++
 12 files changed, 132 insertions(+), 92 deletions(-)
 delete mode 100644 fs/btrfs/rcu-string.h
 create mode 100644 include/linux/rcustring.h

Signature · 1 line

2.1.1

Omar Sandoval

author

2014-09-27 05:35 UTC #1 lore raw

PATCH v3 1/2 Return a value from printk_ratelimited

printk returns an integer; there's no reason for printk_ratelimited to swallow it.

Signed-off-by Omar Sandoval <[email protected]>

Acked-by Paul E. McKenney <[email protected]>

---
 include/linux/printk.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

include/linux/printk.h +3 -1

index d78125f..89bb7ab 100644
@@ -343,12 +343,14 @@ extern asmlinkage void dump_stack(void) __cold;
 #ifdef CONFIG_PRINTK
 #define printk_ratelimited(fmt, ...)					\
 ({									\
+	int __ret_printk_ratelimited = 0;				\
 	static DEFINE_RATELIMIT_STATE(_rs,				\
 				      DEFAULT_RATELIMIT_INTERVAL,	\
 				      DEFAULT_RATELIMIT_BURST);		\
 									\
 	if (__ratelimit(&_rs))						\
-		printk(fmt, ##__VA_ARGS__);				\
+		__ret_printk_ratelimited = printk(fmt, ##__VA_ARGS__);	\
+	__ret_printk_ratelimited;					\
 })
 #else
 #define printk_ratelimited(fmt, ...)					\

Signature · 1 line

2.1.1

Joe Perches

2014-09-27 05:48 UTC #2 lore raw

Re: [PATCH v3 1/2] Return a value from printk_ratelimited

Quoted 6 lines · Omar Sandoval

printk returns an integer; there's no reason for printk_ratelimited to swallow it.
Signed-off-by Omar Sandoval <[email protected]>
Acked-by Paul E. McKenney <[email protected]>
---

I'd prefer to keep it the way it is actually.

I've submitted several patches to convert the int return to void for printk derived functions recently.

Omar Sandoval

author

2014-09-27 07:44 UTC #3 lore raw

Re: [PATCH v3 1/2] Return a value from printk_ratelimited

Quoted 4 lines · Joe Perches

I'd prefer to keep it the way it is actually.
I've submitted several patches to convert the int return to void for printk derived functions recently.

Alright, it doesn't seem like anyone feels strongly for keeping the return values, so I'll go ahead and submit a v4 that doesn't modify printk_ratelimited and doesn't return anything from the printk functions in rcustring.h unless there are any objections.

Signature · 1 line

Omar

Omar Sandoval

author

2014-09-27 05:35 UTC #4 lore raw

PATCH v3 2/2 Move BTRFS RCU string to common library

In reply to 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.

Signed-off-by Omar Sandoval <[email protected]>

Reviewed-by Josh Triplett <[email protected]>

Acked-by Paul E. McKenney <[email protected]>

---
 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  | 92 ++++++++++++++++++++++++++++++++++++++++++++++
 11 files changed, 129 insertions(+), 91 deletions(-)
 delete mode 100644 fs/btrfs/rcu-string.h
 create mode 100644 include/linux/rcustring.h

fs/btrfs/check-integrity.c +3 -3

index ce92ae3..4ccd7da 100644
@@ -94,6 +94,7 @@
 #include <linux/mutex.h>
 #include <linux/genhd.h>
 #include <linux/blkdev.h>
+#include <linux/rcustring.h>
 #include "ctree.h"
 #include "disk-io.h"
 #include "hash.h"

@@ -103,7 +104,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

@@ -851,8 +851,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);

fs/btrfs/dev-replace.c +10 -9

index eea26e1..87d10cc 100644
@@ -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"

@@ -376,9 +376,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));
 
 	tgt_device->total_bytes = src_device->total_bytes;
 	tgt_device->disk_total_bytes = src_device->disk_total_bytes;

@@ -528,9 +528,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->fs_devices->device_list_mutex);
 		mutex_unlock(&root->fs_info->chunk_mutex);

@@ -544,9 +545,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;

@@ -805,10 +806,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);
 	}

fs/btrfs/disk-io.c +3 -3

index a1d36e6..00f7441 100644
@@ -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"

@@ -3059,7 +3059,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
 		 */

@@ -3247,7 +3247,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;

fs/btrfs/extent_io.c +2 -2

index af0359d..441dccd 100644
@@ -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;

@@ -2065,7 +2065,7 @@ int repair_io_failure(struct btrfs_fs_info *fs_info, u64 start,
 	printk_ratelimited_in_rcu(KERN_INFO
 			"BTRFS: read error corrected: ino %lu off %llu "
 		    "(dev %s sector %llu)\n", page->mapping->host->i_ino,
-		    start, rcu_str_deref(dev->name), sector);
+		    start, rcu_string_dereference(dev->name), sector);
 
 	bio_put(bio);
 	return 0;

fs/btrfs/ioctl.c +2 -2

index 8a8e298..9d4cd72 100644
@@ -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"

@@ -1583,7 +1583,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);

fs/btrfs/raid56.c +1 -1

index 0a6b6e4..eecf738 100644
@@ -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

fs/btrfs/rcu-string.h deleted +0 ~~-56~~

56 lines not on this page · raw message

fs/btrfs/scrub.c +8 -7

index f4a41f3..62e335f 100644
@@ -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"
 
 /*

@@ -519,7 +519,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]);

@@ -531,7 +532,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);

@@ -595,7 +596,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,

@@ -796,7 +797,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);

@@ -1198,7 +1199,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:

@@ -1207,7 +1208,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:

fs/btrfs/super.c +1 -1

index c4124de..cf4db62 100644
@@ -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"

fs/btrfs/volumes.c +7 -7

index 2c2d6d1..ada5f86 100644
@@ -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"

@@ -6322,7 +6322,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;
 	}

@@ -6333,7 +6333,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;

@@ -6346,8 +6346,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;
 		}
 	}

@@ -6401,7 +6401,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),

@@ -6421,7 +6421,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),

include/linux/rcustring.h new file +92 -0

92 lines not on this page · raw message

Signature · 1 line

2.1.1

Omar Sandoval

author

2014-09-27 05:45 UTC #5 lore raw

In reply to Omar Sandoval

Quoted 16 lines · Omar Sandoval

This patch series makes the generic RCU string library used internally by BTRFS accessible by anyone.
The first patch makes printk_ratelimited pass on the return value from printk. Version 3 gives the temporary return variable a unique name to avoid name clashes.
The second patch moves the RCU string header and updates the BTRFS code that depends on it. Version 3 adds the __rcu annotation to the relevant functions.
Version 3 also adds Paul's ack and Josh's review.
Omar Sandoval (2):
Return a value from printk_ratelimited
Move BTRFS RCU string to common library

Quoted 16 lines

 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/printk.h     |  4 +-
 include/linux/rcustring.h  | 92 ++++++++++++++++++++++++++++++++++++++++++++++
 12 files changed, 132 insertions(+), 92 deletions(-)
 delete mode 100644 fs/btrfs/rcu-string.h
 create mode 100644 include/linux/rcustring.h

I forgot to mention: this patch series is against 3.17-rc6.

Signature · 1 line

Omar