Re: Accepted dpkg 1.15.8.7 (source all amd64)
Hi!
On Fri, 2011-03-11 at 11:09:07 +0100, Sven Mueller wrote:
> Am 29.12.2010 20:04, schrieb Guillem Jover:
> > On Wed, 2010-12-29 at 17:58:41 +0100, Raphael Hertzog wrote:
> >> It is ok since fsync() is still called afterwards. Both are only useful
> >> to avoid the speed-penalty that fsync() was giving us, but fsync() is
> >> still executed (and is usualy a no-op since the work has already been done
> >> thanks to the sync_file_range calls).
> >
> > Exactly, and although SYNC_FILE_RANGE_WAIT_BEFORE can return with EIO
> > or ENOSPC, if that would happen the pages would not have been flushed
> > and would still be dirty, and fsync() would go over them again.
> >
> [...]
> >
> > This was done on purpose, as those actions should be considered
> > equivalent to asynchronous hints for the kernel from dpkg's code PoV.
>
> Forgive me if this was already done, but should this be documented in
> the code? I mean something like "We are ignoring the return value of
> SYNC_FILE_RANGE_WAIT_BEFORE on purpose since we call fsync() later on
> anyhow." near the SYNC_FILE... call and a "don't remove the fsync call
> unless you check the SYNC_FILE_.... system call return values" near the
> fsync call.
> In other words: Add comments why you do it the way you do it. Don't
> bother with comments that explain what you do.
I've added now some comments around the writeback code explaining this:
<http://git.debian.org/?p=dpkg/dpkg.git;a=commitdiff;h=a468c108>
thanks,
guillem
Reply to: