[sheepdog] [PATCH 2/4] plain store: add support to non-aglined read/write

MORITA Kazutaka morita.kazutaka at lab.ntt.co.jp
Wed May 22 12:23:16 CEST 2013


> --- a/sheep/plain_store.c
> +++ b/sheep/plain_store.c
> @@ -37,6 +37,11 @@ static int get_open_flags(uint64_t oid, bool create)
>  	return flags;
>  }
>  
> +static inline bool flags_direct(int flags)
> +{
> +	return flags & O_DIRECT;
> +}
> +

Does this function really help us?  Inlining (flags & O_DIRECT) looks
simpler and easier to understand to me.


>  static int get_obj_path(uint64_t oid, char *path)
>  {
>  	return snprintf(path, PATH_MAX, "%s/%016" PRIx64,
> @@ -90,6 +95,38 @@ static int err_to_sderr(char *path, uint64_t oid, int err)
>  	}
>  }
>  
> +#define sector_algined(x) ({(x) % SECTOR_SIZE == 0;})

I prefer it to be a inline function, and I think
((x) & (SECTOR_SIZE - 1)) is faster.


> +
> +static inline bool is_aligned(const struct siocb *iocb)

The name 'is_aligned' is too generic.  'iocb_is_aligned' looks better.

> +{
> +	  return  sector_algined(iocb->offset) && sector_algined(iocb->length);
> +}
> +
> +static inline int do_aligned_write(uint64_t oid, const struct siocb *iocb)
> +{
> +	struct siocb new = {
> +		.offset = round_down(iocb->offset, SECTOR_SIZE),
> +		.length = round_up(iocb->offset + iocb->length, SECTOR_SIZE) -
> +			  round_down(iocb->offset, SECTOR_SIZE),
> +		.epoch = iocb->epoch,
> +	};
> +	int ret = SD_RES_SUCCESS;
> +
> +	sd_dprintf("new %"PRIu64 ", %"PRIu32 ", old %"PRIu64 ", %"PRIu32,
> +		   new.offset, new.length, iocb->offset, iocb->length);
> +	new.buf = xvalloc(new.length);

I wonder if it is a good way to read the whole length.  The only
missing data is a sub-sector, which is less than 512 bytes.  How about
reading only [new.offset..iocb->offset] and
[iocb->length..new.length]?

Thanks,

Kazutaka



More information about the sheepdog mailing list