> Date: Tue, 26 Apr 2016 15:58:47 +0300 > From: Ian Ray <[email protected]> > > On Mon, Apr 25, 2016 at 04:49:07PM +0300, Pekka Paalanen wrote: > > On Mon, 25 Apr 2016 15:47:09 +0300 > > Ian Ray <[email protected]> wrote: > > > > > On a slow system that is configured with SMART_SCHEDULE_POSSIBLE, large > > > posix_fallocate() requests may be interrupted by the SmartScheduleTimer > > > (SIGALRM) continuously. Fallback to ftruncate if posix_fallocate fails. > > > > > > Signed-off-by: Ian Ray <[email protected]> > > > --- > > > hw/xwayland/xwayland-shm.c | 9 ++++++--- > > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > > > diff --git a/hw/xwayland/xwayland-shm.c b/hw/xwayland/xwayland-shm.c > > > index e8545b3..342b723 100644 > > > --- a/hw/xwayland/xwayland-shm.c > > > +++ b/hw/xwayland/xwayland-shm.c > > > @@ -142,9 +142,12 @@ os_create_anonymous_file(off_t size) > > > #ifdef HAVE_POSIX_FALLOCATE > > > ret = posix_fallocate(fd, 0, size); > > > if (ret != 0) { > > > - close(fd); > > > - errno = ret; > > > - return -1; > > > + /* Fallback to ftruncate in case of failure. */ > > > + ret = ftruncate(fd, size); > > > + if (ret < 0) { > > > + close(fd); > > > + return -1; > > > + } > > > } > > > #else > > > ret = ftruncate(fd, size); > > > > Hi Ian, > > > > I indeed think this is a bit harsh. The first EINTR may happen on any > > system, so the very least we should try twice before giving up. > > > > I'd also like to see some comments on whether fallocate making no > > progress when repeatedly restarted is normal or not. Maybe that should > > be asked on a kernel list? > > > > > > Thanks, > > pq > > Hi Pekka, > > Referring to http://lxr.free-electrons.com/source/mm/shmem.c#L2235 and > http://lkml.iu.edu/hypermail/linux/kernel/1603.0/03523.html, fallocate > is _not_ expected to make progress when repeatedly restarted.
The crucial bit being that it does an explicit rollback if it gets EINTR. Not very well thought out if you ask me (it is as if an interrupted write call would return EINTR and not make progress), but probably something that is cast into stone and cannot be fixed anymore. That last message you cite is interesting. POSIX states that system calls return EINTR should be automatically restarted for signals that have the SA_RESTART flag set. So that means that the linux kernel should indeed return -ERESTARTSYS here. But since the Xorg smart scheduler code sets SA_RESTART, that means you'd get an infinite loop on the systems where (allegedly) posix_fallocate() never completes in presence of the smart scheduler's repeated SIGALRM. > Taking Yuriy's reply in to account, then there seems to be are a couple > of options: > > 1. try fallocate() a couple of times and in case of EINTR fallback > to ftruncate > > 2. mask SIGALRM while calling fallocate That probably is the right thing to do here. Having the smart scheduler miss a "tick" isn't a disaster, and the other suggestions (looping while EINTR is returned, looping over smaller chunks) would block the request just as well. Or perhaps take a step back and ask yourselves if using posix_fallocate() here is the right thing to do in the first place. SIGBUS really can't be prevented as a malicious client can still ftruncate() the file descriptor. On OpenBSD I implemented a mmap option that prevents SIGBUS on access of the memory even if the request for backing store cannot be fulfilled. But that isn't portable of course. _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
