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

[Liu Yuan]

On 05/22/2013 06:23 PM, MORITA Kazutaka wrote:
>> --- 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.
> 
> 


Can't be inline because off and len has different type.

>> +
>> +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]?
> 

I don't get it. What you mean by whole length? Actually I only read what
is needed. For example, off=513, length=512, with my calculation, we
only read [512, 1536], no more no less.

Thanks,
Yuan