[sheepdog] [PATCH 2/3] object_cache: fix dirty bitmap size

MORITA Kazutaka morita.kazutaka at lab.ntt.co.jp
Fri Aug 2 10:55:13 CEST 2013


At Fri, 2 Aug 2013 16:08:14 +0800,
Liu Yuan wrote:
> 
> On Fri, Aug 02, 2013 at 04:14:40PM +0900, MORITA Kazutaka wrote:
> > From: MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp>
> > 
> > The type of bitmap is not uint64_t, but unsigned long.
> > 
> > Signed-off-by: MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp>
> > ---
> >  sheep/object_cache.c |   10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/sheep/object_cache.c b/sheep/object_cache.c
> > index 27d544d..c12719b 100644
> > --- a/sheep/object_cache.c
> > +++ b/sheep/object_cache.c
> > @@ -28,7 +28,7 @@
> >  
> >  #define CACHE_INDEX_MASK      (CACHE_CREATE_BIT)
> >  
> > -#define CACHE_BLOCK_SIZE      ((UINT64_C(1) << 10) * 64) /* 64 KB */
> > +#define CACHE_BLOCK_SIZE      (BITOP_WORD(SD_DATA_OBJ_SIZE))
> 
> What is BITOP_WORD? We assume CACHE_BLOCK_SIZE equals to 64K and then bitmap
> has 64 bits. unsiged long in 32 bit machine will only have 32 bits. Did you
> mean we get for 32 bit, CACHE_BLOCK_SIZE=128K, for 64 bit, = 64K automatically?

IIUC, BITOP_WORD replaces the number of bits to the number of words.
Maybe, I'm abusing the function here.

The aim of this change is to guarantee that CACHE_BLOCK_SIZE must be
less than (SD_DATA_OBJ_SIZE / sizeof(bmap)).  Otherwise, set_bit() in
calc_object_bmap() causes a buffer overflow.  We use unsigned long for
the bitmap in calc_object_bmap(), so the 64 KB cache size on 32 bit
architecture is not safe.  With the current implementation,
CACHE_BLOCK_SIZE must be larger than 128 KB on 32 bit architectures.

Another (and probably simpler) fix is using uint64_t for bitmap in
calc_object_bmap.  Then it is safe to use 64 KB CACHE_BLOCK_SIZE on 32
bit machines.

Thanks,

Kazutaka



More information about the sheepdog mailing list