Re: [PATCH] Btrfs: use rcu to protect device->name V2

[Josef Bacik <josef@redhat.com>]

On Mon, Jun 11, 2012 at 02:10:34PM -0700, Zach Brown wrote:
> 
> >-		if (state->print_mask&  BTRFSIC_PRINT_MASK_SUPERBLOCK_WRITE)
> >+		if (state->print_mask&  BTRFSIC_PRINT_MASK_SUPERBLOCK_WRITE) {
> >+			struct rcu_string *name;
> >+
> >+			rcu_read_lock();
> >+			name = rcu_dereference(device->name);
> >  			printk(KERN_INFO "New initial S-block (bdev %p, %s)"
> >  			       " @%llu (%s/%llu/%d)\n",
> >-			       superblock_bdev, device->name,
> >+			       superblock_bdev, name->str,
> >  			       (unsigned long long)dev_bytenr,
> >  			       dev_state->name,
> >  			       (unsigned long long)dev_bytenr,
> >  			       superblock_mirror_num);
> >+			rcu_read_unlock();
> >+		}
> 
> That's a whole lot of noise at every call site.  How about some helpers?
> In sloppy-code..
> 
> #define printk_in_rcu(args) do { rcu; printk; rcu; } while (0)
> 
> #define device_name_rcu(dev) ({
> 	struct rcu_string *name = rcu_dereference(dev->name);
> 	name->str; )}
> 
> 	printk_in_rcu("HELLO FRIENDS %s", device_name_rcu(dev));
> 
> It's kind of annoying to have a wrapper for printk(), pr_debug(), etc,
> but it sure seems better than implementing the rcu consistency rules
> around every output.  (And multiple rcu_string locals for multiple
> device names..)
> 

Oh and now I remember why I didn't do this (other than being terribly lazy), all
the call sites are too different, and there's no simple way to figure out which
arg is the device name so I can do the derefence properly.  We'd need to change
all of the messages to have a unified format so that it could just be something
like

#define device_name_printk(dev, level, fmt, ...) do {		\
	struct rcu_string *name;				\
								\
	rcu_read_lock();					\
	name = rcu_dereference(dev->name);			\
	printk(level "%s: " fmt, name->str, ##__VA_ARGS__);	\
	rcu_read_unlock();					\
	} while (0)

and I'm not even sure if that would work out right with the args.  Do we really
want to force everybody to this one convention?  If the answer is yes then
damnit I don't wanna because it's tedious ;).  Thanks,

Josef