diff mbox series

[04/12] bpf: use __anon_inode_getfd

Message ID 20200508153634.249933-5-hch@lst.de
State New
Headers show
Series [01/12] fd: add a new __anon_inode_getfd helper | expand

Commit Message

Christoph Hellwig May 8, 2020, 3:36 p.m. UTC 
Use __anon_inode_getfd instead of opencoding the logic using
get_unused_fd_flags + anon_inode_getfile.  Also switch the
bpf_link_new_file calling conventions to match __anon_inode_getfd.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/bpf.h  |  2 +-
 kernel/bpf/cgroup.c  |  6 +++---
 kernel/bpf/syscall.c | 31 +++++++++----------------------
 3 files changed, 13 insertions(+), 26 deletions(-)

Comments

Andrii Nakryiko May 8, 2020, 5:32 p.m. UTC | #1 
On Fri, May 8, 2020 at 8:39 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Use __anon_inode_getfd instead of opencoding the logic using
> get_unused_fd_flags + anon_inode_getfile.  Also switch the
> bpf_link_new_file calling conventions to match __anon_inode_getfd.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  include/linux/bpf.h  |  2 +-
>  kernel/bpf/cgroup.c  |  6 +++---
>  kernel/bpf/syscall.c | 31 +++++++++----------------------
>  3 files changed, 13 insertions(+), 26 deletions(-)
>

[...]

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 64783da342020..cb2364e17423c 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2307,23 +2307,10 @@ int bpf_link_new_fd(struct bpf_link *link)
>   * complicated and expensive operations and should be delayed until all the fd
>   * reservation and anon_inode creation succeeds.
>   */

The comment above explains the reason why we do want to split getting
fd, getting file, and installing fd later. I'd like to keep it this
way. Also, this code was refactored in bpf-next by [0] (it still uses
get_unused_fd_flag + anon_inode_getfile + fd_install, by design).

  [0] https://patchwork.ozlabs.org/project/netdev/patch/20200429001614.1544-3-andriin@fb.com/

> -struct file *bpf_link_new_file(struct bpf_link *link, int *reserved_fd)
> +int bpf_link_new_file(struct bpf_link *link, struct file **file)
>  {
> -       struct file *file;
> -       int fd;
> -
> -       fd = get_unused_fd_flags(O_CLOEXEC);
> -       if (fd < 0)
> -               return ERR_PTR(fd);
> -
> -       file = anon_inode_getfile("bpf_link", &bpf_link_fops, link, O_CLOEXEC);
> -       if (IS_ERR(file)) {
> -               put_unused_fd(fd);
> -               return file;
> -       }
> -
> -       *reserved_fd = fd;
> -       return file;
> +       return __anon_inode_getfd("bpf_link", &bpf_link_fops, link, O_CLOEXEC,
> +                       file);
>  }
>

[...]
diff mbox series

Patch

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index fd2b2322412d7..539649fe8b885 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1103,7 +1103,7 @@  void bpf_link_cleanup(struct bpf_link *link, struct file *link_file,
 void bpf_link_inc(struct bpf_link *link);
 void bpf_link_put(struct bpf_link *link);
 int bpf_link_new_fd(struct bpf_link *link);
-struct file *bpf_link_new_file(struct bpf_link *link, int *reserved_fd);
+int bpf_link_new_file(struct bpf_link *link, struct file **link_file);
 struct bpf_link *bpf_link_get_from_fd(u32 ufd);
 
 int bpf_obj_pin_user(u32 ufd, const char __user *pathname);
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index cb305e71e7deb..8605287adcd9e 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -836,10 +836,10 @@  int cgroup_bpf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
 	link->cgroup = cgrp;
 	link->type = attr->link_create.attach_type;
 
-	link_file = bpf_link_new_file(&link->link, &link_fd);
-	if (IS_ERR(link_file)) {
+	link_fd = bpf_link_new_file(&link->link, &link_file);
+	if (link_fd < 0) {
 		kfree(link);
-		err = PTR_ERR(link_file);
+		err = link_fd;
 		goto out_put_cgroup;
 	}
 
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 64783da342020..cb2364e17423c 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2307,23 +2307,10 @@  int bpf_link_new_fd(struct bpf_link *link)
  * complicated and expensive operations and should be delayed until all the fd
  * reservation and anon_inode creation succeeds.
  */
-struct file *bpf_link_new_file(struct bpf_link *link, int *reserved_fd)
+int bpf_link_new_file(struct bpf_link *link, struct file **file)
 {
-	struct file *file;
-	int fd;
-
-	fd = get_unused_fd_flags(O_CLOEXEC);
-	if (fd < 0)
-		return ERR_PTR(fd);
-
-	file = anon_inode_getfile("bpf_link", &bpf_link_fops, link, O_CLOEXEC);
-	if (IS_ERR(file)) {
-		put_unused_fd(fd);
-		return file;
-	}
-
-	*reserved_fd = fd;
-	return file;
+	return __anon_inode_getfd("bpf_link", &bpf_link_fops, link, O_CLOEXEC,
+			file);
 }
 
 struct bpf_link *bpf_link_get_from_fd(u32 ufd)
@@ -2406,10 +2393,10 @@  static int bpf_tracing_prog_attach(struct bpf_prog *prog)
 	}
 	bpf_link_init(&link->link, &bpf_tracing_link_lops, prog);
 
-	link_file = bpf_link_new_file(&link->link, &link_fd);
-	if (IS_ERR(link_file)) {
+	link_fd = bpf_link_new_file(&link->link, &link_file);
+	if (link_fd < 0) {
 		kfree(link);
-		err = PTR_ERR(link_file);
+		err = link_fd;
 		goto out_put_prog;
 	}
 
@@ -2520,10 +2507,10 @@  static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
 	bpf_link_init(&link->link, &bpf_raw_tp_lops, prog);
 	link->btp = btp;
 
-	link_file = bpf_link_new_file(&link->link, &link_fd);
-	if (IS_ERR(link_file)) {
+	link_fd = bpf_link_new_file(&link->link, &link_file);
+	if (link_fd < 0) {
 		kfree(link);
-		err = PTR_ERR(link_file);
+		err = link_fd;
 		goto out_put_btp;
 	}