Skip to content
This repository has been archived by the owner on Nov 8, 2019. It is now read-only.

posix_fallocate is not signal-safe #168

Closed
tomaszkapela opened this issue Apr 20, 2016 · 11 comments
Closed

posix_fallocate is not signal-safe #168

tomaszkapela opened this issue Apr 20, 2016 · 11 comments

Comments

@tomaszkapela
Copy link

A call to posix_fallocate can be interrupted by a signal and then the whole operation is cancelled and errno is set to EINTR. If an application using NVML has a lot of signals flying around, the file allocation might be impossible. This issue should be addressed in one way or another.

@krzycz
Copy link

krzycz commented Apr 20, 2016

Seems like similar problem may occur in case of other system calls (i.e. read).

@marcinslusarz
Copy link
Contributor

Yeah, I looked briefly at the syscalls we use and these can be interrupted: close, flock, dup, read.

@krzycz krzycz added this to the 1.2 milestone May 11, 2016
@vitalyvch
Copy link

By the fact there, in NVML, are much many syscalls which are sensitive to signals.

@krzycz krzycz modified the milestones: 1.3, 1.2 Nov 22, 2016
@krzycz krzycz modified the milestones: 1.4, 1.3 May 17, 2017
@GBuella
Copy link

GBuella commented Oct 17, 2017

I looked at this, and I see os_posix_fallocate used in only three places. In all three instances, the error code seems to be propagated to the user in errno. Some pmem* functions can fail with EINTR Should we add this to the documentation?
If the application using NVML uses signals for some communication, it needs to handle its own read ,dup etc. calls anyways, so it let it just handle these interruptions the same way.

@marcinslusarz
Copy link
Contributor

The problem is that restarting pmemobj_create does not help. Every time we do it posix_fallocate starts from the very beginning, which means that it might never finish if application raises signals periodically (like SIGALRM).

@GBuella
Copy link

GBuella commented Oct 18, 2017

Yes, the same is true for whatever syscall that application does. If the application itself calls fallocate on some file, it will need to deal with EINTR.
And the application perhaps doesn't want NVML to continue allocating at all, perhaps the alarm signal is used to communicate to the application, that something other than opening the pool should be done. It might get alarm signals over and over again to signal "don't bother with pmem pool, do this other urgent thing instead".
On the other hand, if the application wants to make sure syscalls don't get interrupted a lot in some code section, it can just stop communicating with signals for that time, or just temporarily block those signals using thread specific signal mask.

@GBuella
Copy link

GBuella commented Oct 18, 2017

In short: you write a library, you don't control the whole application, you didn't raise a signal, you don't know why there was a signal.

@vitalyvch
Copy link

@marcinslusarz , You can play with block/unblock to prevent it.

@plebioda plebioda modified the milestones: 1.4, 1.5 Jan 29, 2018
@marcinslusarz
Copy link
Contributor

We should not block signals, because that may interfere with the way application handles signals.
I think we should have a loop around posix_fallocate and iterate over small enough blocks for one syscall to complete.
Something like:

for (off = 0; off < size; off += blksize)
  while (posix_fallocate(fd, off, blksize) == EINTR)
      ;

@vitalyvch
Copy link

vitalyvch commented Jun 2, 2018

We should not block signals, because that may interfere with the way application handles signals.

Marcin, your words really disappoint me :-(

PS: in your way you should take a look at posix_fallocate() sources and forget about posix_fallocate() at all. Then you can combine syscall(NR_posix_fallocate, fd, 0, size) with something like you have written above.

@marcinslusarz
Copy link
Contributor

Fixed in PMDK 1.6

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants