Bug 64816 - [nfs] [patch] mmap and/or ftruncate does not work correctly on nfs mounted file systems
Summary: [nfs] [patch] mmap and/or ftruncate does not work correctly on nfs mounted fi...
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-bugs mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-03-27 19:00 UTC by Patrick Mackinlay
Modified: 2018-01-03 05:16 UTC (History)
0 users

See Also:


Attachments
patchnfs_getpages.txt (2.99 KB, text/plain)
2004-04-20 01:05 UTC, Peter Edwards
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Mackinlay 2004-03-27 19:00:31 UTC
When using a file on an nfs mounted region, that has its last bytes memory mapped with mmap and then using ftruncate to increase the size of the file, the ftruncate call will result in whatever changes you have made via the mmaped area not being synced to disk (unless you use an msync or mmunmp before the ftruncate call).
The c progam below demonstrates the problem. The files tested on where on a linux 2.4 nfs server and the test program worked fine on linux 2.4 nfs clients.

#include <unistd.h>
#include <stdio.h>
#include <string.h>
#include <errno.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <sys/mman.h>

void error(char *msg)
 {
 fprintf(stderr, "Error: %s\nSystem error %d: %s\n", msg, errno, strerror(errno));
 exit(-1);
 }

#define SZ 1024 // Less than page size

int main(int argn, char *argv[])
 {
 int fd;
 char buffer[SZ];
 char *map;

 if (argn!=2)
  {
  fprintf(stderr, "Usage:\n %s [filename]\n", argv[0]);
  exit(-1);
  }

 memset(buffer, 0, SZ);

 fd=open(argv[1], O_RDWR|O_CREAT|O_TRUNC, S_IRUSR|S_IWUSR);
 if (fd==-1) error("Could not create file");

 if (write(fd, buffer, SZ)!=SZ) error("Could not write buffer");

 map=mmap(NULL, SZ, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
 if (map==MAP_FAILED) error("Map failed");
 map[SZ-1]=1;

 if (ftruncate(fd, SZ+1)!=0) error("Could not truncate file");
 
 if (map[SZ-1]==1)
  printf("Test passed\n");
 else
  printf("Test failed\n");

 exit(0);
 }
Comment 1 Peter Edwards freebsd_committer 2004-04-20 01:05:13 UTC
I'm pretty sure I know what's going on here.

There's an optimisation in nfs_getpages() that returns partially valid 
pages on the assumption that the partially _invalid_ parts are only 
those beyond the end of the file. (In the test program, for example, 
assuming the file doesn't exist initially, this happens when the code 
hits this line:

 > map[SZ-1]=1;

The first 1K of the file is valid, but the rest isn't. (because it 
doesn't exist.)

However, once a page is mapped, and dragged in for a fault, if requested 
page in nfs_getpages() isn't totally valid on return, the invalid parts 
are zeroed and marked as valid by vm_fault().

Later, at the ftruncate(), we call nfs_flush(). While writing out the 
dirty data, it gets marked as no longer "valid" (vfs_busy_pages(), 
called eventually from bwrite()). However, the end of the page doesn't 
go through the bwrite() interface (because it's not present in the 
file), so it's not caught by this.

For our case, by the time we finish ftruncate(), we now have the first 
two DEV_BSIZE chunks of the first page marked invalid and not clean, and 
the remaining 6 marked as valid and clean (assuming intel architecture). 
This is the exact opposite of the assumption that the optimisation 
makes: that the only clean parts happen to be the "real" parts of the file.

The attached patch addresses the problem, and it's run through a 
buildworld over NFS. It simply ensures that a partially valid page meets 
the previously assumed criteria.

I'm awaiting a review from a somewhat more experienced hacker that I've 
been pestering before committing this, but in the meantime, I thought 
I'd share the analysis and patch. Any feedback appreciated.
Comment 2 Peter Edwards freebsd_committer 2004-04-20 01:10:13 UTC
State Changed
From-To: open->analyzed

Analyzed, and patch for consideration. 


Comment 3 Peter Edwards freebsd_committer 2004-04-20 01:10:13 UTC
Responsible Changed
From-To: freebsd-bugs->peadar

I <heart> NFS problems.
Comment 4 Rong-En Fan 2006-03-11 02:32:52 UTC
Hi all,

When browsing NFS related PR, I saw this one. The test program
still fails on 6.0-RELEASE. The patch posted by peadar@ solves
this and makes test program success. 

How's the status of this pr ?

Cheers,
Rong-En Fan
Comment 5 peadar.edwards 2006-03-12 16:16:57 UTC
On 3/11/06, Rong-En Fan <rafan@infor.org> wrote:
> Hi all,
>
> When browsing NFS related PR, I saw this one. The test program
> still fails on 6.0-RELEASE. The patch posted by peadar@ solves
> this and makes test program success.
>
> How's the status of this pr ?
>
> Cheers,
> Rong-En Fan
>
Ah: I'd forgotten about this:
There are unfortunately some problems with the patch: Matt Dillon was
very helpful describing the problem, but before I had a complete
understanding of what he was saying, I got distracted by real life.
I'll refresh my knowledge on the issue and update the PR this week
sometime.
Comment 6 Mark Linimon freebsd_committer freebsd_triage 2007-07-31 15:50:58 UTC
Responsible Changed
From-To: peadar->freebsd-bugs

Committer is away from FreeBSD work right now, so assign this back to 
the general pool, while noting that it contains a patch.
Comment 7 Eitan Adler freebsd_committer freebsd_triage 2012-07-01 16:59:43 UTC
State Changed
From-To: analyzed->open

unowned PRs should not be in analyzed state
Comment 8 Eitan Adler freebsd_committer freebsd_triage 2017-12-31 07:58:30 UTC
For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped