Guys, I just pushed out a new helper function intended for cleaning up various device driver mmap functions, because they are rather messy, and at least part of the problem was the bad impedance between what a driver author would want to have, and the VM interfaces to map a memory range into user space with mmap.
Some drivers would end up doing extensive checks on the length of the mappings and the page offset within the mapping, while other drivers would end up doing no checks at all. The new helper is in commit b4cbb197c7e7 ("vm: add vm_iomap_memory() helper function"), but I didn't actually commit any *users* of it, because I just have this untested patch-collection for a few random drivers (picked across a few different driver subsystems, just to make it interesting). I did that largely just to check the different use cases, but I don't actually tend to *use* all that many fancy drivers, so I don't have much of a way of testing it. The media layer has a few users of [io_]remap_pfn_range() that look like they could do with some tender loving too, but they don't match this particular pattern of "allow users to map a part of a fixed range of memory". In fact, the media pattern seems to be single-page mappings, which probably should use "vm_insert_page()" instead, but that's a whole separate thing. But I didn't check all the media cases (and there's a lot of remap_pfn_range use outside of media drivers I didn't check either), so there might be code that could use the new helper. Anyway, I'm attaching the *untested* patch to several drivers. Guys, mind taking a look? The point here is to simplify the interface, avoiding bugs, but also: 5 files changed, 21 insertions(+), 87 deletions(-) it needs current -git for the new helper function. NOTE! The driver subsystem .mmap functions seem to almost universally do if (io_remap_pfn_range(..)) return -EAGAIN; return 0; and I didn't make the new helper function do that "turn all remap_pfn_range errors into EAGAIN". My *suspicion* is that this is just really old copy-pasta and makes no sense, but maybe there is some actual reasoning behind EAGAIN vs ENOMEM, for example. EAGAIN is documented to be about file/memory locking, which means that it really doesn't make any sense, but obviously there might be some binary that actally depends on this, so I'm perfectly willing to make the helper do that odd error case, I'd just like to know (and a add a comment) WHY. My personal guess is that nobody actually cares (we return other error codes for other cases, notably EINVAL for various out-of-mapping-range issues), and the whole EAGAIN return value is just a completely historical oddity. (And yes, I know the mtdchar code is actually disabled right now. But that was a good example of a driver that had a bug in this area and that I touched myself not too long ago, and recent stable noise reminded me of it, so I did that one despite it not being active). Linus
patch.diff
Description: Binary data