llfio::map(section, reserve) doesn't work on windows

[patstew]

The documentation suggests that the second argument allows you to reserve memory. I was expecting to be able to use a larger value than the section size, so I could later grow the section without changing the address. However, on windows you get error 0xC000001F because you can't create a mapping larger than the section, apparently. mapped_file_handle seems to manage to do it though.

[ned14]

Windows doesn't allow resizing of a section with open maps.

You might note that mapped_file_handle drops all maps if the section is to be resized, then puts them back up after. You'll need to do the same if you're doing maps by hand.

[patstew]

So there's a window where there's nothing mapped if you do the following?

auto f = llfio::mapped_temp_inode(1 << 30).value(); // Reserve a lot of address space
f.truncate(1 << 20); // Allocate 1MB, accessable at f.address();
f.truncate(2 << 20); // Increase to 2MB, the first 1MB is continuously accessable at f.address() (?);

From looking at the code I thought that it only unmapped things if you reduce the size of the section. To extend it it seems to use NtExtendSection which resizes the section and the mapping(?).

[ned14]

Yes, you're right we only tear down the maps if shrinking: https://github.com/ned14/llfio/blob/develop/include/llfio/v2.0/detail/impl/windows/mapped_file_handle.ipp#L146

Sorry my memory is fading as LLFIO generally just works and I don't need to care about implementation any more.

Looking at the source code comments, maps cannot exceed section size, but section size can exceed file size, if the section was created with the right flags.

There is another use case here which might be confusing. Section handles can be backed by an explicit file, or by the swap file. Maps behave differently depending on which, and indeed which flags the file was opened with.

When the documentation says "Create a memory mapped view of a backing storage, optionally reserving additional address space for later growth." and you're supplying a section handle configured by you to use, you're kinda into "I know what I'm doing" territory. It's on you to configure the section handle to be appropriately configured.

The other reason I left this open to success or fail is Microsoft originally promised me they'd fix section handles being so weird to work with in a future kernel release, and to date they have not.

What I probably could do is improve the documentation for that specific function to clarify that you cannot reserve more than the section handle's size?

[ned14]

Hmm. I read here in my own docs:

  /*! \brief Create a memory section backed by a file.
  \param backing The handle to use as backing storage.
  \param bytes The initial size of this section, which cannot be larger than any backing file. Zero means to use `backing.maximum_extent()`.

  This convenience overload create a writable section if the backing file is writable, otherwise a read-only section.

  \errors Any of the values POSIX dup(), open() or NtCreateSection() can return.
  */

Which suggests that section handles cannot exceed the length of their backing file either.

Reading the mapped_file_handle.ipp source code for windows, to achieve address space reservation your map needs to be nocommit and everything needs to be writable. Have you tried that?

[patstew]

Yea, I'm fine if it's just a documentation issue. Whatever mapped_file_handle does when you create it with a reservation then truncate to successively larger sizes seems to work fine for my usecase, so I'm happy with that. My initial try of llfio::map(llfio::section(initial_size).value(), reserved_size, 0, llfio::section_handle::flag::nocommit) failed, though I expected it to work based on the documentation hence the bug report.

you're kinda into "I know what I'm doing" territory

I, on the other hand was hoping to delegate understanding all this platform specific wierdness to you :)

Thanks.

[ned14]

Out of interest, is your backing file zero sized? Linux and Windows won't map a zero sized file (BSD is fine with it).

I, on the other hand was hoping to delegate understanding all this platform specific wierdness to you :)

Yeah, there's a tension there all right. The most common case of a straight mapped file handle with an ample address space reservation for efficiency should "just work", except where it doesn't, but the API design should encourage the avoidance of all those places where it doesn't so 99.9% of users won't ever see problems.

But there are also use cases where you've got some quite customised file mapping going on. Generally speaking, if you get it working on Windows it'll "just work" on POSIX, so I tried to leave open the door for such customised file mapping use cases. TBH on 64 bit there is rarely a need to customise mappings much, it's a big need on 32 bit but not on 64 bit.

There is also a bit of legacy design cruft in there. Originally mapped file handle couldn't do offsets nor partial maps, so the ability to get into map handle and section handle was important. Now that mapped file handle can do offsets and partial maps, a lot of the need for map handle and section handle have gone away. And indeed the proposal up before WG21 just has mapped file handle now.

[patstew]

I'm not using a specific backing file, just a temp_inode. The library seems to handle making a zero length file and only mapping it once it's truncated to some size correctly.

The actual difference seems to be map_handle.ipp:752:

  OUTCOME_TRY(win32_map_flags(nativeh, allocation, prot, commitsize, section.backing() != nullptr, ret.value()._flag));

where section.backing() != nullptr bascially says that anonymous sections can't be reserved. If I just change that to true it seems to work ok including resizing from a brief test. Perhaps it should be section.backing() != nullptr || section._anonymous.is_vaild() or _backing should point to _anonymous or something?

auto sec = llfio::section(1 << 20);
auto map = llfio::map(sec.value(), 1 << 30, 0, llfio::section_handle::flag::readwrite | llfio::section_handle::flag::nocommit);
*(uint64_t*)map.value().address() = 99;
auto tr = sec.value().truncate(2 << 20);
assert(*(uint64_t*)map.value().address() == 99);
*(uint64_t*)(map.value().address() + (1 << 20)) = 99;
assert(*(uint64_t*)(map.value().address() + (1 << 20)) == 99);

[ned14]

That does seem to be a bug then. Thanks for reporting it!