Re: [PATCH v1 08/14] mm/mshare: Add basic page table sharing using mshare

[Khalid Aziz]

On 5/30/22 05:11, Barry Song wrote:
> On Tue, Apr 12, 2022 at 4:07 AM Khalid Aziz <khalid.aziz@xxxxxxxxxx> wrote:
> > @@ -193,6 +226,8 @@ SYSCALL_DEFINE5(mshare, const char __user *, name, unsigned long, addr,
> >          if (IS_ERR(fname))
> >                  goto err_out;
> >
> > +       end = addr + len;
> > +
> >          /*
> >           * Does this mshare entry exist already? If it does, calling
> >           * mshare with O_EXCL|O_CREAT is an error
> > @@ -205,49 +240,165 @@ SYSCALL_DEFINE5(mshare, const char __user *, name, unsigned long, addr,
> >          inode_lock(d_inode(msharefs_sb->s_root));
> >          dentry = d_lookup(msharefs_sb->s_root, &namestr);
> >          if (dentry && (oflag & (O_EXCL|O_CREAT))) {
> > +               inode = d_inode(dentry);
> >                  err = -EEXIST;
> >                  dput(dentry);
> >                  goto err_unlock_inode;
> >          }
> >
> >          if (dentry) {
> > +               unsigned long mapaddr, prot = PROT_NONE;
> > +
> >                  inode = d_inode(dentry);
> >                  if (inode == NULL) {
> > +                       mmap_write_unlock(current->mm);
> >                          err = -EINVAL;
> >                          goto err_out;
> >                  }
> >                  info = inode->i_private;
> > -               refcount_inc(&info->refcnt);
> >                  dput(dentry);
> > +
> > +               /*
> > +                * Map in the address range as anonymous mappings
> > +                */
> > +               oflag &= (O_RDONLY | O_WRONLY | O_RDWR);
> > +               if (oflag & O_RDONLY)
> > +                       prot |= PROT_READ;
> > +               else if (oflag & O_WRONLY)
> > +                       prot |= PROT_WRITE;
> > +               else if (oflag & O_RDWR)
> > +                       prot |= (PROT_READ | PROT_WRITE);
> > +               mapaddr = vm_mmap(NULL, addr, len, prot,
> > +                               MAP_FIXED | MAP_SHARED | MAP_ANONYMOUS, 0);
>
>  From the perspective of hardware, do we have to use MAP_FIXED to make
> sure those processes sharing PTE
> use the same virtual address for the shared area? or actually we don't
> necessarily need it? as long as the
> upper level pgtable entries point to the same lower level pgtable?

Hi Barry,

Sorry, I didn't mean to ignore this. I was out of commission for the last few weeks.

All processes sharing an mshare region must use the same virtual address otherwise page table entry for those processes 
won't be identical and hence can not be shared. Upper bits of virtual address provide index into various level 
directories. It may be possible to manipulate the various page directories to allow for different virtual addresses 
across processes and get hardware page table walk to work correctly, but that would be complex and potentially error prone.

Thanks,
Khalid