On Wed, May 30, 2012 at 04:47:08PM +0900, Tejun Heo wrote:
> A couple more comments from this round of reading.
>
> On Wed, May 09, 2012 at 11:10:48PM -0400, Kent Overstreet wrote:
> > +#define btree_prio USHRT_MAX
> > +#define initial_prio 32768
>
> Why are these in lower case?
No good reason. I'll change it.
>
> > +#define PTR_BUCKET(c, k, n) \
> > + (PTR_CACHE(c, k, n)->buckets + PTR_BUCKET_NR(c, k, n))
>
> PTR_BUCKET(c, k, n)
>
> Awesome. I don't know what type it takes or what each single
> character argument stands for.
Yeah, that should be an inline function, along with some others. Fixed -
well, have to convert the rest of the code to the lowercase names:
Maybe ptr_bucket_idx() instead of ptr_bucket_nr()?
diff --git a/drivers/md/bcache/bcache.h b/drivers/md/bcache/bcache.h
index 1011a73..0617885 100644
--- a/drivers/md/bcache/bcache.h
+++ b/drivers/md/bcache/bcache.h
@@ -717,15 +717,41 @@ PTR_FIELD(PTR_GEN, 0, 8)
#define PTR(gen, offset, dev) \
((((uint64_t) dev) << 51) | ((uint64_t) offset) << 8 | gen)
-#define sector_to_bucket(c, s) ((long) ((s) >> (c)->bucket_bits))
-#define bucket_to_sector(c, b) (((sector_t) (b)) << (c)->bucket_bits)
-#define bucket_remainder(c, b) ((b) & ((c)->sb.bucket_size - 1))
+static inline size_t sector_to_bucket(struct cache_set *c, sector_t s)
+{
+ return s >> c->bucket_bits;
+}
+
+static inline sector_t bucket_to_sector(struct cache_set *c, size_t b)
+{
+ return ((sector_t) b) << c->bucket_bits;
+}
-#define PTR_CACHE(c, k, n) ((c)->cache[PTR_DEV(k, n)])
-#define PTR_BUCKET_NR(c, k, n) sector_to_bucket(c, PTR_OFFSET(k, n))
+static inline sector_t bucket_remainder(struct cache_set *c, sector_t s)
+{
+ return s & (c->sb.bucket_size - 1);
+}
-#define PTR_BUCKET(c, k, n) \
- (PTR_CACHE(c, k, n)->buckets + PTR_BUCKET_NR(c, k, n))
+static inline struct cache *ptr_cache(struct cache_set *c,
+ struct bkey *k,
+ unsigned ptr)
+{
+ return c->cache[PTR_DEV(k, ptr)];
+}
+
+static inline size_t ptr_bucket_nr(struct cache_set *c,
+ struct bkey *k,
+ unsigned ptr)
+{
+ return sector_to_bucket(c, PTR_OFFSET(k, ptr));
+}
+
+static inline struct bucket *ptr_bucket(struct cache_set *c,
+ struct bkey *k,
+ unsigned ptr)
+{
+ return ptr_cache(c, k, ptr)->buckets + ptr_bucket_nr(c, k, ptr);
+}
/* Btree key macros */
>
> > +static inline bool cached_dev_get(struct cached_dev *d)
> > +{
> > + if (!atomic_inc_not_zero(&d->count))
> > + return false;
> > +
> > + smp_mb__after_atomic_inc();
>
> What is this mb() paired with? Whenever using a mb, please specify
> what the mb is paired with.
super.c, cached_dev_attach():
smp_wmb();
/* d->c must be set before d->count != 0 */
atomic_set(&d->count, 1);
I'm improving the comments.
>
> > + return true;
> > +}
>
> Thanks.
>
> --
> tejun
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
