Re: Accepted dpkg 1.15.8.7 (source all amd64)
- To: Guillem Jover <guillem@debian.org>, debian-dpkg@lists.debian.org
- Subject: Re: Accepted dpkg 1.15.8.7 (source all amd64)
- From: Sven Mueller <sven@debian.org>
- Date: Fri, 11 Mar 2011 11:09:07 +0100
- Message-id: <[🔎] 4D79F4C3.7040006@debian.org>
- In-reply-to: <20101229190454.GA6545@gaara.hadrons.org>
- References: <E1PUVXt-0004jc-5X@franck.debian.org> <20101229153207.GY2813@radis.liafa.jussieu.fr> <20101229165841.GC18137@rivendell.home.ouaza.com> <20101229190454.GA6545@gaara.hadrons.org>
Am 29.12.2010 20:04, schrieb Guillem Jover:
> Hi!
>
> On Wed, 2010-12-29 at 17:58:41 +0100, Raphael Hertzog wrote:
>> On Wed, 29 Dec 2010, Julien Cristau wrote:
>>
>>> You're ignoring the return value of sync_file_range (and posix_fadvise).
>>> While that may be ok for SYNC_FILE_RANGE_WRITE, I don't think it is for
>>> SYNC_FILE_RANGE_WAIT_BEFORE.
>>
>> 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.
;-)
regards,
Sv<just stumbled over this discussion>en
Reply to: