Hello. 03.05.2012 19:24, Bart Oldeman wrote: > On 23 April 2012 17:48, Stas Sergeev wrote: >> What are you optimizing for, why there is even a need to >> make the mapping.c functions to call the functions with the >> similar names? > (sorry for the late reply, I checked things a bit more thoroughly now) > > There is of course no need to call functions with similar names, > although it is a bit confusing that mmap_mapping() sometimes doesn't > call mmap(), it should at least to the caller behave like mmap. Yes, sure. Currently it does exactly that: even though mmap to /dev/mem is not possible when the root is dropped and fd is closed, it still behaves to the caller like mmap(), but internally it does mremap.
> Initially I just saw a very janitorial cleanup as combining a sequence: > f(x); > g(y); > into > h(x,y); > where h(x,y) essentially calls f(x) and g(y), optimizing out redundant > code and syscalls, but retaining the possibility to call f(x) and > g(y). So I really don't call for a complete revert. OK, but are you allright with the fact that the f(x) is to be called only before root drop, and yet to merge them? IMHO, the fewer we do being root, the better, so the alternative is to move g(y) to the stage where lowram it being initialized. (for those kmem areas that maps to lowram) Essentially, if you merge them, you'll get a 2 functions inside one, depenging on a "first_call" internal variable or the like. I think it is a very dirty approach in general, even if it saves 1-2 lines of code to the caller. > I think scratch mapping should stay in mmap_mapping, it really is > mmap. If you don't see it fits alloc_mapping(), then how about just mmap_mapping_scratch()? I am advocating to handle it differently because it doesn't even call to the mapping_driver, so it really is different from whoever does. It is almost possible to just drop it entirely and allow the caller to do mmap() directly. > Then alloc_mapping (without kmem), to match valloc() or malloc() > should just have one argument apart from cap, that is size. That might be fine if you already killed MAPPING_INIT_LOWRAM. > Now for the hardware RAM: > >> The point was exactly that: if we need something >> like "DPMI map hardware ram", then we need to allow the user >> to put the allowed ranges in the config, and then we can >> pre-allocate them while we are still root. > That functionality has been there since October 2004 (git commit > 879e51 during the 1.3.2 release cycle), and I certainly wouldn't want > to kill it. But no actual mapping takes place there and then (inside > DPMI function 0x800), all it does is to return the DOS address of the > pre-mapped hardware RAM (from alloc_mapping: mmap_mapping does no > actual mapping for these addresses, but returns the same thing as the > alloc_mapping call just before that, and mprotect()s), since that is > sufficient This is sufficient because MAP_32BIT is used, while otherwise it would not be sufficient. Notice that for the kmems that map to lowram, you don't need MAP_32BIT, because you do mremap anyway, but right now you have to use MAP_32BIT in either case. How about removing MAP_32BIT instead? :) Well, what I am trying to point out, is that the current code *and API* is very generic and flexible. If you do it your way, it will still work, but it will start depending on many obscure things. On the other hand, we can _remove_ the obscure things first, then just clean up the current code, as your proposals will not then fit. > Yes I know I agreed on your changes many years ago, just because it > effectively solved a problem (dealing with /dev/mem mmaps after the > priv drop), but it's always possible to clean up, it's not a contract > we signed. I am not against the cleanups: they have to be done! Actually, I can't beleive I wrote such a messy code (but maybe it became so later without me, I don't know). But we look at the cleanups from the different sides: I want to clean up the implementation in mapping.c, which is really a mess right now, and you want to clean up the callers, while for the callers it is probably only a cost of 2 function calls instead of one, which is not a big deal IMHO. Can we do that in a separate steps? Clean up mapping.c first, and then see if we really need to change the API too much or not. There is certainly no need to do both things together as this way we can overlook the whole picture. I also have the serial code rework now. I'll soon upload it for testing. ------------------------------------------------------------------------------ Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ _______________________________________________ Dosemu-devel mailing list Dosemu-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dosemu-devel
