From 215fd62315de581b4aead96221547d106a534e61 Mon Sep 17 00:00:00 2001 From: Petr Tesarik Date: Fri, 2 Nov 2012 13:36:12 +0100 Subject: [PATCH] Rewrite hed_file_commit() There old implementation was hard to maintain, because it iterated over the logical blocks, updating struct commit_control after each step. As a result, it needed up to THREE temporary buffers to handle different forwards and backwards written chunks. The new paradigm is different. Basically, it makes use of the existing abstraction from the underlying block structure to split the logical file into FILE_BLOCK_SIZE-sized chunks, which are then written to disk. This reduces memory consumption (one buffer is always sufficient), and also makes it easier to implement proper error handling (not done yet!). Note that block-size granularity also applies to the last (partial) block in the file, truncating the file afterwards. This can be an issue when editing a file on a full device if the device block size is smaller than hed's FILE_BLOCK_SIZE. --- libhed/file.c | 274 +++++++++++++++++++++------------------------------------- 1 file changed, 98 insertions(+), 176 deletions(-) diff --git a/libhed/file.c b/libhed/file.c index daa41c5..d0e248a 100644 --- a/libhed/file.c +++ b/libhed/file.c @@ -1860,188 +1860,100 @@ hed_file_insert_once(struct hed_file *file, hed_cursor_t *curs, struct commit_control { struct file_priv *file; int wfd; /* file descriptor for writing */ - int needwrite; /* non-zero if write is needed */ hed_cursor_t begoff, endoff; - hed_off_t shift; - void *partbuf; /* allocated 3*FILE_BLOCK_SIZE */ - void *partial; /* pointer into partbuf */ + void *buffer; /* write buffer (one block) */ }; -/* Get the logical<->physical shift value after the specified block. - * It is essential to get the value _AFTER_ the block, because the - * shift value is used to decide how the current block will be handled. - */ -static hed_off_t -get_shift(const hed_cursor_t *curs) -{ - struct hed_block *block = curs->block; - hed_off_t curshift = curs->pos - block->phys_pos; - return hed_block_is_inserted(block) - ? curshift + hed_block_size(block) - : curshift; -} - -static void -switch_partial(struct commit_control *cc) -{ - cc->partial += FILE_BLOCK_SIZE; - if (cc->partial >= cc->partbuf + 3*FILE_BLOCK_SIZE) - cc->partial = cc->partbuf; -} - -/* Write @writelen bytes from the partial buffer at @cc->begoff. */ -static int -commit_block(struct commit_control *cc, size_t len) +static ssize_t +commit_write(struct commit_control *cc, hed_uoff_t pos, ssize_t len) { - ssize_t written; - - assert(len > 0); BDEBUG(" -> write %lx bytes at %llx\n", - (unsigned long)len, cc->begoff.pos - len); - written = pwrite(cc->wfd, cc->partial, len, cc->begoff.pos - len); - if (written < len) - /* TODO: keep data in a new list of dirty blocks */ - return -1; - return 0; -} - -static int -commit_partial(struct commit_control *cc) -{ - size_t partoff, remain, left; - size_t writelen; - - partoff = FILE_BLOCK_OFF(cc->begoff.pos); - remain = FILE_BLOCK_SIZE - partoff; - if (remain > cc->endoff.pos - cc->begoff.pos) - remain = cc->endoff.pos - cc->begoff.pos; - if ((writelen = partoff + remain) == 0) - return 0; - - BDEBUG("Fill partial %llx-%llx\n", - cc->begoff.pos, cc->begoff.pos + remain); - - left = copy_in(cc->file, cc->partial + partoff, remain, &cc->begoff); - if (left) { - hed_move_relative(&cc->begoff, left); - return -1; - } - - if (FILE_BLOCK_OFF(cc->begoff.pos) && - !hed_block_is_terminal(cc->begoff.block)) - return 0; - - return commit_block(cc, writelen); + (unsigned long)len, pos); + return pwrite(cc->wfd, cc->buffer, len, pos); } -/* Commit forwards. - * Beware, cc->begoff is undefined upon return! - */ +/* Commit forwards. */ static int commit_forwards(struct commit_control *cc) { - hed_uoff_t endpos = cc->endoff.pos; int ret = 0; BDEBUG("Writing forwards %llx-%llx\n", cc->begoff.pos, cc->endoff.pos); - if (!cc->needwrite) - return 0; + while (cc->begoff.pos < cc->endoff.pos) { + size_t left; + ssize_t written; - while (cc->begoff.pos < endpos) - ret |= commit_partial(cc); + left = copy_in(cc->file, cc->buffer, + FILE_BLOCK_SIZE, &cc->begoff); + if (left) { + move_rel_fast(&cc->begoff, left); + ret = -1; + continue; + } + + written = commit_write(cc, cc->begoff.pos - FILE_BLOCK_SIZE, + FILE_BLOCK_SIZE); + if (written < FILE_BLOCK_SIZE) + ret = -1; + } return ret; } -/* Commit forwards. - * Beware, cc->begoff is undefined upon return! - */ +/* Commit backwards. */ static int commit_backwards(struct commit_control *cc) { - void *retpartial = cc->partial; - hed_uoff_t begpos = cc->begoff.pos; - hed_uoff_t blkpos; /* start of current partial block */ + hed_cursor_t curs; int ret = 0; BDEBUG("Writing backwards %llx-%llx\n", cc->begoff.pos, cc->endoff.pos); - if (!cc->needwrite) - return 0; - - blkpos = FILE_BLOCK_ROUND(cc->endoff.pos); - if (blkpos <= begpos) - goto final; - - /* Handle the trailing partial block */ - hed_update_cursor(&cc->file->f, blkpos, &cc->begoff); - switch_partial(cc); - ret |= commit_partial(cc); - retpartial = cc->partial; - - /* Handle the middle part */ - switch_partial(cc); - while ( (blkpos -= FILE_BLOCK_SIZE) > begpos) { - hed_update_cursor(&cc->file->f, blkpos, &cc->begoff); - ret |= commit_partial(cc); - } - switch_partial(cc); /* wrap around */ - - final: - /* Handle the first block (partiall or not) */ - hed_update_cursor(&cc->file->f, begpos, &cc->begoff); - ret |= commit_partial(cc); + hed_dup_cursor(&cc->endoff, &curs); + while (curs.pos > cc->begoff.pos) { + size_t left; + ssize_t written; - cc->partial = retpartial; - return ret; -} - -/* Handle the partial block before a skipped one. */ -static int -begin_skip(struct commit_control *cc) -{ - size_t minsize = FILE_BLOCK_SIZE - FILE_BLOCK_OFF(cc->endoff.pos); - size_t remain; - int ret = 0; - - /* Check if at least one complete physical block can be skipped */ - if (cc->endoff.block->t.size < minsize) - return 0; + move_rel_fast(&curs, -FILE_BLOCK_SIZE); + left = hed_file_cpin(&cc->file->f, cc->buffer, + FILE_BLOCK_SIZE, &curs); + if (left) { + ret = -1; + continue; + } - /* Write out the partially dirty block */ - remain = FILE_BLOCK_OFF(minsize); - hed_move_relative(&cc->endoff, remain); - if (cc->shift <= 0) - ret |= commit_forwards(cc); - else - ret |= commit_backwards(cc); - hed_move_relative(&cc->endoff, -(hed_off_t)remain); - hed_dup2_cursor(&cc->endoff, &cc->begoff); + written = commit_write(cc, curs.pos, FILE_BLOCK_SIZE); + if (written < FILE_BLOCK_SIZE) + ret = -1; + } + hed_put_cursor(&curs); - cc->needwrite = 0; return ret; } -/* Handle the last partially skipped physical block. */ -static int -end_skip(struct commit_control *cc) +/* Return the number of clean bytes following @curs. + * Usage note: the caller must ensure that the starting position + * is clean. + */ +static hed_uoff_t +clean_span(hed_cursor_t *curs) { - size_t partlen; - int ret = 0; - - /* Find the beginning of the physical block */ - hed_dup2_cursor(&cc->endoff, &cc->begoff); - partlen = FILE_BLOCK_OFF(cc->begoff.pos); - hed_move_relative(&cc->begoff, -(hed_off_t)partlen); + struct hed_block *block = curs->block; + hed_uoff_t ret = -curs->off; - /* Read the partial data before this block */ - if (hed_file_cpin(&cc->file->f, cc->partial, partlen, &cc->begoff)) - ret = -1; + assert(!hed_block_is_dirty(block)); - cc->needwrite = 1; + do { + hed_uoff_t next_pos; + ret += hed_block_size(block); + next_pos = block->phys_pos + hed_block_size(block); + block = next_nonzero_block(block); + if (block->phys_pos != next_pos) + break; + } while (!hed_block_is_dirty(block) && !hed_block_is_terminal(block)); return ret; } @@ -2088,9 +2000,8 @@ static int commit_init(struct commit_control *cc, struct file_priv *file) { cc->file = file; - - cc->partbuf = malloc(3*FILE_BLOCK_SIZE); - if (!cc->partbuf) + cc->buffer = malloc(FILE_BLOCK_SIZE); + if (!cc->buffer) goto err; if (file->fd < 0) { @@ -2106,16 +2017,25 @@ commit_init(struct commit_control *cc, struct file_priv *file) return 0; err_free: - free(cc->partbuf); + free(cc->buffer); err: return -1; } +static int +commit_current(struct commit_control *cc, hed_off_t shift) +{ + return shift <= 0 + ? commit_forwards(cc) + : commit_backwards(cc); +} + int hed_file_commit(struct hed_file *f) { struct file_priv *file = file_private(f); struct commit_control cc; + hed_off_t shift; int ret = 0; if (commit_init(&cc, file)) @@ -2123,44 +2043,46 @@ hed_file_commit(struct hed_file *f) dump_blocks(file); - cc.partial = cc.partbuf; get_cursor(file, 0,&cc.begoff); hed_dup_cursor(&cc.begoff, &cc.endoff); - cc.shift = -cc.begoff.block->phys_pos; - cc.needwrite = 0; + shift = -cc.begoff.block->phys_pos; while(!hed_block_is_terminal(cc.endoff.block)) { - hed_off_t newshift = !hed_block_is_eof(cc.endoff.block) - ? get_shift(&cc.endoff) + hed_uoff_t skip; + hed_off_t newshift; + + if (!shift && !hed_block_is_dirty(cc.endoff.block)) { + skip = FILE_BLOCK_ROUND(clean_span(&cc.endoff)); + if (skip) { + ret |= commit_current(&cc, shift); + + BDEBUG("Skipping %llx-%llx\n", + cc.endoff.pos, cc.endoff.pos + skip); + hed_move_relative(&cc.endoff, skip); + hed_dup2_cursor(&cc.endoff, &cc.begoff); + continue; + } + } + + skip = FILE_BLOCK_ROUND(hed_cursor_span(&cc.endoff)); + hed_move_relative(&cc.endoff, skip ?: FILE_BLOCK_SIZE); + + newshift = !hed_block_is_eof(cc.endoff.block) + ? cc.endoff.pos - hed_cursor_phys_pos(&cc.endoff) : 0; - if (cc.shift <= 0 && newshift > 0) { + if (shift <= 0 && newshift > 0) { + move_rel_fast(&cc.endoff, -FILE_BLOCK_SIZE); ret |= commit_forwards(&cc); hed_dup2_cursor(&cc.endoff, &cc.begoff); - } else if (cc.shift > 0 && newshift <= 0) { + } else if (shift > 0 && newshift <= 0) { ret |= commit_backwards(&cc); hed_dup2_cursor(&cc.endoff, &cc.begoff); } - cc.shift = newshift; - - if (!newshift && !hed_block_is_dirty(cc.endoff.block)) { - if (cc.needwrite) - ret |= begin_skip(&cc); - } else if (!cc.needwrite) - ret |= end_skip(&cc); - - cc.endoff.pos += hed_block_size(cc.endoff.block); - if (!cursor_next_block(&cc.endoff)) - break; - } - assert(cc.endoff.pos == hed_file_size(&file->f)); - - if (cc.begoff.pos < hed_file_size(&file->f)) { - if (cc.shift <= 0) - ret |= commit_forwards(&cc); - else - ret |= commit_backwards(&cc); + shift = newshift; } + assert(cc.endoff.pos >= hed_file_size(&file->f)); + ret |= commit_current(&cc, shift); put_cursor(&cc.begoff); put_cursor(&cc.endoff); @@ -2168,7 +2090,7 @@ hed_file_commit(struct hed_file *f) file->f.phys_size = hed_file_size(&file->f); ret |= close(cc.wfd); - free(cc.partbuf); + free(cc.buffer); undirty_blocks(file); -- 2.11.4.GIT