Bug 225586 - ftruncate+mmap+fsync fails for small maps
Summary: ftruncate+mmap+fsync fails for small maps
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: misc (show other bugs)
Version: 11.1-STABLE
Hardware: amd64 Any
: --- Affects Many People
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-01-31 13:40 UTC by tris_vern
Modified: 2019-01-21 21:57 UTC (History)
5 users (show)

See Also:


Attachments
test on tmp filesystem that can be unmounted (1.95 KB, text/plain)
2018-01-31 13:40 UTC, tris_vern
no flags Details
For partially dirty page, only clear dirty bits for completely invalid device blocks. (530 bytes, patch)
2018-02-01 15:25 UTC, Konstantin Belousov
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description tris_vern 2018-01-31 13:40:48 UTC
Created attachment 190220 [details]
test on tmp filesystem that can be unmounted

When doing an ftruncate+mmap+fsync to a size less than 512 bytes the data never seems to be sync'd properly to disk.

This work fine on the latest 11.1-RELEASE but not the latest 11.1-STABLE. It is also ok if the ftruncate is to the existing file size.

While the mapped changes are visible in the file before an umount, after even a failed umount, the changes are no longer present.


(non-error checked) C code below.

After running the binary, check the contents of the file (eg hd) and it will be all 0xFF. Then, umount the filesystem you are on*. Then check the contents of the file again. It will now be 0x01.

Running extra sync's or fsync's from the command line before the umount doesn't seem to help.

* The umount should fail but still shows the problem. However note it doesn't seem to work with the error given for the root filesystem "/".

I've also attached a script that creates a filesystem to umount

---

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <fcntl.h>
#include <string.h>
#include <sys/mman.h>

int
main (int argc, char *argv[])
{
   int fd;
   size_t i, size1, size2;
   char *data;
   char *filename;
   char pattern = 0x01;

   if (argc < 2) {
      fprintf(stderr, "Usage: %s filename size1 size2\n", argv[0]);
      exit(1);
   }

   filename = argv[1];
   size1 = atoi(argv[2]);
   size2 = atoi(argv[3]);

   fd = open(filename, O_RDWR | O_TRUNC | O_CREAT, 0644);
   for (i = 0; i < size1; i++)
      write(fd, &pattern, 1);
   close(fd);

   fd = open(filename, O_RDWR, 0644);
   ftruncate(fd, size2);
   data = mmap(NULL, size2, PROT_READ|PROT_WRITE,MAP_SHARED,fd,0);
   memset(data, 0xFF, size2);
   fsync(fd);

   munmap(data, size2);
   close(fd);
}
Comment 1 Peter Holm freebsd_committer freebsd_triage 2018-02-01 09:29:07 UTC
Problem reproduced on 12.0-CURRENT #0 r328587.
Comment 2 Konstantin Belousov freebsd_committer freebsd_triage 2018-02-01 15:25:02 UTC
Created attachment 190246 [details]
For partially dirty page, only clear dirty bits for completely invalid device blocks.
Comment 3 Konstantin Belousov freebsd_committer freebsd_triage 2018-02-01 17:04:15 UTC
(In reply to Konstantin Belousov from comment #2)
THis is a simple change, but if you can run all mmap(2) tests, I will believe into it correctness much easier.
Comment 4 Peter Holm freebsd_committer freebsd_triage 2018-02-02 08:07:11 UTC
(In reply to Konstantin Belousov from comment #3)
The patch fixed the issue for me. I ran all of the stress2 mmap() test cases and a buildworld. No problems seen.
Comment 5 commit-hook freebsd_committer freebsd_triage 2018-02-02 11:56:33 UTC
A commit references this bug:

Author: kib
Date: Fri Feb  2 11:56:30 UTC 2018
New revision: 328773
URL: https://svnweb.freebsd.org/changeset/base/328773

Log:
  On pageout, in vnode generic pager, for partially dirty page, only
  clear dirty bits for completely invalid blocks.

  Otherwise we might not write out the last chunk that is shorter than
  512 bytes, if the file end is not aligned on disk block boundary.
  This become important after the r324794.

  PR:	225586
  Reported by:	tris_vern@hotmail.com
  Tested by:	pho
  Sponsored by:	The FreeBSD Foundation
  MFC after:	3 days

Changes:
  head/sys/vm/vnode_pager.c
Comment 6 tris_vern 2018-02-02 12:13:07 UTC
(In reply to Peter Holm from comment #4)
Thanks for the fix and the testing. It did fix the problem for me as well after rebuilding the kernel on 11-STABLE. I am unfamiliar with the test suite however.
Comment 7 commit-hook freebsd_committer freebsd_triage 2018-02-05 10:30:03 UTC
A commit references this bug:

Author: kib
Date: Mon Feb  5 10:29:58 UTC 2018
New revision: 328879
URL: https://svnweb.freebsd.org/changeset/base/328879

Log:
  MFC r328773:
  On pageout, in vnode generic pager, for partially dirty page, only
  clear dirty bits for completely invalid blocks.

  PR:	225586

Changes:
_U  stable/11/
  stable/11/sys/vm/vnode_pager.c
Comment 8 Oleksandr Tymoshenko freebsd_committer freebsd_triage 2019-01-21 21:57:47 UTC
There is a commit referencing this PR, but it's still not closed and has been inactive for some time. Closing the PR as fixed but feel free to re-open it if the issue hasn't been completely resolved.

Thanks