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
|
|