unix.c: use posix_fallocate()

[wladmis]

Using of posix_fallocate() guarantees that, if it succeed, the
attempting to write to allocated space range does not fail because of
lack of storage space. This prevents SIGBUS when trying to write to
mmaped file and no space left.

Co-Authored-by: Ivan Zakharyaschev imz@altlinux.org
Reported-by: Mikhail Kulagin <m.kulagin at postgrespro dot ru>

[knet-ci-bot]

Can one of the admins verify this patch?

[chrissie-c]

test this please

[imz]

As discovered by @wladmis , a newline was lost unfortunately.

Suggested change

	#endif
	#endif

[imz]

The resulting patch is quite elegant and robust; on systems without posix_fallocate, the old code employing write would be at work as a fallback.

---

A sidenote: We have seen problems on linux because of write not actually allocating disk space. Perhaps, the problem is that linux optimizes write(fd, calloc(1, write_size), write_size) as creating a hole, because it sees that the pointer returned by calloc points to a zeroed page (also as a result of optimizing calloc).

---

Another optimization idea: further in the code, this fd is mmapped and zeroed with memset before usage. (Something to be checked more accurately!) Now, if we see that write(fd, calloc(1, write_size), write_size) is optimized by linux and works faster than memset (because we got the actual SIGBUS "no space" only when reaching the memset), we could stick to the write(fd, calloc(1, write_size), write_size) method of zeroing instead of memset if zeroing is needed by the logic of the code.

[chrissie-c]

retest this please

[chrissie-c]

I'm not sure how much optimisation that code needs (though I am in favour of posix_fallocate() part, thank you) as it's only called at rb_open() time.

[imz]

I'm not sure how much optimisation that code needs (though I am in favour of posix_fallocate() part, thank you) as it's only called at rb_open() time.

Ok, so we shouldn't bother with optimizing memset as something interesting.

Continuing to think about this: As far as I can see now, even doing write after posix_fallocate shouldn't be needed to get the newly allocated blocks be read as zeros. Several sources coincide on saying they will be read as zeros:

• some tests recently run by @wladmis
• an argument given at stackoverflow:

The original question says something about writing zeros to the file. None of these calls write anything but metadata. If you read from space that's been preallocated but not yet written, you'll get zeros (not whatever was in that disk space previously, that would be a big security hole). You can only read up to the end of a file (the length, set by fallocate, ftruncate, or various other ways)

• NetBSD's manpage:

When posix_fallocate() is applied to an unallocated region in a regular file (a ``hole''), the hole is filled and the visible contents are unaffected; both holes and newly allocated regions read as all zeros.

The old existing blocks in the file remain unzeroed, though. If we consider qb_sys_mmap_file_open in isolation, can this mean that this patch changes its intended behavior in non-equivalent manner?.. (I.e., posix_fallocate not zeroing the initial bytes.) No, because it is used from qb_rb_open in rungbuffer.c; there the flags are set simultanously:

	if (flags & QB_RB_FLAG_CREATE) {
		file_flags |= O_CREAT | O_TRUNC;
	}

So, the zeroing used to happen when O_CREAT is set, and O_TRUNC is set then always, too. So, first, the file is truncated to zero length, and only then extended by the new blocks.

[imz]

The old existing blocks in the file remain unzeroed, though. If we consider qb_sys_mmap_file_open in isolation, can this mean that this patch changes its intended behavior in non-equivalent manner?.. (I.e., posix_fallocate not zeroing the initial bytes.) No, because it is used from qb_rb_open in rungbuffer.c; there the flags are set simultanously:

	if (flags & QB_RB_FLAG_CREATE) {
		file_flags |= O_CREAT | O_TRUNC;
	}

So, the zeroing used to happen when O_CREAT is set, and O_TRUNC is set then always, too. So, first, the file is truncated to zero length, and only then extended by the new blocks.

The only other uses of this function are from ipc_us.c, and there, too, O_CREAT (when zeroing happens) and O_TRUNC got always together:

	fd_hdr = qb_sys_mmap_file_open(path, r->request,
				       SHM_CONTROL_SIZE, O_RDWR);

or

	fd_hdr = qb_sys_mmap_file_open(path, r->request,
				       SHM_CONTROL_SIZE,
				       O_CREAT | O_TRUNC | O_RDWR);

[imz]

Yes, it looks more safe for the code after posix_fallocate to not invoke any write (as it was already done in the previous revision of the patch) and to not invoke even ftruncate (new in this revision) in order not to get the allocated blocks deallocated by the OS (by creating or "smartly" detecting holes).

[chrissie-c]

retest this please

[chrissie-c]

Merged. Thanks very much!