On 04/19/2018 04:15 PM, Eric Dumazet wrote:
> I am not sure we can keep mmap() API, since we probably need to first lock > the socket, > then grab vm semaphore. > We can keep mmap() nice interface, granted we can add one hook like in following patch. David, do you think such patch would be acceptable by lkml and mm/fs maintainers ? Alternative would be implementing an ioctl() or getsockopt() operation, but it seems less natural... Thanks ! diff --git a/include/linux/fs.h b/include/linux/fs.h index 92efaf1f89775f7b017477617dd983c10e0dc4d2..016c711ac33e226b4285ee5bd688e14661dc0879 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1714,6 +1714,7 @@ struct file_operations { long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long); long (*compat_ioctl) (struct file *, unsigned int, unsigned long); int (*mmap) (struct file *, struct vm_area_struct *); + void (*mmap_hook) (struct file *, bool); unsigned long mmap_supported_flags; int (*open) (struct inode *, struct file *); int (*flush) (struct file *, fl_owner_t id); diff --git a/mm/util.c b/mm/util.c index 1fc4fa7576f762bbbf341f056ca6d0be803a423f..b546c59a6169c4dfa9011c61e86da4d03496aa4d 100644 --- a/mm/util.c +++ b/mm/util.c @@ -350,11 +350,20 @@ unsigned long vm_mmap_pgoff(struct file *file, unsigned long addr, ret = security_mmap_file(file, prot, flag); if (!ret) { - if (down_write_killable(&mm->mmap_sem)) + void (*mmap_hook)(struct file *, bool) = file ? file->f_op->mmap_hook : NULL; + + if (mmap_hook) + mmap_hook(file, true); + if (down_write_killable(&mm->mmap_sem)) { + if (mmap_hook) + mmap_hook(file, false); return -EINTR; + } ret = do_mmap_pgoff(file, addr, len, prot, flag, pgoff, &populate, &uf); up_write(&mm->mmap_sem); + if (mmap_hook) + mmap_hook(file, false); userfaultfd_unmap_complete(mm, &uf); if (populate) mm_populate(ret, populate); diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index 4022073b0aeea9d07af0fa825b640a00512908a3..79b05d6d41643e8c309dfb8bd9597dc8b00fb0e1 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -1756,8 +1756,6 @@ int tcp_mmap(struct file *file, struct socket *sock, /* TODO: Maybe the following is not needed if pages are COW */ vma->vm_flags &= ~VM_MAYWRITE; - lock_sock(sk); - ret = -ENOTCONN; if (sk->sk_state == TCP_LISTEN) goto out; @@ -1833,7 +1831,6 @@ int tcp_mmap(struct file *file, struct socket *sock, ret = 0; out: - release_sock(sk); kvfree(pages_array); return ret; } diff --git a/net/socket.c b/net/socket.c index f10f1d947c78c193b49379b0ec641d81367fb4cf..bcabae3c37d765e5c0548a14fc93c19258972b48 100644 --- a/net/socket.c +++ b/net/socket.c @@ -131,6 +131,16 @@ static ssize_t sock_splice_read(struct file *file, loff_t *ppos, struct pipe_inode_info *pipe, size_t len, unsigned int flags); +static void sock_mmap_hook(struct file *file, bool enter) +{ + struct socket *sock = file->private_data; + struct sock *sk = sock->sk; + + if (enter) + lock_sock(sk); + else + release_sock(sk); +} /* * Socket files have a set of 'special' operations as well as the generic file ones. These don't appear * in the operation structures but are done directly via the socketcall() multiplexor. @@ -147,6 +157,7 @@ static const struct file_operations socket_file_ops = { .compat_ioctl = compat_sock_ioctl, #endif .mmap = sock_mmap, + .mmap_hook = sock_mmap_hook, .release = sock_close, .fasync = sock_fasync, .sendpage = sock_sendpage,