Re: Accepted dpkg 1.15.8.7 (source all amd64)

[Guillem Jover <guillem@debian.org>]

Hi!

On Wed, 2010-12-29 at 17:58:41 +0100, Raphael Hertzog wrote:
> On Wed, 29 Dec 2010, Julien Cristau wrote:
> > On Mon, Dec 20, 2010 at 02:32:17 +0000, Guillem Jover wrote:
> > >    * On Linux use sync_file_range() to initiate asynchronous writeback
> > >      of just unpacked files. Suggested by Ted Ts'o <tytso@mit.edu>.
> > >      Thanks to Jonathan Nieder <jrnieder@gmail.com>. Closes: #605009
> > >    * On non-Linux use posix_fadvise(POSIX_FADV_DONTNEED) to notify the kernel
> > >      dpkg does not need the unpacked files any longer, and that it can start
> > >      writeback to be able to evict them from the cache at a later point.
> > 
> > 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.

So while we could fail earlier, this is not really an issue, and it
might actually give some time in some cases (ENOSPC particularly) for
the system to get to a state were no failure migt occur, which seems
better to me.

This was done on purpose, as those actions should be considered
equivalent to asynchronous hints for the kernel from dpkg's code PoV.

thanks,
guillem