* misc cleanups to the resblks interfaces
@ 2023-11-26 13:01 Christoph Hellwig
2023-11-26 13:01 ` [PATCH 1/4] xfs: clean up the XFS_IOC_{GS}ET_RESBLKS handler Christoph Hellwig
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Christoph Hellwig @ 2023-11-26 13:01 UTC (permalink / raw
To: Chandan Babu R; +Cc: Darrick J. Wong, linux-xfs
Hi all,
this series has small cosmetics to the get/set reslks and fscounts
interface.
Diffstat:
xfs_fsops.c | 50 +-------------------------
xfs_fsops.h | 14 +++----
xfs_ioctl.c | 115 ++++++++++++++++++++++++++++++++----------------------------
xfs_mount.c | 8 +---
xfs_super.c | 6 +--
5 files changed, 75 insertions(+), 118 deletions(-)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/4] xfs: clean up the XFS_IOC_{GS}ET_RESBLKS handler
2023-11-26 13:01 misc cleanups to the resblks interfaces Christoph Hellwig
@ 2023-11-26 13:01 ` Christoph Hellwig
2023-11-28 1:53 ` Darrick J. Wong
2023-11-26 13:01 ` [PATCH 2/4] xfs: clean up the XFS_IOC_FSCOUNTS handler Christoph Hellwig
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Christoph Hellwig @ 2023-11-26 13:01 UTC (permalink / raw
To: Chandan Babu R; +Cc: Darrick J. Wong, linux-xfs
The XFS_IOC_GET_RESBLKS and XFS_IOC_SET_RESBLKS already share a fair
amount of code, and will share even more soon. Move the logic for both
of them out of the main xfs_file_ioctl function into a
xfs_ioctl_getset_resblocks helper to share the code and prepare for
additional changes.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_ioctl.c | 87 +++++++++++++++++++++++-----------------------
1 file changed, 43 insertions(+), 44 deletions(-)
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index a82470e027f727..8faaf2ef67a7b8 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1862,6 +1862,46 @@ xfs_fs_eofblocks_from_user(
return 0;
}
+static int
+xfs_ioctl_getset_resblocks(
+ struct file *filp,
+ unsigned int cmd,
+ void __user *arg)
+{
+ struct xfs_mount *mp = XFS_I(file_inode(filp))->i_mount;
+ struct xfs_fsop_resblks fsop = { };
+ int error;
+ uint64_t in;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ if (cmd == XFS_IOC_SET_RESBLKS) {
+ if (xfs_is_readonly(mp))
+ return -EROFS;
+
+ if (copy_from_user(&fsop, arg, sizeof(fsop)))
+ return -EFAULT;
+
+ error = mnt_want_write_file(filp);
+ if (error)
+ return error;
+ in = fsop.resblks;
+ error = xfs_reserve_blocks(mp, &in, &fsop);
+ mnt_drop_write_file(filp);
+ if (error)
+ return error;
+ } else {
+ error = xfs_reserve_blocks(mp, NULL, &fsop);
+ if (error)
+ return error;
+ }
+
+ if (copy_to_user(arg, &fsop, sizeof(fsop)))
+ return -EFAULT;
+ return 0;
+}
+
/*
* These long-unused ioctls were removed from the official ioctl API in 5.17,
* but retain these definitions so that we can log warnings about them.
@@ -2008,50 +2048,9 @@ xfs_file_ioctl(
return 0;
}
- case XFS_IOC_SET_RESBLKS: {
- xfs_fsop_resblks_t inout;
- uint64_t in;
-
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
-
- if (xfs_is_readonly(mp))
- return -EROFS;
-
- if (copy_from_user(&inout, arg, sizeof(inout)))
- return -EFAULT;
-
- error = mnt_want_write_file(filp);
- if (error)
- return error;
-
- /* input parameter is passed in resblks field of structure */
- in = inout.resblks;
- error = xfs_reserve_blocks(mp, &in, &inout);
- mnt_drop_write_file(filp);
- if (error)
- return error;
-
- if (copy_to_user(arg, &inout, sizeof(inout)))
- return -EFAULT;
- return 0;
- }
-
- case XFS_IOC_GET_RESBLKS: {
- xfs_fsop_resblks_t out;
-
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
-
- error = xfs_reserve_blocks(mp, NULL, &out);
- if (error)
- return error;
-
- if (copy_to_user(arg, &out, sizeof(out)))
- return -EFAULT;
-
- return 0;
- }
+ case XFS_IOC_SET_RESBLKS:
+ case XFS_IOC_GET_RESBLKS:
+ return xfs_ioctl_getset_resblocks(filp, cmd, arg);
case XFS_IOC_FSGROWFSDATA: {
struct xfs_growfs_data in;
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/4] xfs: clean up the XFS_IOC_FSCOUNTS handler
2023-11-26 13:01 misc cleanups to the resblks interfaces Christoph Hellwig
2023-11-26 13:01 ` [PATCH 1/4] xfs: clean up the XFS_IOC_{GS}ET_RESBLKS handler Christoph Hellwig
@ 2023-11-26 13:01 ` Christoph Hellwig
2023-11-27 10:19 ` kernel test robot
` (3 more replies)
2023-11-26 13:01 ` [PATCH 3/4] xfs: clean up the xfs_reserve_blocks interface Christoph Hellwig
2023-11-26 13:01 ` [PATCH 4/4] xfs: clean up xfs_fsops.h Christoph Hellwig
3 siblings, 4 replies; 14+ messages in thread
From: Christoph Hellwig @ 2023-11-26 13:01 UTC (permalink / raw
To: Chandan Babu R; +Cc: Darrick J. Wong, linux-xfs
Split XFS_IOC_FSCOUNTS out of the main xfs_file_ioctl function, and
merge the xfs_fs_counts helper into the ioctl handler.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_fsops.c | 16 ----------------
fs/xfs/xfs_fsops.h | 1 -
fs/xfs/xfs_ioctl.c | 29 ++++++++++++++++++++---------
3 files changed, 20 insertions(+), 26 deletions(-)
diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index 7cb75cb6b8e9b4..01681783e2c31a 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c
@@ -343,22 +343,6 @@ xfs_growfs_log(
return error;
}
-/*
- * exported through ioctl XFS_IOC_FSCOUNTS
- */
-
-void
-xfs_fs_counts(
- xfs_mount_t *mp,
- xfs_fsop_counts_t *cnt)
-{
- cnt->allocino = percpu_counter_read_positive(&mp->m_icount);
- cnt->freeino = percpu_counter_read_positive(&mp->m_ifree);
- cnt->freedata = percpu_counter_read_positive(&mp->m_fdblocks) -
- xfs_fdblocks_unavailable(mp);
- cnt->freertx = percpu_counter_read_positive(&mp->m_frextents);
-}
-
/*
* exported through ioctl XFS_IOC_SET_RESBLKS & XFS_IOC_GET_RESBLKS
*
diff --git a/fs/xfs/xfs_fsops.h b/fs/xfs/xfs_fsops.h
index 2cffe51a31e8b2..45f0cb6e805938 100644
--- a/fs/xfs/xfs_fsops.h
+++ b/fs/xfs/xfs_fsops.h
@@ -8,7 +8,6 @@
extern int xfs_growfs_data(struct xfs_mount *mp, struct xfs_growfs_data *in);
extern int xfs_growfs_log(struct xfs_mount *mp, struct xfs_growfs_log *in);
-extern void xfs_fs_counts(xfs_mount_t *mp, xfs_fsop_counts_t *cnt);
extern int xfs_reserve_blocks(xfs_mount_t *mp, uint64_t *inval,
xfs_fsop_resblks_t *outval);
extern int xfs_fs_goingdown(xfs_mount_t *mp, uint32_t inflags);
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 8faaf2ef67a7b8..c8e78c8101c65c 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1902,6 +1902,24 @@ xfs_ioctl_getset_resblocks(
return 0;
}
+static int
+xfs_ioctl_fs_counts(
+ struct xfs_mount *mp,
+ struct xfs_fsop_counts *uarg)
+{
+ struct xfs_fsop_counts out = {
+ .allocino = percpu_counter_read_positive(&mp->m_icount),
+ .freeino = percpu_counter_read_positive(&mp->m_ifree),
+ .freedata = percpu_counter_read_positive(&mp->m_fdblocks) -
+ xfs_fdblocks_unavailable(mp),
+ .freertx = percpu_counter_read_positive(&mp->m_frextents),
+ };
+
+ if (copy_to_user(uarg, &out, sizeof(out)))
+ return -EFAULT;
+ return 0;
+}
+
/*
* These long-unused ioctls were removed from the official ioctl API in 5.17,
* but retain these definitions so that we can log warnings about them.
@@ -2038,15 +2056,8 @@ xfs_file_ioctl(
return error;
}
- case XFS_IOC_FSCOUNTS: {
- xfs_fsop_counts_t out;
-
- xfs_fs_counts(mp, &out);
-
- if (copy_to_user(arg, &out, sizeof(out)))
- return -EFAULT;
- return 0;
- }
+ case XFS_IOC_FSCOUNTS:
+ return xfs_ioctl_fs_counts(mp, arg);
case XFS_IOC_SET_RESBLKS:
case XFS_IOC_GET_RESBLKS:
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/4] xfs: clean up the xfs_reserve_blocks interface
2023-11-26 13:01 misc cleanups to the resblks interfaces Christoph Hellwig
2023-11-26 13:01 ` [PATCH 1/4] xfs: clean up the XFS_IOC_{GS}ET_RESBLKS handler Christoph Hellwig
2023-11-26 13:01 ` [PATCH 2/4] xfs: clean up the XFS_IOC_FSCOUNTS handler Christoph Hellwig
@ 2023-11-26 13:01 ` Christoph Hellwig
2023-11-28 2:09 ` Darrick J. Wong
2023-11-26 13:01 ` [PATCH 4/4] xfs: clean up xfs_fsops.h Christoph Hellwig
3 siblings, 1 reply; 14+ messages in thread
From: Christoph Hellwig @ 2023-11-26 13:01 UTC (permalink / raw
To: Chandan Babu R; +Cc: Darrick J. Wong, linux-xfs
xfs_reserve_blocks has a very odd interface that can only be explained
by it directly deriving from the IRIX fcntl handler back in the day.
Split reporting out the reserved blocks out of xfs_reserve_blocks into
the only caller that cares. This means that the value reported from
XFS_IOC_SET_RESBLKS isn't atomically sampled in the same critical
section as when it was set anymore, but as the values could change
right after setting them anyway that does not matter. It does
provide atomic sampling of both values for XFS_IOC_GET_RESBLKS now,
though.
Also pass a normal scalar integer value for the requested value instead
of the pointless pointer.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_fsops.c | 34 +++-------------------------------
fs/xfs/xfs_fsops.h | 3 +--
fs/xfs/xfs_ioctl.c | 13 ++++++-------
fs/xfs/xfs_mount.c | 8 ++------
fs/xfs/xfs_super.c | 6 ++----
5 files changed, 14 insertions(+), 50 deletions(-)
diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index 01681783e2c31a..4f5da19142f298 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c
@@ -344,43 +344,20 @@ xfs_growfs_log(
}
/*
- * exported through ioctl XFS_IOC_SET_RESBLKS & XFS_IOC_GET_RESBLKS
- *
- * xfs_reserve_blocks is called to set m_resblks
- * in the in-core mount table. The number of unused reserved blocks
- * is kept in m_resblks_avail.
- *
* Reserve the requested number of blocks if available. Otherwise return
* as many as possible to satisfy the request. The actual number
- * reserved are returned in outval
- *
- * A null inval pointer indicates that only the current reserved blocks
- * available should be returned no settings are changed.
+ * reserved are returned in outval.
*/
-
int
xfs_reserve_blocks(
- xfs_mount_t *mp,
- uint64_t *inval,
- xfs_fsop_resblks_t *outval)
+ struct xfs_mount *mp,
+ uint64_t request)
{
int64_t lcounter, delta;
int64_t fdblks_delta = 0;
- uint64_t request;
int64_t free;
int error = 0;
- /* If inval is null, report current values and return */
- if (inval == (uint64_t *)NULL) {
- if (!outval)
- return -EINVAL;
- outval->resblks = mp->m_resblks;
- outval->resblks_avail = mp->m_resblks_avail;
- return 0;
- }
-
- request = *inval;
-
/*
* With per-cpu counters, this becomes an interesting problem. we need
* to work out if we are freeing or allocation blocks first, then we can
@@ -450,11 +427,6 @@ xfs_reserve_blocks(
spin_lock(&mp->m_sb_lock);
}
out:
- if (outval) {
- outval->resblks = mp->m_resblks;
- outval->resblks_avail = mp->m_resblks_avail;
- }
-
spin_unlock(&mp->m_sb_lock);
return error;
}
diff --git a/fs/xfs/xfs_fsops.h b/fs/xfs/xfs_fsops.h
index 45f0cb6e805938..7536f8a92746f6 100644
--- a/fs/xfs/xfs_fsops.h
+++ b/fs/xfs/xfs_fsops.h
@@ -8,8 +8,7 @@
extern int xfs_growfs_data(struct xfs_mount *mp, struct xfs_growfs_data *in);
extern int xfs_growfs_log(struct xfs_mount *mp, struct xfs_growfs_log *in);
-extern int xfs_reserve_blocks(xfs_mount_t *mp, uint64_t *inval,
- xfs_fsop_resblks_t *outval);
+int xfs_reserve_blocks(struct xfs_mount *mp, uint64_t request);
extern int xfs_fs_goingdown(xfs_mount_t *mp, uint32_t inflags);
extern int xfs_fs_reserve_ag_blocks(struct xfs_mount *mp);
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index c8e78c8101c65c..812efb7923abb1 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1871,7 +1871,6 @@ xfs_ioctl_getset_resblocks(
struct xfs_mount *mp = XFS_I(file_inode(filp))->i_mount;
struct xfs_fsop_resblks fsop = { };
int error;
- uint64_t in;
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
@@ -1886,17 +1885,17 @@ xfs_ioctl_getset_resblocks(
error = mnt_want_write_file(filp);
if (error)
return error;
- in = fsop.resblks;
- error = xfs_reserve_blocks(mp, &in, &fsop);
+ error = xfs_reserve_blocks(mp, fsop.resblks);
mnt_drop_write_file(filp);
if (error)
return error;
- } else {
- error = xfs_reserve_blocks(mp, NULL, &fsop);
- if (error)
- return error;
}
+ spin_lock(&mp->m_sb_lock);
+ fsop.resblks = mp->m_resblks;
+ fsop.resblks_avail = mp->m_resblks_avail;
+ spin_unlock(&mp->m_sb_lock);
+
if (copy_to_user(arg, &fsop, sizeof(fsop)))
return -EFAULT;
return 0;
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index aed5be5508fe57..aabb25dc3efab2 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -637,7 +637,6 @@ xfs_mountfs(
struct xfs_sb *sbp = &(mp->m_sb);
struct xfs_inode *rip;
struct xfs_ino_geometry *igeo = M_IGEO(mp);
- uint64_t resblks;
uint quotamount = 0;
uint quotaflags = 0;
int error = 0;
@@ -974,8 +973,7 @@ xfs_mountfs(
* we were already there on the last unmount. Warn if this occurs.
*/
if (!xfs_is_readonly(mp)) {
- resblks = xfs_default_resblks(mp);
- error = xfs_reserve_blocks(mp, &resblks, NULL);
+ error = xfs_reserve_blocks(mp, xfs_default_resblks(mp));
if (error)
xfs_warn(mp,
"Unable to allocate reserve blocks. Continuing without reserve pool.");
@@ -1053,7 +1051,6 @@ void
xfs_unmountfs(
struct xfs_mount *mp)
{
- uint64_t resblks;
int error;
/*
@@ -1090,8 +1087,7 @@ xfs_unmountfs(
* we only every apply deltas to the superblock and hence the incore
* value does not matter....
*/
- resblks = 0;
- error = xfs_reserve_blocks(mp, &resblks, NULL);
+ error = xfs_reserve_blocks(mp, 0);
if (error)
xfs_warn(mp, "Unable to free reserved block pool. "
"Freespace may not be correct on next mount.");
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 764304595e8b00..d0009430a62778 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -906,10 +906,8 @@ xfs_fs_statfs(
STATIC void
xfs_save_resvblks(struct xfs_mount *mp)
{
- uint64_t resblks = 0;
-
mp->m_resblks_save = mp->m_resblks;
- xfs_reserve_blocks(mp, &resblks, NULL);
+ xfs_reserve_blocks(mp, 0);
}
STATIC void
@@ -923,7 +921,7 @@ xfs_restore_resvblks(struct xfs_mount *mp)
} else
resblks = xfs_default_resblks(mp);
- xfs_reserve_blocks(mp, &resblks, NULL);
+ xfs_reserve_blocks(mp, resblks);
}
/*
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/4] xfs: clean up xfs_fsops.h
2023-11-26 13:01 misc cleanups to the resblks interfaces Christoph Hellwig
` (2 preceding siblings ...)
2023-11-26 13:01 ` [PATCH 3/4] xfs: clean up the xfs_reserve_blocks interface Christoph Hellwig
@ 2023-11-26 13:01 ` Christoph Hellwig
2023-11-28 1:51 ` Darrick J. Wong
3 siblings, 1 reply; 14+ messages in thread
From: Christoph Hellwig @ 2023-11-26 13:01 UTC (permalink / raw
To: Chandan Babu R; +Cc: Darrick J. Wong, linux-xfs
Use struct types instead of typedefs so that the header can be included
with pulling in the headers that define the typedefs, and remove the
pointless externs.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_fsops.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/xfs/xfs_fsops.h b/fs/xfs/xfs_fsops.h
index 7536f8a92746f6..44457b0a059376 100644
--- a/fs/xfs/xfs_fsops.h
+++ b/fs/xfs/xfs_fsops.h
@@ -6,12 +6,12 @@
#ifndef __XFS_FSOPS_H__
#define __XFS_FSOPS_H__
-extern int xfs_growfs_data(struct xfs_mount *mp, struct xfs_growfs_data *in);
-extern int xfs_growfs_log(struct xfs_mount *mp, struct xfs_growfs_log *in);
+int xfs_growfs_data(struct xfs_mount *mp, struct xfs_growfs_data *in);
+int xfs_growfs_log(struct xfs_mount *mp, struct xfs_growfs_log *in);
int xfs_reserve_blocks(struct xfs_mount *mp, uint64_t request);
-extern int xfs_fs_goingdown(xfs_mount_t *mp, uint32_t inflags);
+int xfs_fs_goingdown(struct xfs_mount *mp, uint32_t inflags);
-extern int xfs_fs_reserve_ag_blocks(struct xfs_mount *mp);
-extern int xfs_fs_unreserve_ag_blocks(struct xfs_mount *mp);
+int xfs_fs_reserve_ag_blocks(struct xfs_mount *mp);
+int xfs_fs_unreserve_ag_blocks(struct xfs_mount *mp);
#endif /* __XFS_FSOPS_H__ */
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] xfs: clean up the XFS_IOC_FSCOUNTS handler
2023-11-26 13:01 ` [PATCH 2/4] xfs: clean up the XFS_IOC_FSCOUNTS handler Christoph Hellwig
@ 2023-11-27 10:19 ` kernel test robot
2023-11-27 16:40 ` Christoph Hellwig
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2023-11-27 10:19 UTC (permalink / raw
To: Christoph Hellwig; +Cc: oe-kbuild-all
Hi Christoph,
kernel test robot noticed the following build warnings:
[auto build test WARNING on xfs-linux/for-next]
[also build test WARNING on linus/master v6.7-rc3 next-20231127]
[cannot apply to djwong-xfs/djwong-devel]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Christoph-Hellwig/xfs-clean-up-the-XFS_IOC_FSCOUNTS-handler/20231126-220129
base: https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git for-next
patch link: https://lore.kernel.org/r/20231126130124.1251467-3-hch%40lst.de
patch subject: [PATCH 2/4] xfs: clean up the XFS_IOC_FSCOUNTS handler
config: i386-randconfig-062-20231126 (https://download.01.org/0day-ci/archive/20231127/202311271644.IMCBnHFv-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231127/202311271644.IMCBnHFv-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311271644.IMCBnHFv-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> fs/xfs/xfs_ioctl.c:1918:26: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void [noderef] __user *to @@ got struct xfs_fsop_counts *uarg @@
fs/xfs/xfs_ioctl.c:1918:26: sparse: expected void [noderef] __user *to
fs/xfs/xfs_ioctl.c:1918:26: sparse: got struct xfs_fsop_counts *uarg
>> fs/xfs/xfs_ioctl.c:2060:48: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected struct xfs_fsop_counts *uarg @@ got void [noderef] __user *arg @@
fs/xfs/xfs_ioctl.c:2060:48: sparse: expected struct xfs_fsop_counts *uarg
fs/xfs/xfs_ioctl.c:2060:48: sparse: got void [noderef] __user *arg
vim +1918 fs/xfs/xfs_ioctl.c
1904
1905 static int
1906 xfs_ioctl_fs_counts(
1907 struct xfs_mount *mp,
1908 struct xfs_fsop_counts *uarg)
1909 {
1910 struct xfs_fsop_counts out = {
1911 .allocino = percpu_counter_read_positive(&mp->m_icount),
1912 .freeino = percpu_counter_read_positive(&mp->m_ifree),
1913 .freedata = percpu_counter_read_positive(&mp->m_fdblocks) -
1914 xfs_fdblocks_unavailable(mp),
1915 .freertx = percpu_counter_read_positive(&mp->m_frextents),
1916 };
1917
> 1918 if (copy_to_user(uarg, &out, sizeof(out)))
1919 return -EFAULT;
1920 return 0;
1921 }
1922
1923 /*
1924 * These long-unused ioctls were removed from the official ioctl API in 5.17,
1925 * but retain these definitions so that we can log warnings about them.
1926 */
1927 #define XFS_IOC_ALLOCSP _IOW ('X', 10, struct xfs_flock64)
1928 #define XFS_IOC_FREESP _IOW ('X', 11, struct xfs_flock64)
1929 #define XFS_IOC_ALLOCSP64 _IOW ('X', 36, struct xfs_flock64)
1930 #define XFS_IOC_FREESP64 _IOW ('X', 37, struct xfs_flock64)
1931
1932 /*
1933 * Note: some of the ioctl's return positive numbers as a
1934 * byte count indicating success, such as readlink_by_handle.
1935 * So we don't "sign flip" like most other routines. This means
1936 * true errors need to be returned as a negative value.
1937 */
1938 long
1939 xfs_file_ioctl(
1940 struct file *filp,
1941 unsigned int cmd,
1942 unsigned long p)
1943 {
1944 struct inode *inode = file_inode(filp);
1945 struct xfs_inode *ip = XFS_I(inode);
1946 struct xfs_mount *mp = ip->i_mount;
1947 void __user *arg = (void __user *)p;
1948 int error;
1949
1950 trace_xfs_file_ioctl(ip);
1951
1952 switch (cmd) {
1953 case FITRIM:
1954 return xfs_ioc_trim(mp, arg);
1955 case FS_IOC_GETFSLABEL:
1956 return xfs_ioc_getlabel(mp, arg);
1957 case FS_IOC_SETFSLABEL:
1958 return xfs_ioc_setlabel(filp, mp, arg);
1959 case XFS_IOC_ALLOCSP:
1960 case XFS_IOC_FREESP:
1961 case XFS_IOC_ALLOCSP64:
1962 case XFS_IOC_FREESP64:
1963 xfs_warn_once(mp,
1964 "%s should use fallocate; XFS_IOC_{ALLOC,FREE}SP ioctl unsupported",
1965 current->comm);
1966 return -ENOTTY;
1967 case XFS_IOC_DIOINFO: {
1968 struct xfs_buftarg *target = xfs_inode_buftarg(ip);
1969 struct dioattr da;
1970
1971 da.d_mem = da.d_miniosz = target->bt_logical_sectorsize;
1972 da.d_maxiosz = INT_MAX & ~(da.d_miniosz - 1);
1973
1974 if (copy_to_user(arg, &da, sizeof(da)))
1975 return -EFAULT;
1976 return 0;
1977 }
1978
1979 case XFS_IOC_FSBULKSTAT_SINGLE:
1980 case XFS_IOC_FSBULKSTAT:
1981 case XFS_IOC_FSINUMBERS:
1982 return xfs_ioc_fsbulkstat(filp, cmd, arg);
1983
1984 case XFS_IOC_BULKSTAT:
1985 return xfs_ioc_bulkstat(filp, cmd, arg);
1986 case XFS_IOC_INUMBERS:
1987 return xfs_ioc_inumbers(mp, cmd, arg);
1988
1989 case XFS_IOC_FSGEOMETRY_V1:
1990 return xfs_ioc_fsgeometry(mp, arg, 3);
1991 case XFS_IOC_FSGEOMETRY_V4:
1992 return xfs_ioc_fsgeometry(mp, arg, 4);
1993 case XFS_IOC_FSGEOMETRY:
1994 return xfs_ioc_fsgeometry(mp, arg, 5);
1995
1996 case XFS_IOC_AG_GEOMETRY:
1997 return xfs_ioc_ag_geometry(mp, arg);
1998
1999 case XFS_IOC_GETVERSION:
2000 return put_user(inode->i_generation, (int __user *)arg);
2001
2002 case XFS_IOC_FSGETXATTRA:
2003 return xfs_ioc_fsgetxattra(ip, arg);
2004
2005 case XFS_IOC_GETBMAP:
2006 case XFS_IOC_GETBMAPA:
2007 case XFS_IOC_GETBMAPX:
2008 return xfs_ioc_getbmap(filp, cmd, arg);
2009
2010 case FS_IOC_GETFSMAP:
2011 return xfs_ioc_getfsmap(ip, arg);
2012
2013 case XFS_IOC_SCRUB_METADATA:
2014 return xfs_ioc_scrub_metadata(filp, arg);
2015
2016 case XFS_IOC_FD_TO_HANDLE:
2017 case XFS_IOC_PATH_TO_HANDLE:
2018 case XFS_IOC_PATH_TO_FSHANDLE: {
2019 xfs_fsop_handlereq_t hreq;
2020
2021 if (copy_from_user(&hreq, arg, sizeof(hreq)))
2022 return -EFAULT;
2023 return xfs_find_handle(cmd, &hreq);
2024 }
2025 case XFS_IOC_OPEN_BY_HANDLE: {
2026 xfs_fsop_handlereq_t hreq;
2027
2028 if (copy_from_user(&hreq, arg, sizeof(xfs_fsop_handlereq_t)))
2029 return -EFAULT;
2030 return xfs_open_by_handle(filp, &hreq);
2031 }
2032
2033 case XFS_IOC_READLINK_BY_HANDLE: {
2034 xfs_fsop_handlereq_t hreq;
2035
2036 if (copy_from_user(&hreq, arg, sizeof(xfs_fsop_handlereq_t)))
2037 return -EFAULT;
2038 return xfs_readlink_by_handle(filp, &hreq);
2039 }
2040 case XFS_IOC_ATTRLIST_BY_HANDLE:
2041 return xfs_attrlist_by_handle(filp, arg);
2042
2043 case XFS_IOC_ATTRMULTI_BY_HANDLE:
2044 return xfs_attrmulti_by_handle(filp, arg);
2045
2046 case XFS_IOC_SWAPEXT: {
2047 struct xfs_swapext sxp;
2048
2049 if (copy_from_user(&sxp, arg, sizeof(xfs_swapext_t)))
2050 return -EFAULT;
2051 error = mnt_want_write_file(filp);
2052 if (error)
2053 return error;
2054 error = xfs_ioc_swapext(&sxp);
2055 mnt_drop_write_file(filp);
2056 return error;
2057 }
2058
2059 case XFS_IOC_FSCOUNTS:
> 2060 return xfs_ioctl_fs_counts(mp, arg);
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] xfs: clean up the XFS_IOC_FSCOUNTS handler
2023-11-26 13:01 ` [PATCH 2/4] xfs: clean up the XFS_IOC_FSCOUNTS handler Christoph Hellwig
2023-11-27 10:19 ` kernel test robot
@ 2023-11-27 16:40 ` Christoph Hellwig
2023-11-28 1:58 ` Darrick J. Wong
2023-12-03 6:31 ` kernel test robot
3 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2023-11-27 16:40 UTC (permalink / raw
To: Christoph Hellwig; +Cc: Chandan Babu R, Darrick J. Wong, linux-xfs
> +static int
> +xfs_ioctl_fs_counts(
> + struct xfs_mount *mp,
> + struct xfs_fsop_counts *uarg)
FYI, the buildbot (rightly) complained about a missing __user
annotation for sparse here.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] xfs: clean up xfs_fsops.h
2023-11-26 13:01 ` [PATCH 4/4] xfs: clean up xfs_fsops.h Christoph Hellwig
@ 2023-11-28 1:51 ` Darrick J. Wong
0 siblings, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2023-11-28 1:51 UTC (permalink / raw
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Sun, Nov 26, 2023 at 02:01:24PM +0100, Christoph Hellwig wrote:
> Use struct types instead of typedefs so that the header can be included
> with pulling in the headers that define the typedefs, and remove the
> pointless externs.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Easy review!
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> fs/xfs/xfs_fsops.h | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/xfs/xfs_fsops.h b/fs/xfs/xfs_fsops.h
> index 7536f8a92746f6..44457b0a059376 100644
> --- a/fs/xfs/xfs_fsops.h
> +++ b/fs/xfs/xfs_fsops.h
> @@ -6,12 +6,12 @@
> #ifndef __XFS_FSOPS_H__
> #define __XFS_FSOPS_H__
>
> -extern int xfs_growfs_data(struct xfs_mount *mp, struct xfs_growfs_data *in);
> -extern int xfs_growfs_log(struct xfs_mount *mp, struct xfs_growfs_log *in);
> +int xfs_growfs_data(struct xfs_mount *mp, struct xfs_growfs_data *in);
> +int xfs_growfs_log(struct xfs_mount *mp, struct xfs_growfs_log *in);
> int xfs_reserve_blocks(struct xfs_mount *mp, uint64_t request);
> -extern int xfs_fs_goingdown(xfs_mount_t *mp, uint32_t inflags);
> +int xfs_fs_goingdown(struct xfs_mount *mp, uint32_t inflags);
>
> -extern int xfs_fs_reserve_ag_blocks(struct xfs_mount *mp);
> -extern int xfs_fs_unreserve_ag_blocks(struct xfs_mount *mp);
> +int xfs_fs_reserve_ag_blocks(struct xfs_mount *mp);
> +int xfs_fs_unreserve_ag_blocks(struct xfs_mount *mp);
>
> #endif /* __XFS_FSOPS_H__ */
> --
> 2.39.2
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] xfs: clean up the XFS_IOC_{GS}ET_RESBLKS handler
2023-11-26 13:01 ` [PATCH 1/4] xfs: clean up the XFS_IOC_{GS}ET_RESBLKS handler Christoph Hellwig
@ 2023-11-28 1:53 ` Darrick J. Wong
0 siblings, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2023-11-28 1:53 UTC (permalink / raw
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Sun, Nov 26, 2023 at 02:01:21PM +0100, Christoph Hellwig wrote:
> The XFS_IOC_GET_RESBLKS and XFS_IOC_SET_RESBLKS already share a fair
> amount of code, and will share even more soon. Move the logic for both
> of them out of the main xfs_file_ioctl function into a
> xfs_ioctl_getset_resblocks helper to share the code and prepare for
> additional changes.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/xfs_ioctl.c | 87 +++++++++++++++++++++++-----------------------
> 1 file changed, 43 insertions(+), 44 deletions(-)
>
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index a82470e027f727..8faaf2ef67a7b8 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1862,6 +1862,46 @@ xfs_fs_eofblocks_from_user(
> return 0;
> }
>
> +static int
> +xfs_ioctl_getset_resblocks(
> + struct file *filp,
> + unsigned int cmd,
> + void __user *arg)
> +{
> + struct xfs_mount *mp = XFS_I(file_inode(filp))->i_mount;
> + struct xfs_fsop_resblks fsop = { };
> + int error;
> + uint64_t in;
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + if (cmd == XFS_IOC_SET_RESBLKS) {
> + if (xfs_is_readonly(mp))
> + return -EROFS;
> +
> + if (copy_from_user(&fsop, arg, sizeof(fsop)))
> + return -EFAULT;
> +
> + error = mnt_want_write_file(filp);
> + if (error)
> + return error;
> + in = fsop.resblks;
> + error = xfs_reserve_blocks(mp, &in, &fsop);
<shudder> what an interface...
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> + mnt_drop_write_file(filp);
> + if (error)
> + return error;
> + } else {
> + error = xfs_reserve_blocks(mp, NULL, &fsop);
> + if (error)
> + return error;
> + }
> +
> + if (copy_to_user(arg, &fsop, sizeof(fsop)))
> + return -EFAULT;
> + return 0;
> +}
> +
> /*
> * These long-unused ioctls were removed from the official ioctl API in 5.17,
> * but retain these definitions so that we can log warnings about them.
> @@ -2008,50 +2048,9 @@ xfs_file_ioctl(
> return 0;
> }
>
> - case XFS_IOC_SET_RESBLKS: {
> - xfs_fsop_resblks_t inout;
> - uint64_t in;
> -
> - if (!capable(CAP_SYS_ADMIN))
> - return -EPERM;
> -
> - if (xfs_is_readonly(mp))
> - return -EROFS;
> -
> - if (copy_from_user(&inout, arg, sizeof(inout)))
> - return -EFAULT;
> -
> - error = mnt_want_write_file(filp);
> - if (error)
> - return error;
> -
> - /* input parameter is passed in resblks field of structure */
> - in = inout.resblks;
> - error = xfs_reserve_blocks(mp, &in, &inout);
> - mnt_drop_write_file(filp);
> - if (error)
> - return error;
> -
> - if (copy_to_user(arg, &inout, sizeof(inout)))
> - return -EFAULT;
> - return 0;
> - }
> -
> - case XFS_IOC_GET_RESBLKS: {
> - xfs_fsop_resblks_t out;
> -
> - if (!capable(CAP_SYS_ADMIN))
> - return -EPERM;
> -
> - error = xfs_reserve_blocks(mp, NULL, &out);
> - if (error)
> - return error;
> -
> - if (copy_to_user(arg, &out, sizeof(out)))
> - return -EFAULT;
> -
> - return 0;
> - }
> + case XFS_IOC_SET_RESBLKS:
> + case XFS_IOC_GET_RESBLKS:
> + return xfs_ioctl_getset_resblocks(filp, cmd, arg);
>
> case XFS_IOC_FSGROWFSDATA: {
> struct xfs_growfs_data in;
> --
> 2.39.2
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] xfs: clean up the XFS_IOC_FSCOUNTS handler
2023-11-26 13:01 ` [PATCH 2/4] xfs: clean up the XFS_IOC_FSCOUNTS handler Christoph Hellwig
2023-11-27 10:19 ` kernel test robot
2023-11-27 16:40 ` Christoph Hellwig
@ 2023-11-28 1:58 ` Darrick J. Wong
2023-11-28 5:30 ` Christoph Hellwig
2023-12-03 6:31 ` kernel test robot
3 siblings, 1 reply; 14+ messages in thread
From: Darrick J. Wong @ 2023-11-28 1:58 UTC (permalink / raw
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Sun, Nov 26, 2023 at 02:01:22PM +0100, Christoph Hellwig wrote:
> Split XFS_IOC_FSCOUNTS out of the main xfs_file_ioctl function, and
> merge the xfs_fs_counts helper into the ioctl handler.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/xfs_fsops.c | 16 ----------------
> fs/xfs/xfs_fsops.h | 1 -
> fs/xfs/xfs_ioctl.c | 29 ++++++++++++++++++++---------
> 3 files changed, 20 insertions(+), 26 deletions(-)
>
> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> index 7cb75cb6b8e9b4..01681783e2c31a 100644
> --- a/fs/xfs/xfs_fsops.c
> +++ b/fs/xfs/xfs_fsops.c
> @@ -343,22 +343,6 @@ xfs_growfs_log(
> return error;
> }
>
> -/*
> - * exported through ioctl XFS_IOC_FSCOUNTS
> - */
> -
> -void
> -xfs_fs_counts(
> - xfs_mount_t *mp,
> - xfs_fsop_counts_t *cnt)
> -{
> - cnt->allocino = percpu_counter_read_positive(&mp->m_icount);
> - cnt->freeino = percpu_counter_read_positive(&mp->m_ifree);
> - cnt->freedata = percpu_counter_read_positive(&mp->m_fdblocks) -
> - xfs_fdblocks_unavailable(mp);
> - cnt->freertx = percpu_counter_read_positive(&mp->m_frextents);
> -}
> -
> /*
> * exported through ioctl XFS_IOC_SET_RESBLKS & XFS_IOC_GET_RESBLKS
> *
> diff --git a/fs/xfs/xfs_fsops.h b/fs/xfs/xfs_fsops.h
> index 2cffe51a31e8b2..45f0cb6e805938 100644
> --- a/fs/xfs/xfs_fsops.h
> +++ b/fs/xfs/xfs_fsops.h
> @@ -8,7 +8,6 @@
>
> extern int xfs_growfs_data(struct xfs_mount *mp, struct xfs_growfs_data *in);
> extern int xfs_growfs_log(struct xfs_mount *mp, struct xfs_growfs_log *in);
> -extern void xfs_fs_counts(xfs_mount_t *mp, xfs_fsop_counts_t *cnt);
> extern int xfs_reserve_blocks(xfs_mount_t *mp, uint64_t *inval,
> xfs_fsop_resblks_t *outval);
> extern int xfs_fs_goingdown(xfs_mount_t *mp, uint32_t inflags);
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 8faaf2ef67a7b8..c8e78c8101c65c 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1902,6 +1902,24 @@ xfs_ioctl_getset_resblocks(
> return 0;
> }
>
> +static int
> +xfs_ioctl_fs_counts(
> + struct xfs_mount *mp,
> + struct xfs_fsop_counts *uarg)
> +{
> + struct xfs_fsop_counts out = {
> + .allocino = percpu_counter_read_positive(&mp->m_icount),
> + .freeino = percpu_counter_read_positive(&mp->m_ifree),
> + .freedata = percpu_counter_read_positive(&mp->m_fdblocks) -
> + xfs_fdblocks_unavailable(mp),
> + .freertx = percpu_counter_read_positive(&mp->m_frextents),
> + };
struct xfs_fsop_counts out = {
.allocino = percpu_counter_read_positive(&mp->m_icount),
.freeino = percpu_counter_read_positive(&mp->m_ifree),
.freedata = percpu_counter_read_positive(&mp->m_fdblocks) -
xfs_fdblocks_unavailable(mp),
.freertx = percpu_counter_read_positive(&mp->m_frextents),
};
Nit: Would you mind lining up the columns?
Otherwise looks good to me,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> +
> + if (copy_to_user(uarg, &out, sizeof(out)))
> + return -EFAULT;
> + return 0;
> +}
> +
> /*
> * These long-unused ioctls were removed from the official ioctl API in 5.17,
> * but retain these definitions so that we can log warnings about them.
> @@ -2038,15 +2056,8 @@ xfs_file_ioctl(
> return error;
> }
>
> - case XFS_IOC_FSCOUNTS: {
> - xfs_fsop_counts_t out;
> -
> - xfs_fs_counts(mp, &out);
> -
> - if (copy_to_user(arg, &out, sizeof(out)))
> - return -EFAULT;
> - return 0;
> - }
> + case XFS_IOC_FSCOUNTS:
> + return xfs_ioctl_fs_counts(mp, arg);
>
> case XFS_IOC_SET_RESBLKS:
> case XFS_IOC_GET_RESBLKS:
> --
> 2.39.2
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] xfs: clean up the xfs_reserve_blocks interface
2023-11-26 13:01 ` [PATCH 3/4] xfs: clean up the xfs_reserve_blocks interface Christoph Hellwig
@ 2023-11-28 2:09 ` Darrick J. Wong
2023-11-28 5:32 ` Christoph Hellwig
0 siblings, 1 reply; 14+ messages in thread
From: Darrick J. Wong @ 2023-11-28 2:09 UTC (permalink / raw
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Sun, Nov 26, 2023 at 02:01:23PM +0100, Christoph Hellwig wrote:
> xfs_reserve_blocks has a very odd interface that can only be explained
> by it directly deriving from the IRIX fcntl handler back in the day.
>
> Split reporting out the reserved blocks out of xfs_reserve_blocks into
> the only caller that cares. This means that the value reported from
> XFS_IOC_SET_RESBLKS isn't atomically sampled in the same critical
> section as when it was set anymore, but as the values could change
That wasn't true for the case of increasing the reserve before this
patch either. :)
> right after setting them anyway that does not matter. It does
> provide atomic sampling of both values for XFS_IOC_GET_RESBLKS now,
> though.
Hey, that's neat!
> Also pass a normal scalar integer value for the requested value instead
> of the pointless pointer.
Guh. What a calling convention.
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/xfs_fsops.c | 34 +++-------------------------------
> fs/xfs/xfs_fsops.h | 3 +--
> fs/xfs/xfs_ioctl.c | 13 ++++++-------
> fs/xfs/xfs_mount.c | 8 ++------
> fs/xfs/xfs_super.c | 6 ++----
> 5 files changed, 14 insertions(+), 50 deletions(-)
>
> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> index 01681783e2c31a..4f5da19142f298 100644
> --- a/fs/xfs/xfs_fsops.c
> +++ b/fs/xfs/xfs_fsops.c
> @@ -344,43 +344,20 @@ xfs_growfs_log(
> }
>
> /*
> - * exported through ioctl XFS_IOC_SET_RESBLKS & XFS_IOC_GET_RESBLKS
> - *
> - * xfs_reserve_blocks is called to set m_resblks
> - * in the in-core mount table. The number of unused reserved blocks
> - * is kept in m_resblks_avail.
> - *
> * Reserve the requested number of blocks if available. Otherwise return
> * as many as possible to satisfy the request. The actual number
> - * reserved are returned in outval
> - *
> - * A null inval pointer indicates that only the current reserved blocks
> - * available should be returned no settings are changed.
> + * reserved are returned in outval.
> */
> -
> int
> xfs_reserve_blocks(
> - xfs_mount_t *mp,
> - uint64_t *inval,
> - xfs_fsop_resblks_t *outval)
> + struct xfs_mount *mp,
> + uint64_t request)
> {
> int64_t lcounter, delta;
> int64_t fdblks_delta = 0;
> - uint64_t request;
> int64_t free;
> int error = 0;
>
> - /* If inval is null, report current values and return */
> - if (inval == (uint64_t *)NULL) {
> - if (!outval)
> - return -EINVAL;
> - outval->resblks = mp->m_resblks;
> - outval->resblks_avail = mp->m_resblks_avail;
> - return 0;
> - }
> -
> - request = *inval;
> -
> /*
> * With per-cpu counters, this becomes an interesting problem. we need
> * to work out if we are freeing or allocation blocks first, then we can
> @@ -450,11 +427,6 @@ xfs_reserve_blocks(
> spin_lock(&mp->m_sb_lock);
> }
> out:
> - if (outval) {
> - outval->resblks = mp->m_resblks;
> - outval->resblks_avail = mp->m_resblks_avail;
> - }
> -
> spin_unlock(&mp->m_sb_lock);
> return error;
> }
> diff --git a/fs/xfs/xfs_fsops.h b/fs/xfs/xfs_fsops.h
> index 45f0cb6e805938..7536f8a92746f6 100644
> --- a/fs/xfs/xfs_fsops.h
> +++ b/fs/xfs/xfs_fsops.h
> @@ -8,8 +8,7 @@
>
> extern int xfs_growfs_data(struct xfs_mount *mp, struct xfs_growfs_data *in);
> extern int xfs_growfs_log(struct xfs_mount *mp, struct xfs_growfs_log *in);
> -extern int xfs_reserve_blocks(xfs_mount_t *mp, uint64_t *inval,
> - xfs_fsop_resblks_t *outval);
> +int xfs_reserve_blocks(struct xfs_mount *mp, uint64_t request);
> extern int xfs_fs_goingdown(xfs_mount_t *mp, uint32_t inflags);
>
> extern int xfs_fs_reserve_ag_blocks(struct xfs_mount *mp);
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index c8e78c8101c65c..812efb7923abb1 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1871,7 +1871,6 @@ xfs_ioctl_getset_resblocks(
> struct xfs_mount *mp = XFS_I(file_inode(filp))->i_mount;
> struct xfs_fsop_resblks fsop = { };
> int error;
> - uint64_t in;
>
> if (!capable(CAP_SYS_ADMIN))
> return -EPERM;
> @@ -1886,17 +1885,17 @@ xfs_ioctl_getset_resblocks(
> error = mnt_want_write_file(filp);
> if (error)
> return error;
> - in = fsop.resblks;
> - error = xfs_reserve_blocks(mp, &in, &fsop);
> + error = xfs_reserve_blocks(mp, fsop.resblks);
> mnt_drop_write_file(filp);
> if (error)
> return error;
> - } else {
> - error = xfs_reserve_blocks(mp, NULL, &fsop);
> - if (error)
> - return error;
> }
>
> + spin_lock(&mp->m_sb_lock);
> + fsop.resblks = mp->m_resblks;
> + fsop.resblks_avail = mp->m_resblks_avail;
> + spin_unlock(&mp->m_sb_lock);
Hm. I sorta preferred keeping these details hidden in xfs_fsops.c
rather than scattering them around and lengthening xfs_ioctl.c, but
I think the calling convention cleanup is worthy enough for:
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> +
> if (copy_to_user(arg, &fsop, sizeof(fsop)))
> return -EFAULT;
> return 0;
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index aed5be5508fe57..aabb25dc3efab2 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -637,7 +637,6 @@ xfs_mountfs(
> struct xfs_sb *sbp = &(mp->m_sb);
> struct xfs_inode *rip;
> struct xfs_ino_geometry *igeo = M_IGEO(mp);
> - uint64_t resblks;
> uint quotamount = 0;
> uint quotaflags = 0;
> int error = 0;
> @@ -974,8 +973,7 @@ xfs_mountfs(
> * we were already there on the last unmount. Warn if this occurs.
> */
> if (!xfs_is_readonly(mp)) {
> - resblks = xfs_default_resblks(mp);
> - error = xfs_reserve_blocks(mp, &resblks, NULL);
> + error = xfs_reserve_blocks(mp, xfs_default_resblks(mp));
> if (error)
> xfs_warn(mp,
> "Unable to allocate reserve blocks. Continuing without reserve pool.");
> @@ -1053,7 +1051,6 @@ void
> xfs_unmountfs(
> struct xfs_mount *mp)
> {
> - uint64_t resblks;
> int error;
>
> /*
> @@ -1090,8 +1087,7 @@ xfs_unmountfs(
> * we only every apply deltas to the superblock and hence the incore
> * value does not matter....
> */
> - resblks = 0;
> - error = xfs_reserve_blocks(mp, &resblks, NULL);
> + error = xfs_reserve_blocks(mp, 0);
> if (error)
> xfs_warn(mp, "Unable to free reserved block pool. "
> "Freespace may not be correct on next mount.");
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 764304595e8b00..d0009430a62778 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -906,10 +906,8 @@ xfs_fs_statfs(
> STATIC void
> xfs_save_resvblks(struct xfs_mount *mp)
> {
> - uint64_t resblks = 0;
> -
> mp->m_resblks_save = mp->m_resblks;
> - xfs_reserve_blocks(mp, &resblks, NULL);
> + xfs_reserve_blocks(mp, 0);
> }
>
> STATIC void
> @@ -923,7 +921,7 @@ xfs_restore_resvblks(struct xfs_mount *mp)
> } else
> resblks = xfs_default_resblks(mp);
>
> - xfs_reserve_blocks(mp, &resblks, NULL);
> + xfs_reserve_blocks(mp, resblks);
> }
>
> /*
> --
> 2.39.2
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] xfs: clean up the XFS_IOC_FSCOUNTS handler
2023-11-28 1:58 ` Darrick J. Wong
@ 2023-11-28 5:30 ` Christoph Hellwig
0 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2023-11-28 5:30 UTC (permalink / raw
To: Darrick J. Wong; +Cc: Christoph Hellwig, Chandan Babu R, linux-xfs
On Mon, Nov 27, 2023 at 05:58:12PM -0800, Darrick J. Wong wrote:
> struct xfs_fsop_counts out = {
> .allocino = percpu_counter_read_positive(&mp->m_icount),
> .freeino = percpu_counter_read_positive(&mp->m_ifree),
> .freedata = percpu_counter_read_positive(&mp->m_fdblocks) -
> xfs_fdblocks_unavailable(mp),
> .freertx = percpu_counter_read_positive(&mp->m_frextents),
> };
>
> Nit: Would you mind lining up the columns?
Sure. I need to respin for the __user annotation anyway.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] xfs: clean up the xfs_reserve_blocks interface
2023-11-28 2:09 ` Darrick J. Wong
@ 2023-11-28 5:32 ` Christoph Hellwig
0 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2023-11-28 5:32 UTC (permalink / raw
To: Darrick J. Wong; +Cc: Christoph Hellwig, Chandan Babu R, linux-xfs
On Mon, Nov 27, 2023 at 06:09:33PM -0800, Darrick J. Wong wrote:
> > + spin_lock(&mp->m_sb_lock);
> > + fsop.resblks = mp->m_resblks;
> > + fsop.resblks_avail = mp->m_resblks_avail;
> > + spin_unlock(&mp->m_sb_lock);
>
> Hm. I sorta preferred keeping these details hidden in xfs_fsops.c
> rather than scattering them around and lengthening xfs_ioctl.c, but
> I think the calling convention cleanup is worthy enough for:
If you prefer I can keep a helper to fill in a xfs_fsop_resblks
structure under m_sb_lock in fsops.c, but I'm not sure that's worth it.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] xfs: clean up the XFS_IOC_FSCOUNTS handler
2023-11-26 13:01 ` [PATCH 2/4] xfs: clean up the XFS_IOC_FSCOUNTS handler Christoph Hellwig
` (2 preceding siblings ...)
2023-11-28 1:58 ` Darrick J. Wong
@ 2023-12-03 6:31 ` kernel test robot
3 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2023-12-03 6:31 UTC (permalink / raw
To: Christoph Hellwig; +Cc: oe-kbuild-all
Hi Christoph,
kernel test robot noticed the following build warnings:
[auto build test WARNING on xfs-linux/for-next]
[also build test WARNING on linus/master v6.7-rc3 next-20231201]
[cannot apply to djwong-xfs/djwong-devel]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Christoph-Hellwig/xfs-clean-up-the-XFS_IOC_FSCOUNTS-handler/20231126-220129
base: https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git for-next
patch link: https://lore.kernel.org/r/20231126130124.1251467-3-hch%40lst.de
patch subject: [PATCH 2/4] xfs: clean up the XFS_IOC_FSCOUNTS handler
config: i386-randconfig-062-20231126 (https://download.01.org/0day-ci/archive/20231202/202312022209.TrhJqh0l-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231202/202312022209.TrhJqh0l-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312022209.TrhJqh0l-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> fs/xfs/xfs_ioctl.c:1918:26: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void [noderef] __user *to @@ got struct xfs_fsop_counts *uarg @@
fs/xfs/xfs_ioctl.c:1918:26: sparse: expected void [noderef] __user *to
fs/xfs/xfs_ioctl.c:1918:26: sparse: got struct xfs_fsop_counts *uarg
>> fs/xfs/xfs_ioctl.c:2060:48: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected struct xfs_fsop_counts *uarg @@ got void [noderef] __user *arg @@
fs/xfs/xfs_ioctl.c:2060:48: sparse: expected struct xfs_fsop_counts *uarg
fs/xfs/xfs_ioctl.c:2060:48: sparse: got void [noderef] __user *arg
vim +1918 fs/xfs/xfs_ioctl.c
1904
1905 static int
1906 xfs_ioctl_fs_counts(
1907 struct xfs_mount *mp,
1908 struct xfs_fsop_counts *uarg)
1909 {
1910 struct xfs_fsop_counts out = {
1911 .allocino = percpu_counter_read_positive(&mp->m_icount),
1912 .freeino = percpu_counter_read_positive(&mp->m_ifree),
1913 .freedata = percpu_counter_read_positive(&mp->m_fdblocks) -
1914 xfs_fdblocks_unavailable(mp),
1915 .freertx = percpu_counter_read_positive(&mp->m_frextents),
1916 };
1917
> 1918 if (copy_to_user(uarg, &out, sizeof(out)))
1919 return -EFAULT;
1920 return 0;
1921 }
1922
1923 /*
1924 * These long-unused ioctls were removed from the official ioctl API in 5.17,
1925 * but retain these definitions so that we can log warnings about them.
1926 */
1927 #define XFS_IOC_ALLOCSP _IOW ('X', 10, struct xfs_flock64)
1928 #define XFS_IOC_FREESP _IOW ('X', 11, struct xfs_flock64)
1929 #define XFS_IOC_ALLOCSP64 _IOW ('X', 36, struct xfs_flock64)
1930 #define XFS_IOC_FREESP64 _IOW ('X', 37, struct xfs_flock64)
1931
1932 /*
1933 * Note: some of the ioctl's return positive numbers as a
1934 * byte count indicating success, such as readlink_by_handle.
1935 * So we don't "sign flip" like most other routines. This means
1936 * true errors need to be returned as a negative value.
1937 */
1938 long
1939 xfs_file_ioctl(
1940 struct file *filp,
1941 unsigned int cmd,
1942 unsigned long p)
1943 {
1944 struct inode *inode = file_inode(filp);
1945 struct xfs_inode *ip = XFS_I(inode);
1946 struct xfs_mount *mp = ip->i_mount;
1947 void __user *arg = (void __user *)p;
1948 int error;
1949
1950 trace_xfs_file_ioctl(ip);
1951
1952 switch (cmd) {
1953 case FITRIM:
1954 return xfs_ioc_trim(mp, arg);
1955 case FS_IOC_GETFSLABEL:
1956 return xfs_ioc_getlabel(mp, arg);
1957 case FS_IOC_SETFSLABEL:
1958 return xfs_ioc_setlabel(filp, mp, arg);
1959 case XFS_IOC_ALLOCSP:
1960 case XFS_IOC_FREESP:
1961 case XFS_IOC_ALLOCSP64:
1962 case XFS_IOC_FREESP64:
1963 xfs_warn_once(mp,
1964 "%s should use fallocate; XFS_IOC_{ALLOC,FREE}SP ioctl unsupported",
1965 current->comm);
1966 return -ENOTTY;
1967 case XFS_IOC_DIOINFO: {
1968 struct xfs_buftarg *target = xfs_inode_buftarg(ip);
1969 struct dioattr da;
1970
1971 da.d_mem = da.d_miniosz = target->bt_logical_sectorsize;
1972 da.d_maxiosz = INT_MAX & ~(da.d_miniosz - 1);
1973
1974 if (copy_to_user(arg, &da, sizeof(da)))
1975 return -EFAULT;
1976 return 0;
1977 }
1978
1979 case XFS_IOC_FSBULKSTAT_SINGLE:
1980 case XFS_IOC_FSBULKSTAT:
1981 case XFS_IOC_FSINUMBERS:
1982 return xfs_ioc_fsbulkstat(filp, cmd, arg);
1983
1984 case XFS_IOC_BULKSTAT:
1985 return xfs_ioc_bulkstat(filp, cmd, arg);
1986 case XFS_IOC_INUMBERS:
1987 return xfs_ioc_inumbers(mp, cmd, arg);
1988
1989 case XFS_IOC_FSGEOMETRY_V1:
1990 return xfs_ioc_fsgeometry(mp, arg, 3);
1991 case XFS_IOC_FSGEOMETRY_V4:
1992 return xfs_ioc_fsgeometry(mp, arg, 4);
1993 case XFS_IOC_FSGEOMETRY:
1994 return xfs_ioc_fsgeometry(mp, arg, 5);
1995
1996 case XFS_IOC_AG_GEOMETRY:
1997 return xfs_ioc_ag_geometry(mp, arg);
1998
1999 case XFS_IOC_GETVERSION:
2000 return put_user(inode->i_generation, (int __user *)arg);
2001
2002 case XFS_IOC_FSGETXATTRA:
2003 return xfs_ioc_fsgetxattra(ip, arg);
2004
2005 case XFS_IOC_GETBMAP:
2006 case XFS_IOC_GETBMAPA:
2007 case XFS_IOC_GETBMAPX:
2008 return xfs_ioc_getbmap(filp, cmd, arg);
2009
2010 case FS_IOC_GETFSMAP:
2011 return xfs_ioc_getfsmap(ip, arg);
2012
2013 case XFS_IOC_SCRUB_METADATA:
2014 return xfs_ioc_scrub_metadata(filp, arg);
2015
2016 case XFS_IOC_FD_TO_HANDLE:
2017 case XFS_IOC_PATH_TO_HANDLE:
2018 case XFS_IOC_PATH_TO_FSHANDLE: {
2019 xfs_fsop_handlereq_t hreq;
2020
2021 if (copy_from_user(&hreq, arg, sizeof(hreq)))
2022 return -EFAULT;
2023 return xfs_find_handle(cmd, &hreq);
2024 }
2025 case XFS_IOC_OPEN_BY_HANDLE: {
2026 xfs_fsop_handlereq_t hreq;
2027
2028 if (copy_from_user(&hreq, arg, sizeof(xfs_fsop_handlereq_t)))
2029 return -EFAULT;
2030 return xfs_open_by_handle(filp, &hreq);
2031 }
2032
2033 case XFS_IOC_READLINK_BY_HANDLE: {
2034 xfs_fsop_handlereq_t hreq;
2035
2036 if (copy_from_user(&hreq, arg, sizeof(xfs_fsop_handlereq_t)))
2037 return -EFAULT;
2038 return xfs_readlink_by_handle(filp, &hreq);
2039 }
2040 case XFS_IOC_ATTRLIST_BY_HANDLE:
2041 return xfs_attrlist_by_handle(filp, arg);
2042
2043 case XFS_IOC_ATTRMULTI_BY_HANDLE:
2044 return xfs_attrmulti_by_handle(filp, arg);
2045
2046 case XFS_IOC_SWAPEXT: {
2047 struct xfs_swapext sxp;
2048
2049 if (copy_from_user(&sxp, arg, sizeof(xfs_swapext_t)))
2050 return -EFAULT;
2051 error = mnt_want_write_file(filp);
2052 if (error)
2053 return error;
2054 error = xfs_ioc_swapext(&sxp);
2055 mnt_drop_write_file(filp);
2056 return error;
2057 }
2058
2059 case XFS_IOC_FSCOUNTS:
> 2060 return xfs_ioctl_fs_counts(mp, arg);
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-12-03 6:31 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-26 13:01 misc cleanups to the resblks interfaces Christoph Hellwig
2023-11-26 13:01 ` [PATCH 1/4] xfs: clean up the XFS_IOC_{GS}ET_RESBLKS handler Christoph Hellwig
2023-11-28 1:53 ` Darrick J. Wong
2023-11-26 13:01 ` [PATCH 2/4] xfs: clean up the XFS_IOC_FSCOUNTS handler Christoph Hellwig
2023-11-27 10:19 ` kernel test robot
2023-11-27 16:40 ` Christoph Hellwig
2023-11-28 1:58 ` Darrick J. Wong
2023-11-28 5:30 ` Christoph Hellwig
2023-12-03 6:31 ` kernel test robot
2023-11-26 13:01 ` [PATCH 3/4] xfs: clean up the xfs_reserve_blocks interface Christoph Hellwig
2023-11-28 2:09 ` Darrick J. Wong
2023-11-28 5:32 ` Christoph Hellwig
2023-11-26 13:01 ` [PATCH 4/4] xfs: clean up xfs_fsops.h Christoph Hellwig
2023-11-28 1:51 ` Darrick J. Wong
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.