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
