On Tue, 2012-06-12 at 10:24 -0700, Kent Overstreet wrote:
> On Tue, Jun 12, 2012 at 09:50:58AM -0700, Joe Perches wrote:
> > 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.
>
> I've got the pr_fmt, but I don't want to use it here because it's
> dumping a btree node (100s of lines) so the bcache: would be redundant,
> but more importantly I don't want lines getting truncated.
>
> > Doesn't this throw a gcc warning for argument mismatch?
>
> No, why?
"(uint64_t *)k - i->d" is what type again?
What is a %zu?
Isn't that a mismatch?
> > > +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.
>
> It is a strange hack.
>
> The idea is that we want to indent more as we recurse; we could build up
> a new string of tabs each time we recurse that's got one more tab than
> our parent's, but that'd be a pain in the ass and it'd use more stack
> space (though that should be fine here), so instead it's just
> decrementing the pointer to the tab string to produce a string with one
> more tab.
It's a nice idea, but that's not what's happening
as I believe you reference tabs only once as &tabs[4]
> I'm not opposed to taking it out if you know cleaner way that isn't
> ridiculously verbose. But this code needs to be rewritten to not use
> single_open() (which I tihnk is going to be a pain in the ass) so it's
> not really at the top of my list.
>
> >
> > > + 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, §ors);
> > > +
> >
> > Why not just:
> >
> > btree_root(dump, c, &op, "\t", &last, §ors);
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
