filemap_nopage is broken!!

While looking at what is needs to happen to get large file support working on linux-intel, I discovered an interesting twist with generic_file_mmap.

mmap being the most interesting case to port, because the generic interface allows for non aligned mappings. Making it the most intersting case to handle.

generic_file_mmap allows filesystem block aligned mappings if the mapping is private. The way it implements this, after I finally tracked it is broken.

For private mappings the same filemap_nopage function that is used for shared mappings is used. The filemap_nopage function alwasy make sure it's pages are in the page cache before it uses them.

For a private mapping (not page aligned) this results in a non-aligned page to be created in the page cache.

Now if the following sequence of actions occure.
a) A page is mapped privately with poor alignment.
b) That part of the file is written again.
c) The page is again mapped privately with poor alignment.

When the page cache page is not scavenged between a and c, the same data is read, despite the fact it has changed on disk, and in the aligned page cache page!

That is broken behavior.

Does anyone know where it comes from?

Eric

Hi,

I don't think this is necessarily a problem. The kernel simply does not guarantee full correspondance semantics between filesystem updates and the page cache for non-aligned pages, but then again, it is not required to --- it is not even required to support such mmaps, so I can live with an undefined behaviour in this case!

--Stephen

ST> I don't think this is necessarily a problem. The kernel simply does not ST> guarantee full correspondance semantics between filesystem updates and ST> the page cache for non-aligned pages, but then again, it is not required ST> to --- it is not even required to support such mmaps, so I can live with ST> an undefined behaviour in this case!

Ah, but suppose we have a mythological a.out programmer. This programmer could run a program, doesn't like the result, compiles a new version which overwrites the old, and attempts to execute the new program. And executes the old!

There may be a lock in there that I haven't spotted, and likely there will be a truncation when the file is overwritten which would flush the page cache but it is possible there isn't.

As the kernel internally uses these mappings for a.out executables this is an undefined case which propogates. It's undefined which a.out program executes :( That part is much harder to live with.

I guess what is I find most objectionable is
a) There is no big fat warning anywhere.
b) The current implementation will pass simple tests so it will look like it works, and then fail at strange weird unpredictable times.

I doubt it will be anything like a show stopper for 2.2 but if this code get's touched it should be fixed to do something consistent.

Eric

Hi,

ST> I don't think this is necessarily a problem. The kernel simply does not ST> guarantee full correspondance semantics between filesystem updates and ST> the page cache for non-aligned pages, but then again, it is not required ST> to --- it is not even required to support such mmaps, so I can live with ST> an undefined behaviour in this case!

There is. truncate_inode_pages() will invalidate all of the mappings when a file is either truncated or deleted. Any overwrite of the file will do the right thing.

I don't think there's any problem.

--Stephen