On Thu, Aug 31, 2017 at 9:49 AM, Linus Torvalds <torva...@linux-foundation.org> wrote: > This patch strikes me as insane. > > On Wed, Aug 30, 2017 at 4:08 PM, Dan Williams <dan.j.willi...@intel.com> > wrote: >> switch (flags & MAP_TYPE) { >> + case (MAP_SHARED|MAP_VALIDATE): >> + /* TODO: new map flags */ >> + return -EINVAL; >> case MAP_SHARED: >> if ((prot&PROT_WRITE) && !(file->f_mode&FMODE_WRITE)) >> return -EACCES; > > So you "add" support for MAP_SHARED|MAP_VALIDATE, but then error out on it. > > And you don't add support for MAP_PRIVATE|MAP_VALIDATE at all, so that > errors out too. > > Which makes me think that you actually only want MAP:_VALIDATE support > for shared mappings. > > Which in turn means that all your blathering about how this cannot > work on HP-UX is just complete garbage, because you might as well just > realize that MAP_TYPE isn't a mask of _bitmasks_, it's a mask of > values. > > So just make MAP_VALIDATE be 0x3. Which works for everybody. Make it > mean the same as MAP_SHARED with flag validation. End of story. > > None of these stupid games that are complete and utter garbage, and > make people think that the MAP_TYPE bits are somehow a bitmask. They > aren't. The bitmasks are all the *other* bits that aren't in > MAP_TYTPE. > > Yes, yes, I see why you *think* you want a bitmap. You think you want > a bitmap because you want to make MAP_VALIDATE be part of MAP_SYNC > etc, so that people can do > > ret = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED | > MAP_SYNC, fd, 0); > > and "know" that MAP_SYNC actually takes. > > And I'm saying that whole wish is bogus. You're fundamentally > depending on special semantics, just make it explicit. It's already > not portable, so don't try to make it so. > > Rename that MAP_VALIDATE as MAP_SHARED_VALIDATE, make it have a valud > of 0x3, and make people do > > ret = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED_VALIDATE > | MAP_SYNC, fd, 0);
Yeah, we originally had MAP_VALIDATE defined as (MAP_SHARED|MAP_PRIVATE), but Kirill was concerned that would make something like MAP_PRIVATE|MAP_SYNC silently provide MAP_SHARED semantics. MAP_SHARED_VALIDATE solves that problem. > and then the kernel side is easier too (none of that random garbage > playing games with looking at the "MAP_VALIDATE bit", but just another > case statement in that map type thing. > > Boom. Done. Looks good to me. _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm