lkml.org 
[lkml]   [2012]   [Jun]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [Bcache v14 16/16] bcache: Debug and tracing code
From
Date
On Tue, 2012-06-12 at 08:39 -0700, Kent Overstreet wrote:

> diff --git a/drivers/md/bcache/debug.c b/drivers/md/bcache/debug.c

> +static void dump_bset(struct btree *b, struct bset *i)
> +{
> + for (struct bkey *k = i->start; k < end(i); k = bkey_next(k)) {
> + printk(KERN_ERR "block %zu key %zu/%i: %s", index(i, b),
> + (uint64_t *) k - i->d, i->keys, pkey(k));

Add #define pr_fmt and use pr_<level> not printk everywhere.
Doesn't this throw a gcc warning for argument mismatch?

> +static void vdump_bucket_and_panic(struct btree *b, const char *m, va_list args)
> +{

m should be renamed fmt

> + struct bset *i;
> +
> + console_lock();
> +
> + for_each_sorted_set(b, i)
> + dump_bset(b, i);
> +
> + vprintk(m, args);
> +
> + console_unlock();
> +
> + panic("at %s\n", pbtree(b));
> +}
> +
> +static void dump_bucket_and_panic(struct btree *b, const char *m, ...)
> +{

here too.

> + va_list args;
> + va_start(args, m);
> + vdump_bucket_and_panic(b, m, args);
> + va_end(args);
> +}
> +
> +static void __maybe_unused
> +dump_key_and_panic(struct btree *b, struct bset *i, int j)
> +{
> + long bucket = PTR_BUCKET_NR(b->c, node(i, j), 0);
> + long r = PTR_OFFSET(node(i, j), 0) & ~(~0 << b->c->bucket_bits);
> +
> + printk(KERN_ERR "level %i block %zu key %i/%i: %s "
> + "bucket %llu offset %li into bucket\n",

coalesce formats please.

> + b->level, index(i, b), j, i->keys, pkey(node(i, j)),
> + (uint64_t) bucket, r);
> + dump_bucket_and_panic(b, "");
> +}

[]

> +static int debug_seq_show(struct seq_file *f, void *data)
> +{
> + static const char *tabs = "\t\t\t\t\t";

Seems a _very_ odd use.

> + uint64_t last = 0, sectors = 0;
> + struct cache_set *c = f->private;
> +
> + struct btree_op op;
> + bch_btree_op_init_stack(&op);
> +
> + btree_root(dump, c, &op, f, &tabs[4], &last, &sectors);
> +

Why not just:

btree_root(dump, c, &op, "\t", &last, &sectors);

Please don't be lazy when modifying code.

[]

> diff --git a/drivers/md/bcache/debug.h b/drivers/md/bcache/debug.h
[]
> +#define KEYHACK_SIZE 80
> +struct keyprint_hack {
> + char s[KEYHACK_SIZE];
> +};

structures named _hack are generally a bad idea.




\
 
 \ /
  Last update: 2012-06-12 19:41    [from the cache]
©2003-2014 Jasper Spaans. hosted at Digital Ocean