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