All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Change quotactl_path() to an fd-based syscall
@ 2021-06-02 15:15 Jan Kara
  2021-06-02 15:15 ` [PATCH 1/2] quota: Change quotactl_path() systcall to an fd-based one Jan Kara
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Jan Kara @ 2021-06-02 15:15 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: Christoph Hellwig, brauner, linux-api, Sascha Hauer, Jan Kara

Hello,

this patch series changes Sasha's quotactl_path() syscall to an fd-based one
quotactl_fd() syscall and enables the syscall again. The fd-based syscall was
chosen over the path based one because there's no real need for the path -
identifying filesystem to operate on by fd is perfectly fine for quotactl and
thus we can avoid the need to specify all the details of path lookup in the
quotactl_path() API (and possibly keep that uptodate with all the developments
in that field).

Patches passed some basic functional testing. Please review.

								Honza

^ permalink raw reply	[flat|nested] 12+ messages in thread
* [PATCH 1/2] quota: Change quotactl_path() systcall to an fd-based one
  2021-06-02 15:15 [PATCH 0/2] Change quotactl_path() to an fd-based syscall Jan Kara
@ 2021-06-02 15:15 ` Jan Kara
  2021-06-03  9:41   ` Sascha Hauer
                     ` (2 more replies)
  2021-06-02 15:15 ` [PATCH 2/2] quota: Wire up quotactl_fd syscall Jan Kara
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 12+ messages in thread
From: Jan Kara @ 2021-06-02 15:15 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: Christoph Hellwig, brauner, linux-api, Sascha Hauer, Jan Kara

Some users have pointed out that path-based syscalls are problematic in
some environments and at least directory fd argument and possibly also
resolve flags are desirable for such syscalls. Rather than
reimplementing all details of pathname lookup and following where it may
eventually evolve, let's go for full file descriptor based syscall
similar to how ioctl(2) works since the beginning. Managing of quotas
isn't performance sensitive so the extra overhead of open does not
matter and we are able to consume O_PATH descriptors as well which makes
open cheap anyway. Also for frequent operations (such as retrieving
usage information for all users) we can reuse single fd and in fact get
even better performance as well as avoiding races with possible remounts
etc.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/quota/quota.c                  | 27 ++++++++++++---------------
 include/linux/syscalls.h          |  4 ++--
 include/uapi/asm-generic/unistd.h |  4 ++--
 kernel/sys_ni.c                   |  2 +-
 4 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/fs/quota/quota.c b/fs/quota/quota.c
index 05e4bd9ab6d6..8450bb6186f4 100644
--- a/fs/quota/quota.c
+++ b/fs/quota/quota.c
@@ -968,31 +968,29 @@ SYSCALL_DEFINE4(quotactl, unsigned int, cmd, const char __user *, special,
 	return ret;
 }
 
-SYSCALL_DEFINE4(quotactl_path, unsigned int, cmd, const char __user *,
-		mountpoint, qid_t, id, void __user *, addr)
+SYSCALL_DEFINE4(quotactl_fd, unsigned int, fd, unsigned int, cmd,
+		qid_t, id, void __user *, addr)
 {
 	struct super_block *sb;
-	struct path mountpath;
 	unsigned int cmds = cmd >> SUBCMDSHIFT;
 	unsigned int type = cmd & SUBCMDMASK;
+	struct fd f = fdget_raw(fd);
 	int ret;
 
-	if (type >= MAXQUOTAS)
-		return -EINVAL;
+	if (!f.file)
+		return -EBADF;
 
-	ret = user_path_at(AT_FDCWD, mountpoint,
-			     LOOKUP_FOLLOW | LOOKUP_AUTOMOUNT, &mountpath);
-	if (ret)
-		return ret;
-
-	sb = mountpath.mnt->mnt_sb;
+	ret = -EINVAL;
+	if (type >= MAXQUOTAS)
+		goto out;
 
 	if (quotactl_cmd_write(cmds)) {
-		ret = mnt_want_write(mountpath.mnt);
+		ret = mnt_want_write(f.file->f_path.mnt);
 		if (ret)
 			goto out;
 	}
 
+	sb = f.file->f_path.mnt->mnt_sb;
 	if (quotactl_cmd_onoff(cmds))
 		down_write(&sb->s_umount);
 	else
@@ -1006,9 +1004,8 @@ SYSCALL_DEFINE4(quotactl_path, unsigned int, cmd, const char __user *,
 		up_read(&sb->s_umount);
 
 	if (quotactl_cmd_write(cmds))
-		mnt_drop_write(mountpath.mnt);
+		mnt_drop_write(f.file->f_path.mnt);
 out:
-	path_put(&mountpath);
-
+	fdput(f);
 	return ret;
 }
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 050511e8f1f8..586128d5c3b8 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -485,8 +485,8 @@ asmlinkage long sys_pipe2(int __user *fildes, int flags);
 /* fs/quota.c */
 asmlinkage long sys_quotactl(unsigned int cmd, const char __user *special,
 				qid_t id, void __user *addr);
-asmlinkage long sys_quotactl_path(unsigned int cmd, const char __user *mountpoint,
-				  qid_t id, void __user *addr);
+asmlinkage long sys_quotactl_fd(unsigned int fd, unsigned int cmd, qid_t id,
+				void __user *addr);
 
 /* fs/readdir.c */
 asmlinkage long sys_getdents64(unsigned int fd,
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 6de5a7fc066b..f211961ce1da 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -863,8 +863,8 @@ __SYSCALL(__NR_process_madvise, sys_process_madvise)
 __SC_COMP(__NR_epoll_pwait2, sys_epoll_pwait2, compat_sys_epoll_pwait2)
 #define __NR_mount_setattr 442
 __SYSCALL(__NR_mount_setattr, sys_mount_setattr)
-#define __NR_quotactl_path 443
-__SYSCALL(__NR_quotactl_path, sys_quotactl_path)
+#define __NR_quotactl_fd 443
+__SYSCALL(__NR_quotactl_fd, sys_quotactl_fd)
 
 #define __NR_landlock_create_ruleset 444
 __SYSCALL(__NR_landlock_create_ruleset, sys_landlock_create_ruleset)
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index 0ea8128468c3..dad4d994641e 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -99,7 +99,7 @@ COND_SYSCALL(flock);
 
 /* fs/quota.c */
 COND_SYSCALL(quotactl);
-COND_SYSCALL(quotactl_path);
+COND_SYSCALL(quotactl_fd);
 
 /* fs/readdir.c */
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH 2/2] quota: Wire up quotactl_fd syscall
  2021-06-02 15:15 [PATCH 0/2] Change quotactl_path() to an fd-based syscall Jan Kara
  2021-06-02 15:15 ` [PATCH 1/2] quota: Change quotactl_path() systcall to an fd-based one Jan Kara
@ 2021-06-02 15:15 ` Jan Kara
  2021-06-02 15:19 ` [PATCH 0/2] Change quotactl_path() to an fd-based syscall Jan Kara
  2021-06-07  6:14 ` Christoph Hellwig
  3 siblings, 0 replies; 12+ messages in thread
From: Jan Kara @ 2021-06-02 15:15 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: Christoph Hellwig, brauner, linux-api, Sascha Hauer, Jan Kara

Wire up the quotactl_fd syscall.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 arch/alpha/kernel/syscalls/syscall.tbl      | 2 +-
 arch/arm/tools/syscall.tbl                  | 2 +-
 arch/arm64/include/asm/unistd32.h           | 3 ++-
 arch/ia64/kernel/syscalls/syscall.tbl       | 2 +-
 arch/m68k/kernel/syscalls/syscall.tbl       | 2 +-
 arch/microblaze/kernel/syscalls/syscall.tbl | 2 +-
 arch/mips/kernel/syscalls/syscall_n32.tbl   | 2 +-
 arch/mips/kernel/syscalls/syscall_n64.tbl   | 2 +-
 arch/mips/kernel/syscalls/syscall_o32.tbl   | 2 +-
 arch/parisc/kernel/syscalls/syscall.tbl     | 2 +-
 arch/powerpc/kernel/syscalls/syscall.tbl    | 2 +-
 arch/s390/kernel/syscalls/syscall.tbl       | 2 +-
 arch/sh/kernel/syscalls/syscall.tbl         | 2 +-
 arch/sparc/kernel/syscalls/syscall.tbl      | 2 +-
 arch/x86/entry/syscalls/syscall_32.tbl      | 2 +-
 arch/x86/entry/syscalls/syscall_64.tbl      | 2 +-
 arch/xtensa/kernel/syscalls/syscall.tbl     | 2 +-
 17 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
index 3000a2e8ee21..a17687ed4b51 100644
--- a/arch/alpha/kernel/syscalls/syscall.tbl
+++ b/arch/alpha/kernel/syscalls/syscall.tbl
@@ -482,7 +482,7 @@
 550	common	process_madvise			sys_process_madvise
 551	common	epoll_pwait2			sys_epoll_pwait2
 552	common	mount_setattr			sys_mount_setattr
-# 553 reserved for quotactl_path
+553	common	quotactl_fd			sys_quotactl_fd
 554	common	landlock_create_ruleset		sys_landlock_create_ruleset
 555	common	landlock_add_rule		sys_landlock_add_rule
 556	common	landlock_restrict_self		sys_landlock_restrict_self
diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
index 28e03b5fec00..c5df1179fc5d 100644
--- a/arch/arm/tools/syscall.tbl
+++ b/arch/arm/tools/syscall.tbl
@@ -456,7 +456,7 @@
 440	common	process_madvise			sys_process_madvise
 441	common	epoll_pwait2			sys_epoll_pwait2
 442	common	mount_setattr			sys_mount_setattr
-# 443 reserved for quotactl_path
+443	common	quotactl_fd			sys_quotactl_fd
 444	common	landlock_create_ruleset		sys_landlock_create_ruleset
 445	common	landlock_add_rule		sys_landlock_add_rule
 446	common	landlock_restrict_self		sys_landlock_restrict_self
diff --git a/arch/arm64/include/asm/unistd32.h b/arch/arm64/include/asm/unistd32.h
index 5dab69d2c22b..99ffcafc736c 100644
--- a/arch/arm64/include/asm/unistd32.h
+++ b/arch/arm64/include/asm/unistd32.h
@@ -893,7 +893,8 @@ __SYSCALL(__NR_process_madvise, sys_process_madvise)
 __SYSCALL(__NR_epoll_pwait2, compat_sys_epoll_pwait2)
 #define __NR_mount_setattr 442
 __SYSCALL(__NR_mount_setattr, sys_mount_setattr)
-/* 443 is reserved for quotactl_path */
+#define __NR_quotactl_fd 443
+__SYSCALL(__NR_quotactl_fd, sys_quotactl_fd)
 #define __NR_landlock_create_ruleset 444
 __SYSCALL(__NR_landlock_create_ruleset, sys_landlock_create_ruleset)
 #define __NR_landlock_add_rule 445
diff --git a/arch/ia64/kernel/syscalls/syscall.tbl b/arch/ia64/kernel/syscalls/syscall.tbl
index bb11fe4c875a..6d07742c57b8 100644
--- a/arch/ia64/kernel/syscalls/syscall.tbl
+++ b/arch/ia64/kernel/syscalls/syscall.tbl
@@ -363,7 +363,7 @@
 440	common	process_madvise			sys_process_madvise
 441	common	epoll_pwait2			sys_epoll_pwait2
 442	common	mount_setattr			sys_mount_setattr
-# 443 reserved for quotactl_path
+443	common	quotactl_fd			sys_quotactl_fd
 444	common	landlock_create_ruleset		sys_landlock_create_ruleset
 445	common	landlock_add_rule		sys_landlock_add_rule
 446	common	landlock_restrict_self		sys_landlock_restrict_self
diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl
index 79c2d24c89dd..541bc1b3a8f9 100644
--- a/arch/m68k/kernel/syscalls/syscall.tbl
+++ b/arch/m68k/kernel/syscalls/syscall.tbl
@@ -442,7 +442,7 @@
 440	common	process_madvise			sys_process_madvise
 441	common	epoll_pwait2			sys_epoll_pwait2
 442	common	mount_setattr			sys_mount_setattr
-# 443 reserved for quotactl_path
+443	common	quotactl_fd			sys_quotactl_fd
 444	common	landlock_create_ruleset		sys_landlock_create_ruleset
 445	common	landlock_add_rule		sys_landlock_add_rule
 446	common	landlock_restrict_self		sys_landlock_restrict_self
diff --git a/arch/microblaze/kernel/syscalls/syscall.tbl b/arch/microblaze/kernel/syscalls/syscall.tbl
index b11395a20c20..a176faca2927 100644
--- a/arch/microblaze/kernel/syscalls/syscall.tbl
+++ b/arch/microblaze/kernel/syscalls/syscall.tbl
@@ -448,7 +448,7 @@
 440	common	process_madvise			sys_process_madvise
 441	common	epoll_pwait2			sys_epoll_pwait2
 442	common	mount_setattr			sys_mount_setattr
-# 443 reserved for quotactl_path
+443	common	quotactl_fd			sys_quotactl_fd
 444	common	landlock_create_ruleset		sys_landlock_create_ruleset
 445	common	landlock_add_rule		sys_landlock_add_rule
 446	common	landlock_restrict_self		sys_landlock_restrict_self
diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
index 9220909526f9..c2d2e19abea8 100644
--- a/arch/mips/kernel/syscalls/syscall_n32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
@@ -381,7 +381,7 @@
 440	n32	process_madvise			sys_process_madvise
 441	n32	epoll_pwait2			compat_sys_epoll_pwait2
 442	n32	mount_setattr			sys_mount_setattr
-# 443 reserved for quotactl_path
+443	n32	quotactl_fd			sys_quotactl_fd
 444	n32	landlock_create_ruleset		sys_landlock_create_ruleset
 445	n32	landlock_add_rule		sys_landlock_add_rule
 446	n32	landlock_restrict_self		sys_landlock_restrict_self
diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl b/arch/mips/kernel/syscalls/syscall_n64.tbl
index 9cd1c34f31b5..ac653d08b1ea 100644
--- a/arch/mips/kernel/syscalls/syscall_n64.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n64.tbl
@@ -357,7 +357,7 @@
 440	n64	process_madvise			sys_process_madvise
 441	n64	epoll_pwait2			sys_epoll_pwait2
 442	n64	mount_setattr			sys_mount_setattr
-# 443 reserved for quotactl_path
+443	n64	quotactl_fd			sys_quotactl_fd
 444	n64	landlock_create_ruleset		sys_landlock_create_ruleset
 445	n64	landlock_add_rule		sys_landlock_add_rule
 446	n64	landlock_restrict_self		sys_landlock_restrict_self
diff --git a/arch/mips/kernel/syscalls/syscall_o32.tbl b/arch/mips/kernel/syscalls/syscall_o32.tbl
index d560c467a8c6..253f2cd70b6b 100644
--- a/arch/mips/kernel/syscalls/syscall_o32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_o32.tbl
@@ -430,7 +430,7 @@
 440	o32	process_madvise			sys_process_madvise
 441	o32	epoll_pwait2			sys_epoll_pwait2		compat_sys_epoll_pwait2
 442	o32	mount_setattr			sys_mount_setattr
-# 443 reserved for quotactl_path
+443	o32	quotactl_fd			sys_quotactl_fd
 444	o32	landlock_create_ruleset		sys_landlock_create_ruleset
 445	o32	landlock_add_rule		sys_landlock_add_rule
 446	o32	landlock_restrict_self		sys_landlock_restrict_self
diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
index aabc37f8cae3..e26187b9ab87 100644
--- a/arch/parisc/kernel/syscalls/syscall.tbl
+++ b/arch/parisc/kernel/syscalls/syscall.tbl
@@ -440,7 +440,7 @@
 440	common	process_madvise			sys_process_madvise
 441	common	epoll_pwait2			sys_epoll_pwait2		compat_sys_epoll_pwait2
 442	common	mount_setattr			sys_mount_setattr
-# 443 reserved for quotactl_path
+443	common	quotactl_fd			sys_quotactl_fd
 444	common	landlock_create_ruleset		sys_landlock_create_ruleset
 445	common	landlock_add_rule		sys_landlock_add_rule
 446	common	landlock_restrict_self		sys_landlock_restrict_self
diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index 8f052ff4058c..aef2a290e71a 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl
@@ -522,7 +522,7 @@
 440	common	process_madvise			sys_process_madvise
 441	common	epoll_pwait2			sys_epoll_pwait2		compat_sys_epoll_pwait2
 442	common	mount_setattr			sys_mount_setattr
-# 443 reserved for quotactl_path
+443	common	quotactl_fd			sys_quotactl_fd
 444	common	landlock_create_ruleset		sys_landlock_create_ruleset
 445	common	landlock_add_rule		sys_landlock_add_rule
 446	common	landlock_restrict_self		sys_landlock_restrict_self
diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
index 0690263df1dd..64d51ab5a8b4 100644
--- a/arch/s390/kernel/syscalls/syscall.tbl
+++ b/arch/s390/kernel/syscalls/syscall.tbl
@@ -445,7 +445,7 @@
 440  common	process_madvise		sys_process_madvise		sys_process_madvise
 441  common	epoll_pwait2		sys_epoll_pwait2		compat_sys_epoll_pwait2
 442  common	mount_setattr		sys_mount_setattr		sys_mount_setattr
-# 443 reserved for quotactl_path
+443  common	quotactl_fd		sys_quotactl_fd			sys_quotactl_fd
 444  common	landlock_create_ruleset	sys_landlock_create_ruleset	sys_landlock_create_ruleset
 445  common	landlock_add_rule	sys_landlock_add_rule		sys_landlock_add_rule
 446  common	landlock_restrict_self	sys_landlock_restrict_self	sys_landlock_restrict_self
diff --git a/arch/sh/kernel/syscalls/syscall.tbl b/arch/sh/kernel/syscalls/syscall.tbl
index 0b91499ebdcf..e0a70be77d84 100644
--- a/arch/sh/kernel/syscalls/syscall.tbl
+++ b/arch/sh/kernel/syscalls/syscall.tbl
@@ -445,7 +445,7 @@
 440	common	process_madvise			sys_process_madvise
 441	common	epoll_pwait2			sys_epoll_pwait2
 442	common	mount_setattr			sys_mount_setattr
-# 443 reserved for quotactl_path
+443	common	quotactl_fd			sys_quotactl_fd
 444	common	landlock_create_ruleset		sys_landlock_create_ruleset
 445	common	landlock_add_rule		sys_landlock_add_rule
 446	common	landlock_restrict_self		sys_landlock_restrict_self
diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl
index e34cc30ef22c..603f5a821502 100644
--- a/arch/sparc/kernel/syscalls/syscall.tbl
+++ b/arch/sparc/kernel/syscalls/syscall.tbl
@@ -488,7 +488,7 @@
 440	common	process_madvise			sys_process_madvise
 441	common	epoll_pwait2			sys_epoll_pwait2		compat_sys_epoll_pwait2
 442	common	mount_setattr			sys_mount_setattr
-# 443 reserved for quotactl_path
+443	common	quotactl_fd			sys_quotactl_fd
 444	common	landlock_create_ruleset		sys_landlock_create_ruleset
 445	common	landlock_add_rule		sys_landlock_add_rule
 446	common	landlock_restrict_self		sys_landlock_restrict_self
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index 4bbc267fb36b..fba2f615119a 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -447,7 +447,7 @@
 440	i386	process_madvise		sys_process_madvise
 441	i386	epoll_pwait2		sys_epoll_pwait2		compat_sys_epoll_pwait2
 442	i386	mount_setattr		sys_mount_setattr
-# 443 reserved for quotactl_path
+443	i386	quotactl_fd		sys_quotactl_fd
 444	i386	landlock_create_ruleset	sys_landlock_create_ruleset
 445	i386	landlock_add_rule	sys_landlock_add_rule
 446	i386	landlock_restrict_self	sys_landlock_restrict_self
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index ce18119ea0d0..af973e400053 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -364,7 +364,7 @@
 440	common	process_madvise		sys_process_madvise
 441	common	epoll_pwait2		sys_epoll_pwait2
 442	common	mount_setattr		sys_mount_setattr
-# 443 reserved for quotactl_path
+443	common	quotactl_fd		sys_quotactl_fd
 444	common	landlock_create_ruleset	sys_landlock_create_ruleset
 445	common	landlock_add_rule	sys_landlock_add_rule
 446	common	landlock_restrict_self	sys_landlock_restrict_self
diff --git a/arch/xtensa/kernel/syscalls/syscall.tbl b/arch/xtensa/kernel/syscalls/syscall.tbl
index fd2f30227d96..235d67d6ceb4 100644
--- a/arch/xtensa/kernel/syscalls/syscall.tbl
+++ b/arch/xtensa/kernel/syscalls/syscall.tbl
@@ -413,7 +413,7 @@
 440	common	process_madvise			sys_process_madvise
 441	common	epoll_pwait2			sys_epoll_pwait2
 442	common	mount_setattr			sys_mount_setattr
-# 443 reserved for quotactl_path
+443	common	quotactl_fd			sys_quotactl_fd
 444	common	landlock_create_ruleset		sys_landlock_create_ruleset
 445	common	landlock_add_rule		sys_landlock_add_rule
 446	common	landlock_restrict_self		sys_landlock_restrict_self
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] Change quotactl_path() to an fd-based syscall
  2021-06-02 15:15 [PATCH 0/2] Change quotactl_path() to an fd-based syscall Jan Kara
  2021-06-02 15:15 ` [PATCH 1/2] quota: Change quotactl_path() systcall to an fd-based one Jan Kara
  2021-06-02 15:15 ` [PATCH 2/2] quota: Wire up quotactl_fd syscall Jan Kara
@ 2021-06-02 15:19 ` Jan Kara
  2021-06-03 12:03   ` Christian Brauner
  2021-06-07  6:14 ` Christoph Hellwig
  3 siblings, 1 reply; 12+ messages in thread
From: Jan Kara @ 2021-06-02 15:19 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: Christoph Hellwig, linux-api, Sascha Hauer, Jan Kara, Christian Brauner

On Wed 02-06-21 17:15:51, Jan Kara wrote:
> Hello,
> 
> this patch series changes Sasha's quotactl_path() syscall to an fd-based one
> quotactl_fd() syscall and enables the syscall again. The fd-based syscall was
> chosen over the path based one because there's no real need for the path -
> identifying filesystem to operate on by fd is perfectly fine for quotactl and
> thus we can avoid the need to specify all the details of path lookup in the
> quotactl_path() API (and possibly keep that uptodate with all the developments
> in that field).
> 
> Patches passed some basic functional testing. Please review.

Sorry Christian, I've messed up your address when submitting this series...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] quota: Change quotactl_path() systcall to an fd-based one
  2021-06-02 15:15 ` [PATCH 1/2] quota: Change quotactl_path() systcall to an fd-based one Jan Kara
@ 2021-06-03  9:41   ` Sascha Hauer
  2021-06-03 10:11     ` Jan Kara
  2021-06-03 12:25   ` Christian Brauner
  2021-06-03 14:27     ` kernel test robot
  2 siblings, 1 reply; 12+ messages in thread
From: Sascha Hauer @ 2021-06-03  9:41 UTC (permalink / raw)
  To: Jan Kara; +Cc: linux-fsdevel, Christoph Hellwig, brauner, linux-api

Hello Honza,

On Wed, Jun 02, 2021 at 05:15:52PM +0200, Jan Kara wrote:
> Some users have pointed out that path-based syscalls are problematic in
> some environments and at least directory fd argument and possibly also
> resolve flags are desirable for such syscalls. Rather than
> reimplementing all details of pathname lookup and following where it may
> eventually evolve, let's go for full file descriptor based syscall
> similar to how ioctl(2) works since the beginning. Managing of quotas
> isn't performance sensitive so the extra overhead of open does not
> matter and we are able to consume O_PATH descriptors as well which makes
> open cheap anyway. Also for frequent operations (such as retrieving
> usage information for all users) we can reuse single fd and in fact get
> even better performance as well as avoiding races with possible remounts
> etc.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/quota/quota.c                  | 27 ++++++++++++---------------
>  include/linux/syscalls.h          |  4 ++--
>  include/uapi/asm-generic/unistd.h |  4 ++--
>  kernel/sys_ni.c                   |  2 +-
>  4 files changed, 17 insertions(+), 20 deletions(-)

Thanks for taking care of this.

I also gave this some testing and it's working ok for me, so at least I
can give you my:

Tested-by: Sascha Hauer <s.hauer@pengutronix.de>

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] quota: Change quotactl_path() systcall to an fd-based one
  2021-06-03  9:41   ` Sascha Hauer
@ 2021-06-03 10:11     ` Jan Kara
  0 siblings, 0 replies; 12+ messages in thread
From: Jan Kara @ 2021-06-03 10:11 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Jan Kara, linux-fsdevel, Christoph Hellwig, brauner, linux-api

On Thu 03-06-21 11:41:02, Sascha Hauer wrote:
> Hello Honza,
> 
> On Wed, Jun 02, 2021 at 05:15:52PM +0200, Jan Kara wrote:
> > Some users have pointed out that path-based syscalls are problematic in
> > some environments and at least directory fd argument and possibly also
> > resolve flags are desirable for such syscalls. Rather than
> > reimplementing all details of pathname lookup and following where it may
> > eventually evolve, let's go for full file descriptor based syscall
> > similar to how ioctl(2) works since the beginning. Managing of quotas
> > isn't performance sensitive so the extra overhead of open does not
> > matter and we are able to consume O_PATH descriptors as well which makes
> > open cheap anyway. Also for frequent operations (such as retrieving
> > usage information for all users) we can reuse single fd and in fact get
> > even better performance as well as avoiding races with possible remounts
> > etc.
> > 
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> >  fs/quota/quota.c                  | 27 ++++++++++++---------------
> >  include/linux/syscalls.h          |  4 ++--
> >  include/uapi/asm-generic/unistd.h |  4 ++--
> >  kernel/sys_ni.c                   |  2 +-
> >  4 files changed, 17 insertions(+), 20 deletions(-)
> 
> Thanks for taking care of this.
> 
> I also gave this some testing and it's working ok for me, so at least I
> can give you my:
> 
> Tested-by: Sascha Hauer <s.hauer@pengutronix.de>

Thanks!

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] Change quotactl_path() to an fd-based syscall
  2021-06-02 15:19 ` [PATCH 0/2] Change quotactl_path() to an fd-based syscall Jan Kara
@ 2021-06-03 12:03   ` Christian Brauner
  0 siblings, 0 replies; 12+ messages in thread
From: Christian Brauner @ 2021-06-03 12:03 UTC (permalink / raw)
  To: Jan Kara; +Cc: linux-fsdevel, Christoph Hellwig, linux-api, Sascha Hauer

On Wed, Jun 02, 2021 at 05:19:32PM +0200, Jan Kara wrote:
> On Wed 02-06-21 17:15:51, Jan Kara wrote:
> > Hello,
> > 
> > this patch series changes Sasha's quotactl_path() syscall to an fd-based one
> > quotactl_fd() syscall and enables the syscall again. The fd-based syscall was
> > chosen over the path based one because there's no real need for the path -
> > identifying filesystem to operate on by fd is perfectly fine for quotactl and
> > thus we can avoid the need to specify all the details of path lookup in the
> > quotactl_path() API (and possibly keep that uptodate with all the developments
> > in that field).
> > 
> > Patches passed some basic functional testing. Please review.
> 
> Sorry Christian, I've messed up your address when submitting this series...

No problem at all. :)

Christian

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] quota: Change quotactl_path() systcall to an fd-based one
  2021-06-02 15:15 ` [PATCH 1/2] quota: Change quotactl_path() systcall to an fd-based one Jan Kara
  2021-06-03  9:41   ` Sascha Hauer
@ 2021-06-03 12:25   ` Christian Brauner
  2021-06-03 15:02     ` Jan Kara
  2021-06-03 14:27     ` kernel test robot
  2 siblings, 1 reply; 12+ messages in thread
From: Christian Brauner @ 2021-06-03 12:25 UTC (permalink / raw)
  To: Jan Kara
  Cc: linux-fsdevel, Christoph Hellwig, Christian Brauner, linux-api,
	Sascha Hauer

On Wed, Jun 02, 2021 at 05:15:52PM +0200, Jan Kara wrote:
> Some users have pointed out that path-based syscalls are problematic in
> some environments and at least directory fd argument and possibly also
> resolve flags are desirable for such syscalls. Rather than
> reimplementing all details of pathname lookup and following where it may
> eventually evolve, let's go for full file descriptor based syscall

Fair, I can accept that.

> similar to how ioctl(2) works since the beginning. Managing of quotas
> isn't performance sensitive so the extra overhead of open does not
> matter and we are able to consume O_PATH descriptors as well which makes
> open cheap anyway. Also for frequent operations (such as retrieving
> usage information for all users) we can reuse single fd and in fact get
> even better performance as well as avoiding races with possible remounts
> etc.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/quota/quota.c                  | 27 ++++++++++++---------------
>  include/linux/syscalls.h          |  4 ++--
>  include/uapi/asm-generic/unistd.h |  4 ++--
>  kernel/sys_ni.c                   |  2 +-
>  4 files changed, 17 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/quota/quota.c b/fs/quota/quota.c
> index 05e4bd9ab6d6..8450bb6186f4 100644
> --- a/fs/quota/quota.c
> +++ b/fs/quota/quota.c
> @@ -968,31 +968,29 @@ SYSCALL_DEFINE4(quotactl, unsigned int, cmd, const char __user *, special,
>  	return ret;
>  }
>  
> -SYSCALL_DEFINE4(quotactl_path, unsigned int, cmd, const char __user *,
> -		mountpoint, qid_t, id, void __user *, addr)
> +SYSCALL_DEFINE4(quotactl_fd, unsigned int, fd, unsigned int, cmd,
> +		qid_t, id, void __user *, addr)
>  {
>  	struct super_block *sb;
> -	struct path mountpath;
>  	unsigned int cmds = cmd >> SUBCMDSHIFT;
>  	unsigned int type = cmd & SUBCMDMASK;
> +	struct fd f = fdget_raw(fd);
>  	int ret;
>  
> -	if (type >= MAXQUOTAS)
> -		return -EINVAL;
> +	if (!f.file)
> +		return -EBADF;

I would maybe change this to

f = fdget_raw(fd);
if (!f.file)
	return -EBADF;

instead of directly assigning when declaring the variable.
(And it might make sense to fold the second commit into this one.)
But other than these nits this looks good,

Acked-by: Christian Brauner <christian.brauner@ubuntu.com>

>  
> -	ret = user_path_at(AT_FDCWD, mountpoint,
> -			     LOOKUP_FOLLOW | LOOKUP_AUTOMOUNT, &mountpath);
> -	if (ret)
> -		return ret;
> -
> -	sb = mountpath.mnt->mnt_sb;
> +	ret = -EINVAL;
> +	if (type >= MAXQUOTAS)
> +		goto out;
>  
>  	if (quotactl_cmd_write(cmds)) {
> -		ret = mnt_want_write(mountpath.mnt);
> +		ret = mnt_want_write(f.file->f_path.mnt);
>  		if (ret)
>  			goto out;
>  	}
>  
> +	sb = f.file->f_path.mnt->mnt_sb;
>  	if (quotactl_cmd_onoff(cmds))
>  		down_write(&sb->s_umount);
>  	else
> @@ -1006,9 +1004,8 @@ SYSCALL_DEFINE4(quotactl_path, unsigned int, cmd, const char __user *,
>  		up_read(&sb->s_umount);
>  
>  	if (quotactl_cmd_write(cmds))
> -		mnt_drop_write(mountpath.mnt);
> +		mnt_drop_write(f.file->f_path.mnt);
>  out:
> -	path_put(&mountpath);
> -
> +	fdput(f);
>  	return ret;
>  }
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 050511e8f1f8..586128d5c3b8 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -485,8 +485,8 @@ asmlinkage long sys_pipe2(int __user *fildes, int flags);
>  /* fs/quota.c */
>  asmlinkage long sys_quotactl(unsigned int cmd, const char __user *special,
>  				qid_t id, void __user *addr);
> -asmlinkage long sys_quotactl_path(unsigned int cmd, const char __user *mountpoint,
> -				  qid_t id, void __user *addr);
> +asmlinkage long sys_quotactl_fd(unsigned int fd, unsigned int cmd, qid_t id,
> +				void __user *addr);
>  
>  /* fs/readdir.c */
>  asmlinkage long sys_getdents64(unsigned int fd,
> diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
> index 6de5a7fc066b..f211961ce1da 100644
> --- a/include/uapi/asm-generic/unistd.h
> +++ b/include/uapi/asm-generic/unistd.h
> @@ -863,8 +863,8 @@ __SYSCALL(__NR_process_madvise, sys_process_madvise)
>  __SC_COMP(__NR_epoll_pwait2, sys_epoll_pwait2, compat_sys_epoll_pwait2)
>  #define __NR_mount_setattr 442
>  __SYSCALL(__NR_mount_setattr, sys_mount_setattr)
> -#define __NR_quotactl_path 443
> -__SYSCALL(__NR_quotactl_path, sys_quotactl_path)
> +#define __NR_quotactl_fd 443
> +__SYSCALL(__NR_quotactl_fd, sys_quotactl_fd)
>  
>  #define __NR_landlock_create_ruleset 444
>  __SYSCALL(__NR_landlock_create_ruleset, sys_landlock_create_ruleset)
> diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
> index 0ea8128468c3..dad4d994641e 100644
> --- a/kernel/sys_ni.c
> +++ b/kernel/sys_ni.c
> @@ -99,7 +99,7 @@ COND_SYSCALL(flock);
>  
>  /* fs/quota.c */
>  COND_SYSCALL(quotactl);
> -COND_SYSCALL(quotactl_path);
> +COND_SYSCALL(quotactl_fd);
>  
>  /* fs/readdir.c */
>  
> -- 
> 2.26.2
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] quota: Change quotactl_path() systcall to an fd-based one
  2021-06-02 15:15 ` [PATCH 1/2] quota: Change quotactl_path() systcall to an fd-based one Jan Kara
@ 2021-06-03 14:27     ` kernel test robot
  2021-06-03 12:25   ` Christian Brauner
  2021-06-03 14:27     ` kernel test robot
  2 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2021-06-03 14:27 UTC (permalink / raw)
  To: Jan Kara, linux-fsdevel
  Cc: kbuild-all, clang-built-linux, Christoph Hellwig, brauner,
	linux-api, Sascha Hauer, Jan Kara

[-- Attachment #1: Type: text/plain, Size: 14141 bytes --]

Hi Jan,

I love your patch! Perhaps something to improve:

[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on m68k/for-next powerpc/next s390/features linus/master v5.13-rc4 next-20210603]
[cannot apply to tip/x86/asm asm-generic/master hp-parisc/for-next sparc-next/master sparc/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Jan-Kara/Change-quotactl_path-to-an-fd-based-syscall/20210602-232203
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: arm64-randconfig-r002-20210603 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project d8e0ae9a76a62bdc6117630d59bf9967ac9bb4ea)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # https://github.com/0day-ci/linux/commit/2f0109eecacfbf0ade367f8c7631ad18e4368ab5
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jan-Kara/Change-quotactl_path-to-an-fd-based-syscall/20210602-232203
        git checkout 2f0109eecacfbf0ade367f8c7631ad18e4368ab5
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

           asmlinkage long __weak __arm64_compat_sys_##name(const struct pt_regs *regs)    \
                                  ^
   <scratch space>:41:1: note: expanded from here
   __arm64_compat_sys_epoll_pwait2
   ^
   kernel/sys_ni.c:72:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:41:13: note: expanded from macro 'COND_SYSCALL_COMPAT'
           asmlinkage long __weak __arm64_compat_sys_##name(const struct pt_regs *regs)    \
                      ^
   kernel/sys_ni.c:77:1: warning: no previous prototype for function '__arm64_sys_inotify_init1' [-Wmissing-prototypes]
   COND_SYSCALL(inotify_init1);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:42:1: note: expanded from here
   __arm64_sys_inotify_init1
   ^
   kernel/sys_ni.c:77:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:78:1: warning: no previous prototype for function '__arm64_sys_inotify_add_watch' [-Wmissing-prototypes]
   COND_SYSCALL(inotify_add_watch);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:43:1: note: expanded from here
   __arm64_sys_inotify_add_watch
   ^
   kernel/sys_ni.c:78:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:79:1: warning: no previous prototype for function '__arm64_sys_inotify_rm_watch' [-Wmissing-prototypes]
   COND_SYSCALL(inotify_rm_watch);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:44:1: note: expanded from here
   __arm64_sys_inotify_rm_watch
   ^
   kernel/sys_ni.c:79:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:84:1: warning: no previous prototype for function '__arm64_sys_ioprio_set' [-Wmissing-prototypes]
   COND_SYSCALL(ioprio_set);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:45:1: note: expanded from here
   __arm64_sys_ioprio_set
   ^
   kernel/sys_ni.c:84:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:85:1: warning: no previous prototype for function '__arm64_sys_ioprio_get' [-Wmissing-prototypes]
   COND_SYSCALL(ioprio_get);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:46:1: note: expanded from here
   __arm64_sys_ioprio_get
   ^
   kernel/sys_ni.c:85:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:88:1: warning: no previous prototype for function '__arm64_sys_flock' [-Wmissing-prototypes]
   COND_SYSCALL(flock);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:47:1: note: expanded from here
   __arm64_sys_flock
   ^
   kernel/sys_ni.c:88:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:101:1: warning: no previous prototype for function '__arm64_sys_quotactl' [-Wmissing-prototypes]
   COND_SYSCALL(quotactl);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:48:1: note: expanded from here
   __arm64_sys_quotactl
   ^
   kernel/sys_ni.c:101:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
>> kernel/sys_ni.c:102:1: warning: no previous prototype for function '__arm64_sys_quotactl_fd' [-Wmissing-prototypes]
   COND_SYSCALL(quotactl_fd);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:49:1: note: expanded from here
   __arm64_sys_quotactl_fd
   ^
   kernel/sys_ni.c:102:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:113:1: warning: no previous prototype for function '__arm64_sys_signalfd4' [-Wmissing-prototypes]
   COND_SYSCALL(signalfd4);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:50:1: note: expanded from here
   __arm64_sys_signalfd4
   ^
   kernel/sys_ni.c:113:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:114:1: warning: no previous prototype for function '__arm64_compat_sys_signalfd4' [-Wmissing-prototypes]
   COND_SYSCALL_COMPAT(signalfd4);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:41:25: note: expanded from macro 'COND_SYSCALL_COMPAT'
           asmlinkage long __weak __arm64_compat_sys_##name(const struct pt_regs *regs)    \
                                  ^
   <scratch space>:51:1: note: expanded from here
   __arm64_compat_sys_signalfd4
   ^
   kernel/sys_ni.c:114:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:41:13: note: expanded from macro 'COND_SYSCALL_COMPAT'
           asmlinkage long __weak __arm64_compat_sys_##name(const struct pt_regs *regs)    \
                      ^
   kernel/sys_ni.c:123:1: warning: no previous prototype for function '__arm64_sys_timerfd_create' [-Wmissing-prototypes]
   COND_SYSCALL(timerfd_create);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:52:1: note: expanded from here
   __arm64_sys_timerfd_create
   ^
   kernel/sys_ni.c:123:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:124:1: warning: no previous prototype for function '__arm64_sys_timerfd_settime' [-Wmissing-prototypes]
   COND_SYSCALL(timerfd_settime);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:53:1: note: expanded from here
   __arm64_sys_timerfd_settime
   ^
   kernel/sys_ni.c:124:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:125:1: warning: no previous prototype for function '__arm64_sys_timerfd_settime32' [-Wmissing-prototypes]
   COND_SYSCALL(timerfd_settime32);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:54:1: note: expanded from here
   __arm64_sys_timerfd_settime32
   ^
   kernel/sys_ni.c:125:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:126:1: warning: no previous prototype for function '__arm64_sys_timerfd_gettime' [-Wmissing-prototypes]
   COND_SYSCALL(timerfd_gettime);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:55:1: note: expanded from here
   __arm64_sys_timerfd_gettime
   ^
   kernel/sys_ni.c:126:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:127:1: warning: no previous prototype for function '__arm64_sys_timerfd_gettime32' [-Wmissing-prototypes]
   COND_SYSCALL(timerfd_gettime32);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:56:1: note: expanded from here
   __arm64_sys_timerfd_gettime32
   ^
   kernel/sys_ni.c:127:1: note: declare 'static' if the function is not intended to be used outside of this translation unit


vim +/__arm64_sys_quotactl_fd +102 kernel/sys_ni.c

    99	
   100	/* fs/quota.c */
   101	COND_SYSCALL(quotactl);
 > 102	COND_SYSCALL(quotactl_fd);
   103	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 40587 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] quota: Change quotactl_path() systcall to an fd-based one
@ 2021-06-03 14:27     ` kernel test robot
  0 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2021-06-03 14:27 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 14389 bytes --]

Hi Jan,

I love your patch! Perhaps something to improve:

[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on m68k/for-next powerpc/next s390/features linus/master v5.13-rc4 next-20210603]
[cannot apply to tip/x86/asm asm-generic/master hp-parisc/for-next sparc-next/master sparc/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Jan-Kara/Change-quotactl_path-to-an-fd-based-syscall/20210602-232203
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: arm64-randconfig-r002-20210603 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project d8e0ae9a76a62bdc6117630d59bf9967ac9bb4ea)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # https://github.com/0day-ci/linux/commit/2f0109eecacfbf0ade367f8c7631ad18e4368ab5
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jan-Kara/Change-quotactl_path-to-an-fd-based-syscall/20210602-232203
        git checkout 2f0109eecacfbf0ade367f8c7631ad18e4368ab5
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

           asmlinkage long __weak __arm64_compat_sys_##name(const struct pt_regs *regs)    \
                                  ^
   <scratch space>:41:1: note: expanded from here
   __arm64_compat_sys_epoll_pwait2
   ^
   kernel/sys_ni.c:72:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:41:13: note: expanded from macro 'COND_SYSCALL_COMPAT'
           asmlinkage long __weak __arm64_compat_sys_##name(const struct pt_regs *regs)    \
                      ^
   kernel/sys_ni.c:77:1: warning: no previous prototype for function '__arm64_sys_inotify_init1' [-Wmissing-prototypes]
   COND_SYSCALL(inotify_init1);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:42:1: note: expanded from here
   __arm64_sys_inotify_init1
   ^
   kernel/sys_ni.c:77:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:78:1: warning: no previous prototype for function '__arm64_sys_inotify_add_watch' [-Wmissing-prototypes]
   COND_SYSCALL(inotify_add_watch);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:43:1: note: expanded from here
   __arm64_sys_inotify_add_watch
   ^
   kernel/sys_ni.c:78:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:79:1: warning: no previous prototype for function '__arm64_sys_inotify_rm_watch' [-Wmissing-prototypes]
   COND_SYSCALL(inotify_rm_watch);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:44:1: note: expanded from here
   __arm64_sys_inotify_rm_watch
   ^
   kernel/sys_ni.c:79:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:84:1: warning: no previous prototype for function '__arm64_sys_ioprio_set' [-Wmissing-prototypes]
   COND_SYSCALL(ioprio_set);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:45:1: note: expanded from here
   __arm64_sys_ioprio_set
   ^
   kernel/sys_ni.c:84:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:85:1: warning: no previous prototype for function '__arm64_sys_ioprio_get' [-Wmissing-prototypes]
   COND_SYSCALL(ioprio_get);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:46:1: note: expanded from here
   __arm64_sys_ioprio_get
   ^
   kernel/sys_ni.c:85:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:88:1: warning: no previous prototype for function '__arm64_sys_flock' [-Wmissing-prototypes]
   COND_SYSCALL(flock);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:47:1: note: expanded from here
   __arm64_sys_flock
   ^
   kernel/sys_ni.c:88:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:101:1: warning: no previous prototype for function '__arm64_sys_quotactl' [-Wmissing-prototypes]
   COND_SYSCALL(quotactl);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:48:1: note: expanded from here
   __arm64_sys_quotactl
   ^
   kernel/sys_ni.c:101:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
>> kernel/sys_ni.c:102:1: warning: no previous prototype for function '__arm64_sys_quotactl_fd' [-Wmissing-prototypes]
   COND_SYSCALL(quotactl_fd);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:49:1: note: expanded from here
   __arm64_sys_quotactl_fd
   ^
   kernel/sys_ni.c:102:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:113:1: warning: no previous prototype for function '__arm64_sys_signalfd4' [-Wmissing-prototypes]
   COND_SYSCALL(signalfd4);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:50:1: note: expanded from here
   __arm64_sys_signalfd4
   ^
   kernel/sys_ni.c:113:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:114:1: warning: no previous prototype for function '__arm64_compat_sys_signalfd4' [-Wmissing-prototypes]
   COND_SYSCALL_COMPAT(signalfd4);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:41:25: note: expanded from macro 'COND_SYSCALL_COMPAT'
           asmlinkage long __weak __arm64_compat_sys_##name(const struct pt_regs *regs)    \
                                  ^
   <scratch space>:51:1: note: expanded from here
   __arm64_compat_sys_signalfd4
   ^
   kernel/sys_ni.c:114:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:41:13: note: expanded from macro 'COND_SYSCALL_COMPAT'
           asmlinkage long __weak __arm64_compat_sys_##name(const struct pt_regs *regs)    \
                      ^
   kernel/sys_ni.c:123:1: warning: no previous prototype for function '__arm64_sys_timerfd_create' [-Wmissing-prototypes]
   COND_SYSCALL(timerfd_create);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:52:1: note: expanded from here
   __arm64_sys_timerfd_create
   ^
   kernel/sys_ni.c:123:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:124:1: warning: no previous prototype for function '__arm64_sys_timerfd_settime' [-Wmissing-prototypes]
   COND_SYSCALL(timerfd_settime);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:53:1: note: expanded from here
   __arm64_sys_timerfd_settime
   ^
   kernel/sys_ni.c:124:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:125:1: warning: no previous prototype for function '__arm64_sys_timerfd_settime32' [-Wmissing-prototypes]
   COND_SYSCALL(timerfd_settime32);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:54:1: note: expanded from here
   __arm64_sys_timerfd_settime32
   ^
   kernel/sys_ni.c:125:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:126:1: warning: no previous prototype for function '__arm64_sys_timerfd_gettime' [-Wmissing-prototypes]
   COND_SYSCALL(timerfd_gettime);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:55:1: note: expanded from here
   __arm64_sys_timerfd_gettime
   ^
   kernel/sys_ni.c:126:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   arch/arm64/include/asm/syscall_wrapper.h:76:13: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                      ^
   kernel/sys_ni.c:127:1: warning: no previous prototype for function '__arm64_sys_timerfd_gettime32' [-Wmissing-prototypes]
   COND_SYSCALL(timerfd_gettime32);
   ^
   arch/arm64/include/asm/syscall_wrapper.h:76:25: note: expanded from macro 'COND_SYSCALL'
           asmlinkage long __weak __arm64_sys_##name(const struct pt_regs *regs)   \
                                  ^
   <scratch space>:56:1: note: expanded from here
   __arm64_sys_timerfd_gettime32
   ^
   kernel/sys_ni.c:127:1: note: declare 'static' if the function is not intended to be used outside of this translation unit


vim +/__arm64_sys_quotactl_fd +102 kernel/sys_ni.c

    99	
   100	/* fs/quota.c */
   101	COND_SYSCALL(quotactl);
 > 102	COND_SYSCALL(quotactl_fd);
   103	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 40587 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] quota: Change quotactl_path() systcall to an fd-based one
  2021-06-03 12:25   ` Christian Brauner
@ 2021-06-03 15:02     ` Jan Kara
  0 siblings, 0 replies; 12+ messages in thread
From: Jan Kara @ 2021-06-03 15:02 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Jan Kara, linux-fsdevel, Christoph Hellwig, Christian Brauner,
	linux-api, Sascha Hauer

On Thu 03-06-21 14:25:04, Christian Brauner wrote:
> On Wed, Jun 02, 2021 at 05:15:52PM +0200, Jan Kara wrote:
> > Some users have pointed out that path-based syscalls are problematic in
> > some environments and at least directory fd argument and possibly also
> > resolve flags are desirable for such syscalls. Rather than
> > reimplementing all details of pathname lookup and following where it may
> > eventually evolve, let's go for full file descriptor based syscall
> 
> Fair, I can accept that.
> 
> > similar to how ioctl(2) works since the beginning. Managing of quotas
> > isn't performance sensitive so the extra overhead of open does not
> > matter and we are able to consume O_PATH descriptors as well which makes
> > open cheap anyway. Also for frequent operations (such as retrieving
> > usage information for all users) we can reuse single fd and in fact get
> > even better performance as well as avoiding races with possible remounts
> > etc.
> > 
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> >  fs/quota/quota.c                  | 27 ++++++++++++---------------
> >  include/linux/syscalls.h          |  4 ++--
> >  include/uapi/asm-generic/unistd.h |  4 ++--
> >  kernel/sys_ni.c                   |  2 +-
> >  4 files changed, 17 insertions(+), 20 deletions(-)
> > 
> > diff --git a/fs/quota/quota.c b/fs/quota/quota.c
> > index 05e4bd9ab6d6..8450bb6186f4 100644
> > --- a/fs/quota/quota.c
> > +++ b/fs/quota/quota.c
> > @@ -968,31 +968,29 @@ SYSCALL_DEFINE4(quotactl, unsigned int, cmd, const char __user *, special,
> >  	return ret;
> >  }
> >  
> > -SYSCALL_DEFINE4(quotactl_path, unsigned int, cmd, const char __user *,
> > -		mountpoint, qid_t, id, void __user *, addr)
> > +SYSCALL_DEFINE4(quotactl_fd, unsigned int, fd, unsigned int, cmd,
> > +		qid_t, id, void __user *, addr)
> >  {
> >  	struct super_block *sb;
> > -	struct path mountpath;
> >  	unsigned int cmds = cmd >> SUBCMDSHIFT;
> >  	unsigned int type = cmd & SUBCMDMASK;
> > +	struct fd f = fdget_raw(fd);
> >  	int ret;
> >  
> > -	if (type >= MAXQUOTAS)
> > -		return -EINVAL;
> > +	if (!f.file)
> > +		return -EBADF;
> 
> I would maybe change this to
> 
> f = fdget_raw(fd);
> if (!f.file)
> 	return -EBADF;
> 
> instead of directly assigning when declaring the variable.

OK, I'll do this.

> (And it might make sense to fold the second commit into this one.)

Well, I wanted to keep mostly mechanical (and partly autogenerated ;))
changes separately from the "real" stuff that needs serious review... So I
prefer to keep the split.

> But other than these nits this looks good,
> 
> Acked-by: Christian Brauner <christian.brauner@ubuntu.com>

Thanks for review!

								Honza
> 
> >  
> > -	ret = user_path_at(AT_FDCWD, mountpoint,
> > -			     LOOKUP_FOLLOW | LOOKUP_AUTOMOUNT, &mountpath);
> > -	if (ret)
> > -		return ret;
> > -
> > -	sb = mountpath.mnt->mnt_sb;
> > +	ret = -EINVAL;
> > +	if (type >= MAXQUOTAS)
> > +		goto out;
> >  
> >  	if (quotactl_cmd_write(cmds)) {
> > -		ret = mnt_want_write(mountpath.mnt);
> > +		ret = mnt_want_write(f.file->f_path.mnt);
> >  		if (ret)
> >  			goto out;
> >  	}
> >  
> > +	sb = f.file->f_path.mnt->mnt_sb;
> >  	if (quotactl_cmd_onoff(cmds))
> >  		down_write(&sb->s_umount);
> >  	else
> > @@ -1006,9 +1004,8 @@ SYSCALL_DEFINE4(quotactl_path, unsigned int, cmd, const char __user *,
> >  		up_read(&sb->s_umount);
> >  
> >  	if (quotactl_cmd_write(cmds))
> > -		mnt_drop_write(mountpath.mnt);
> > +		mnt_drop_write(f.file->f_path.mnt);
> >  out:
> > -	path_put(&mountpath);
> > -
> > +	fdput(f);
> >  	return ret;
> >  }
> > diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> > index 050511e8f1f8..586128d5c3b8 100644
> > --- a/include/linux/syscalls.h
> > +++ b/include/linux/syscalls.h
> > @@ -485,8 +485,8 @@ asmlinkage long sys_pipe2(int __user *fildes, int flags);
> >  /* fs/quota.c */
> >  asmlinkage long sys_quotactl(unsigned int cmd, const char __user *special,
> >  				qid_t id, void __user *addr);
> > -asmlinkage long sys_quotactl_path(unsigned int cmd, const char __user *mountpoint,
> > -				  qid_t id, void __user *addr);
> > +asmlinkage long sys_quotactl_fd(unsigned int fd, unsigned int cmd, qid_t id,
> > +				void __user *addr);
> >  
> >  /* fs/readdir.c */
> >  asmlinkage long sys_getdents64(unsigned int fd,
> > diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
> > index 6de5a7fc066b..f211961ce1da 100644
> > --- a/include/uapi/asm-generic/unistd.h
> > +++ b/include/uapi/asm-generic/unistd.h
> > @@ -863,8 +863,8 @@ __SYSCALL(__NR_process_madvise, sys_process_madvise)
> >  __SC_COMP(__NR_epoll_pwait2, sys_epoll_pwait2, compat_sys_epoll_pwait2)
> >  #define __NR_mount_setattr 442
> >  __SYSCALL(__NR_mount_setattr, sys_mount_setattr)
> > -#define __NR_quotactl_path 443
> > -__SYSCALL(__NR_quotactl_path, sys_quotactl_path)
> > +#define __NR_quotactl_fd 443
> > +__SYSCALL(__NR_quotactl_fd, sys_quotactl_fd)
> >  
> >  #define __NR_landlock_create_ruleset 444
> >  __SYSCALL(__NR_landlock_create_ruleset, sys_landlock_create_ruleset)
> > diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
> > index 0ea8128468c3..dad4d994641e 100644
> > --- a/kernel/sys_ni.c
> > +++ b/kernel/sys_ni.c
> > @@ -99,7 +99,7 @@ COND_SYSCALL(flock);
> >  
> >  /* fs/quota.c */
> >  COND_SYSCALL(quotactl);
> > -COND_SYSCALL(quotactl_path);
> > +COND_SYSCALL(quotactl_fd);
> >  
> >  /* fs/readdir.c */
> >  
> > -- 
> > 2.26.2
> > 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] Change quotactl_path() to an fd-based syscall
  2021-06-02 15:15 [PATCH 0/2] Change quotactl_path() to an fd-based syscall Jan Kara
                   ` (2 preceding siblings ...)
  2021-06-02 15:19 ` [PATCH 0/2] Change quotactl_path() to an fd-based syscall Jan Kara
@ 2021-06-07  6:14 ` Christoph Hellwig
  3 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2021-06-07  6:14 UTC (permalink / raw)
  To: Jan Kara
  Cc: linux-fsdevel, Christoph Hellwig, brauner, linux-api, Sascha Hauer

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-06-07  6:15 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-02 15:15 [PATCH 0/2] Change quotactl_path() to an fd-based syscall Jan Kara
2021-06-02 15:15 ` [PATCH 1/2] quota: Change quotactl_path() systcall to an fd-based one Jan Kara
2021-06-03  9:41   ` Sascha Hauer
2021-06-03 10:11     ` Jan Kara
2021-06-03 12:25   ` Christian Brauner
2021-06-03 15:02     ` Jan Kara
2021-06-03 14:27   ` kernel test robot
2021-06-03 14:27     ` kernel test robot
2021-06-02 15:15 ` [PATCH 2/2] quota: Wire up quotactl_fd syscall Jan Kara
2021-06-02 15:19 ` [PATCH 0/2] Change quotactl_path() to an fd-based syscall Jan Kara
2021-06-03 12:03   ` Christian Brauner
2021-06-07  6:14 ` Christoph Hellwig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.