* [PATCH] fs: move FMODE_UNSIGNED_OFFSET to fop_flags
@ 2024-08-09 10:38 Christian Brauner
2024-08-09 12:23 ` Jeff Layton
2024-08-09 17:14 ` Jan Kara
0 siblings, 2 replies; 6+ messages in thread
From: Christian Brauner @ 2024-08-09 10:38 UTC (permalink / raw)
To: linux-fsdevel
Cc: Jan Kara, Jeff Layton, Al Viro, Josef Bacik, Christian Brauner
This is another flag that is statically set and doesn't need to use up
an FMODE_* bit. Move it to ->fop_flags and free up another FMODE_* bit.
(1) mem_open() used from proc_mem_operations
(2) adi_open() used from adi_fops
(3) drm_open_helper():
(3.1) accel_open() used from DRM_ACCEL_FOPS
(3.2) drm_open() used from
(3.2.1) amdgpu_driver_kms_fops
(3.2.2) psb_gem_fops
(3.2.3) i915_driver_fops
(3.2.4) nouveau_driver_fops
(3.2.5) panthor_drm_driver_fops
(3.2.6) radeon_driver_kms_fops
(3.2.7) tegra_drm_fops
(3.2.8) vmwgfx_driver_fops
(3.2.9) xe_driver_fops
(3.2.10) DRM_GEM_FOPS
(3.2.11) DEFINE_DRM_GEM_DMA_FOPS
(4) struct memdev sets fmode flags based on type of device opened. For
devices using struct mem_fops unsigned offset is used.
Mark all these file operations as FOP_UNSIGNED_OFFSET and add asserts
into the open helper to ensure that the flag is always set.
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
---
drivers/char/adi.c | 8 +-------
drivers/char/mem.c | 3 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 +
drivers/gpu/drm/drm_file.c | 3 ++-
drivers/gpu/drm/gma500/psb_drv.c | 1 +
drivers/gpu/drm/i915/i915_driver.c | 1 +
drivers/gpu/drm/nouveau/nouveau_drm.c | 1 +
drivers/gpu/drm/radeon/radeon_drv.c | 1 +
drivers/gpu/drm/tegra/drm.c | 1 +
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 1 +
drivers/gpu/drm/xe/xe_device.c | 1 +
fs/proc/base.c | 10 ++++------
fs/read_write.c | 2 +-
include/drm/drm_accel.h | 3 ++-
include/drm/drm_gem.h | 3 ++-
include/drm/drm_gem_dma_helper.h | 1 +
include/linux/fs.h | 5 +++--
mm/mmap.c | 2 +-
18 files changed, 27 insertions(+), 21 deletions(-)
diff --git a/drivers/char/adi.c b/drivers/char/adi.c
index 751d7cc0da1b..1c76c8758f0f 100644
--- a/drivers/char/adi.c
+++ b/drivers/char/adi.c
@@ -14,12 +14,6 @@
#define MAX_BUF_SZ PAGE_SIZE
-static int adi_open(struct inode *inode, struct file *file)
-{
- file->f_mode |= FMODE_UNSIGNED_OFFSET;
- return 0;
-}
-
static int read_mcd_tag(unsigned long addr)
{
long err;
@@ -206,9 +200,9 @@ static loff_t adi_llseek(struct file *file, loff_t offset, int whence)
static const struct file_operations adi_fops = {
.owner = THIS_MODULE,
.llseek = adi_llseek,
- .open = adi_open,
.read = adi_read,
.write = adi_write,
+ .fop_flags = FOP_UNSIGNED_OFFSET,
};
static struct miscdevice adi_miscdev = {
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 7c359cc406d5..169eed162a7f 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -643,6 +643,7 @@ static const struct file_operations __maybe_unused mem_fops = {
.get_unmapped_area = get_unmapped_area_mem,
.mmap_capabilities = memory_mmap_capabilities,
#endif
+ .fop_flags = FOP_UNSIGNED_OFFSET,
};
static const struct file_operations null_fops = {
@@ -693,7 +694,7 @@ static const struct memdev {
umode_t mode;
} devlist[] = {
#ifdef CONFIG_DEVMEM
- [DEVMEM_MINOR] = { "mem", &mem_fops, FMODE_UNSIGNED_OFFSET, 0 },
+ [DEVMEM_MINOR] = { "mem", &mem_fops, 0, 0 },
#endif
[3] = { "null", &null_fops, FMODE_NOWAIT, 0666 },
#ifdef CONFIG_DEVPORT
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 094498a0964b..d7ef8cbecf6c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2908,6 +2908,7 @@ static const struct file_operations amdgpu_driver_kms_fops = {
#ifdef CONFIG_PROC_FS
.show_fdinfo = drm_show_fdinfo,
#endif
+ .fop_flags = FOP_UNSIGNED_OFFSET,
};
int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv)
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 714e42b05108..f8de3cba1a08 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -318,6 +318,8 @@ int drm_open_helper(struct file *filp, struct drm_minor *minor)
if (dev->switch_power_state != DRM_SWITCH_POWER_ON &&
dev->switch_power_state != DRM_SWITCH_POWER_DYNAMIC_OFF)
return -EINVAL;
+ if (WARN_ON_ONCE(!(filp->f_op->fop_flags & FOP_UNSIGNED_OFFSET)))
+ return -EINVAL;
drm_dbg_core(dev, "comm=\"%s\", pid=%d, minor=%d\n",
current->comm, task_pid_nr(current), minor->index);
@@ -335,7 +337,6 @@ int drm_open_helper(struct file *filp, struct drm_minor *minor)
}
filp->private_data = priv;
- filp->f_mode |= FMODE_UNSIGNED_OFFSET;
priv->filp = filp;
mutex_lock(&dev->filelist_mutex);
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 8b64f61ffaf9..d67c2b3ad901 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -498,6 +498,7 @@ static const struct file_operations psb_gem_fops = {
.mmap = drm_gem_mmap,
.poll = drm_poll,
.read = drm_read,
+ .fop_flags = FOP_UNSIGNED_OFFSET,
};
static const struct drm_driver driver = {
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index fb8e9c2fcea5..cf276299bccb 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -1671,6 +1671,7 @@ static const struct file_operations i915_driver_fops = {
#ifdef CONFIG_PROC_FS
.show_fdinfo = drm_show_fdinfo,
#endif
+ .fop_flags = FOP_UNSIGNED_OFFSET,
};
static int
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index a58c31089613..e243b42f8582 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -1274,6 +1274,7 @@ nouveau_driver_fops = {
.compat_ioctl = nouveau_compat_ioctl,
#endif
.llseek = noop_llseek,
+ .fop_flags = FOP_UNSIGNED_OFFSET,
};
static struct drm_driver
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 7bf08164140e..ac49779ed03d 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -520,6 +520,7 @@ static const struct file_operations radeon_driver_kms_fops = {
#ifdef CONFIG_COMPAT
.compat_ioctl = radeon_kms_compat_ioctl,
#endif
+ .fop_flags = FOP_UNSIGNED_OFFSET,
};
static const struct drm_ioctl_desc radeon_ioctls_kms[] = {
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 03d1c76aec2d..108c26a33edb 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -801,6 +801,7 @@ static const struct file_operations tegra_drm_fops = {
.read = drm_read,
.compat_ioctl = drm_compat_ioctl,
.llseek = noop_llseek,
+ .fop_flags = FOP_UNSIGNED_OFFSET,
};
static int tegra_drm_context_cleanup(int id, void *p, void *data)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 50ad3105c16e..2825dd3149ed 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -1609,6 +1609,7 @@ static const struct file_operations vmwgfx_driver_fops = {
.compat_ioctl = vmw_compat_ioctl,
#endif
.llseek = noop_llseek,
+ .fop_flags = FOP_UNSIGNED_OFFSET,
};
static const struct drm_driver driver = {
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index 76109415eba6..ea7e3ff6feba 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -197,6 +197,7 @@ static const struct file_operations xe_driver_fops = {
#ifdef CONFIG_PROC_FS
.show_fdinfo = drm_show_fdinfo,
#endif
+ .fop_flags = FOP_UNSIGNED_OFFSET,
};
static struct drm_driver driver = {
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 72a1acd03675..1409d1003101 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -827,12 +827,9 @@ static int __mem_open(struct inode *inode, struct file *file, unsigned int mode)
static int mem_open(struct inode *inode, struct file *file)
{
- int ret = __mem_open(inode, file, PTRACE_MODE_ATTACH);
-
- /* OK to pass negative loff_t, we can catch out-of-range */
- file->f_mode |= FMODE_UNSIGNED_OFFSET;
-
- return ret;
+ if (WARN_ON_ONCE(!(file->f_op->fop_flags & FOP_UNSIGNED_OFFSET)))
+ return -EINVAL;
+ return __mem_open(inode, file, PTRACE_MODE_ATTACH);
}
static ssize_t mem_rw(struct file *file, char __user *buf,
@@ -932,6 +929,7 @@ static const struct file_operations proc_mem_operations = {
.write = mem_write,
.open = mem_open,
.release = mem_release,
+ .fop_flags = FOP_UNSIGNED_OFFSET,
};
static int environ_open(struct inode *inode, struct file *file)
diff --git a/fs/read_write.c b/fs/read_write.c
index 90e283b31ca1..89d4af0e3b93 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -36,7 +36,7 @@ EXPORT_SYMBOL(generic_ro_fops);
static inline bool unsigned_offsets(struct file *file)
{
- return file->f_mode & FMODE_UNSIGNED_OFFSET;
+ return file->f_op->fop_flags & FOP_UNSIGNED_OFFSET;
}
/**
diff --git a/include/drm/drm_accel.h b/include/drm/drm_accel.h
index f4d3784b1dce..41c78b7d712c 100644
--- a/include/drm/drm_accel.h
+++ b/include/drm/drm_accel.h
@@ -28,7 +28,8 @@
.poll = drm_poll,\
.read = drm_read,\
.llseek = noop_llseek, \
- .mmap = drm_gem_mmap
+ .mmap = drm_gem_mmap, \
+ .fop_flags = FOP_UNSIGNED_OFFSET
/**
* DEFINE_DRM_ACCEL_FOPS() - macro to generate file operations for accelerators drivers
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index bae4865b2101..d8b86df2ec0d 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -447,7 +447,8 @@ struct drm_gem_object {
.poll = drm_poll,\
.read = drm_read,\
.llseek = noop_llseek,\
- .mmap = drm_gem_mmap
+ .mmap = drm_gem_mmap, \
+ .fop_flags = FOP_UNSIGNED_OFFSET
/**
* DEFINE_DRM_GEM_FOPS() - macro to generate file operations for GEM drivers
diff --git a/include/drm/drm_gem_dma_helper.h b/include/drm/drm_gem_dma_helper.h
index a827bde494f6..f2678e7ecb98 100644
--- a/include/drm/drm_gem_dma_helper.h
+++ b/include/drm/drm_gem_dma_helper.h
@@ -267,6 +267,7 @@ unsigned long drm_gem_dma_get_unmapped_area(struct file *filp,
.read = drm_read,\
.llseek = noop_llseek,\
.mmap = drm_gem_mmap,\
+ .fop_flags = FOP_UNSIGNED_OFFSET, \
DRM_GEM_DMA_UNMAPPED_AREA_FOPS \
}
diff --git a/include/linux/fs.h b/include/linux/fs.h
index fd34b5755c0b..40ebfa09112c 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -146,8 +146,7 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
/* Expect random access pattern */
#define FMODE_RANDOM ((__force fmode_t)(1 << 12))
-/* File is huge (eg. /dev/mem): treat loff_t as unsigned */
-#define FMODE_UNSIGNED_OFFSET ((__force fmode_t)(1 << 13))
+/* FMODE_* bit 13 */
/* File is opened with O_PATH; almost nothing can be done with it */
#define FMODE_PATH ((__force fmode_t)(1 << 14))
@@ -2073,6 +2072,8 @@ struct file_operations {
#define FOP_DIO_PARALLEL_WRITE ((__force fop_flags_t)(1 << 3))
/* Contains huge pages */
#define FOP_HUGE_PAGES ((__force fop_flags_t)(1 << 4))
+/* Treat loff_t as unsigned (e.g., /dev/mem) */
+#define FOP_UNSIGNED_OFFSET ((__force fop_flags_t)(1 << 5))
/* Wrap a directory iterator that needs exclusive inode access */
int wrap_directory_iterator(struct file *, struct dir_context *,
diff --git a/mm/mmap.c b/mm/mmap.c
index d0dfc85b209b..6ddb278a5ee8 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1229,7 +1229,7 @@ static inline u64 file_mmap_size_max(struct file *file, struct inode *inode)
return MAX_LFS_FILESIZE;
/* Special "we do even unsigned file positions" case */
- if (file->f_mode & FMODE_UNSIGNED_OFFSET)
+ if (file->f_op->fop_flags & FOP_UNSIGNED_OFFSET)
return 0;
/* Yes, random drivers might want more. But I'm tired of buggy drivers */
---
base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b
change-id: 20240809-work-fop_unsigned-5f6f7734cb7b
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] fs: move FMODE_UNSIGNED_OFFSET to fop_flags
2024-08-09 10:38 Christian Brauner
@ 2024-08-09 12:23 ` Jeff Layton
2024-08-09 17:14 ` Jan Kara
1 sibling, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2024-08-09 12:23 UTC (permalink / raw)
To: Christian Brauner, linux-fsdevel; +Cc: Jan Kara, Al Viro, Josef Bacik
On Fri, 2024-08-09 at 12:38 +0200, Christian Brauner wrote:
> This is another flag that is statically set and doesn't need to use up
> an FMODE_* bit. Move it to ->fop_flags and free up another FMODE_* bit.
>
> (1) mem_open() used from proc_mem_operations
> (2) adi_open() used from adi_fops
> (3) drm_open_helper():
> (3.1) accel_open() used from DRM_ACCEL_FOPS
> (3.2) drm_open() used from
> (3.2.1) amdgpu_driver_kms_fops
> (3.2.2) psb_gem_fops
> (3.2.3) i915_driver_fops
> (3.2.4) nouveau_driver_fops
> (3.2.5) panthor_drm_driver_fops
> (3.2.6) radeon_driver_kms_fops
> (3.2.7) tegra_drm_fops
> (3.2.8) vmwgfx_driver_fops
> (3.2.9) xe_driver_fops
> (3.2.10) DRM_GEM_FOPS
> (3.2.11) DEFINE_DRM_GEM_DMA_FOPS
> (4) struct memdev sets fmode flags based on type of device opened. For
> devices using struct mem_fops unsigned offset is used.
>
> Mark all these file operations as FOP_UNSIGNED_OFFSET and add asserts
> into the open helper to ensure that the flag is always set.
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
> ---
> drivers/char/adi.c | 8 +-------
> drivers/char/mem.c | 3 ++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 +
> drivers/gpu/drm/drm_file.c | 3 ++-
> drivers/gpu/drm/gma500/psb_drv.c | 1 +
> drivers/gpu/drm/i915/i915_driver.c | 1 +
> drivers/gpu/drm/nouveau/nouveau_drm.c | 1 +
> drivers/gpu/drm/radeon/radeon_drv.c | 1 +
> drivers/gpu/drm/tegra/drm.c | 1 +
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 1 +
> drivers/gpu/drm/xe/xe_device.c | 1 +
> fs/proc/base.c | 10 ++++------
> fs/read_write.c | 2 +-
> include/drm/drm_accel.h | 3 ++-
> include/drm/drm_gem.h | 3 ++-
> include/drm/drm_gem_dma_helper.h | 1 +
> include/linux/fs.h | 5 +++--
> mm/mmap.c | 2 +-
> 18 files changed, 27 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/char/adi.c b/drivers/char/adi.c
> index 751d7cc0da1b..1c76c8758f0f 100644
> --- a/drivers/char/adi.c
> +++ b/drivers/char/adi.c
> @@ -14,12 +14,6 @@
>
> #define MAX_BUF_SZ PAGE_SIZE
>
> -static int adi_open(struct inode *inode, struct file *file)
> -{
> - file->f_mode |= FMODE_UNSIGNED_OFFSET;
> - return 0;
> -}
> -
> static int read_mcd_tag(unsigned long addr)
> {
> long err;
> @@ -206,9 +200,9 @@ static loff_t adi_llseek(struct file *file, loff_t offset, int whence)
> static const struct file_operations adi_fops = {
> .owner = THIS_MODULE,
> .llseek = adi_llseek,
> - .open = adi_open,
> .read = adi_read,
> .write = adi_write,
> + .fop_flags = FOP_UNSIGNED_OFFSET,
> };
>
> static struct miscdevice adi_miscdev = {
> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> index 7c359cc406d5..169eed162a7f 100644
> --- a/drivers/char/mem.c
> +++ b/drivers/char/mem.c
> @@ -643,6 +643,7 @@ static const struct file_operations __maybe_unused mem_fops = {
> .get_unmapped_area = get_unmapped_area_mem,
> .mmap_capabilities = memory_mmap_capabilities,
> #endif
> + .fop_flags = FOP_UNSIGNED_OFFSET,
> };
>
> static const struct file_operations null_fops = {
> @@ -693,7 +694,7 @@ static const struct memdev {
> umode_t mode;
> } devlist[] = {
> #ifdef CONFIG_DEVMEM
> - [DEVMEM_MINOR] = { "mem", &mem_fops, FMODE_UNSIGNED_OFFSET, 0 },
> + [DEVMEM_MINOR] = { "mem", &mem_fops, 0, 0 },
> #endif
> [3] = { "null", &null_fops, FMODE_NOWAIT, 0666 },
> #ifdef CONFIG_DEVPORT
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 094498a0964b..d7ef8cbecf6c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2908,6 +2908,7 @@ static const struct file_operations amdgpu_driver_kms_fops = {
> #ifdef CONFIG_PROC_FS
> .show_fdinfo = drm_show_fdinfo,
> #endif
> + .fop_flags = FOP_UNSIGNED_OFFSET,
> };
>
> int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv)
> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> index 714e42b05108..f8de3cba1a08 100644
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -318,6 +318,8 @@ int drm_open_helper(struct file *filp, struct drm_minor *minor)
> if (dev->switch_power_state != DRM_SWITCH_POWER_ON &&
> dev->switch_power_state != DRM_SWITCH_POWER_DYNAMIC_OFF)
> return -EINVAL;
> + if (WARN_ON_ONCE(!(filp->f_op->fop_flags & FOP_UNSIGNED_OFFSET)))
> + return -EINVAL;
>
> drm_dbg_core(dev, "comm=\"%s\", pid=%d, minor=%d\n",
> current->comm, task_pid_nr(current), minor->index);
> @@ -335,7 +337,6 @@ int drm_open_helper(struct file *filp, struct drm_minor *minor)
> }
>
> filp->private_data = priv;
> - filp->f_mode |= FMODE_UNSIGNED_OFFSET;
> priv->filp = filp;
>
> mutex_lock(&dev->filelist_mutex);
> diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
> index 8b64f61ffaf9..d67c2b3ad901 100644
> --- a/drivers/gpu/drm/gma500/psb_drv.c
> +++ b/drivers/gpu/drm/gma500/psb_drv.c
> @@ -498,6 +498,7 @@ static const struct file_operations psb_gem_fops = {
> .mmap = drm_gem_mmap,
> .poll = drm_poll,
> .read = drm_read,
> + .fop_flags = FOP_UNSIGNED_OFFSET,
> };
>
> static const struct drm_driver driver = {
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index fb8e9c2fcea5..cf276299bccb 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -1671,6 +1671,7 @@ static const struct file_operations i915_driver_fops = {
> #ifdef CONFIG_PROC_FS
> .show_fdinfo = drm_show_fdinfo,
> #endif
> + .fop_flags = FOP_UNSIGNED_OFFSET,
> };
>
> static int
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index a58c31089613..e243b42f8582 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -1274,6 +1274,7 @@ nouveau_driver_fops = {
> .compat_ioctl = nouveau_compat_ioctl,
> #endif
> .llseek = noop_llseek,
> + .fop_flags = FOP_UNSIGNED_OFFSET,
> };
>
> static struct drm_driver
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> index 7bf08164140e..ac49779ed03d 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -520,6 +520,7 @@ static const struct file_operations radeon_driver_kms_fops = {
> #ifdef CONFIG_COMPAT
> .compat_ioctl = radeon_kms_compat_ioctl,
> #endif
> + .fop_flags = FOP_UNSIGNED_OFFSET,
> };
>
> static const struct drm_ioctl_desc radeon_ioctls_kms[] = {
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index 03d1c76aec2d..108c26a33edb 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -801,6 +801,7 @@ static const struct file_operations tegra_drm_fops = {
> .read = drm_read,
> .compat_ioctl = drm_compat_ioctl,
> .llseek = noop_llseek,
> + .fop_flags = FOP_UNSIGNED_OFFSET,
> };
>
> static int tegra_drm_context_cleanup(int id, void *p, void *data)
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index 50ad3105c16e..2825dd3149ed 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -1609,6 +1609,7 @@ static const struct file_operations vmwgfx_driver_fops = {
> .compat_ioctl = vmw_compat_ioctl,
> #endif
> .llseek = noop_llseek,
> + .fop_flags = FOP_UNSIGNED_OFFSET,
> };
>
> static const struct drm_driver driver = {
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 76109415eba6..ea7e3ff6feba 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -197,6 +197,7 @@ static const struct file_operations xe_driver_fops = {
> #ifdef CONFIG_PROC_FS
> .show_fdinfo = drm_show_fdinfo,
> #endif
> + .fop_flags = FOP_UNSIGNED_OFFSET,
> };
>
> static struct drm_driver driver = {
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 72a1acd03675..1409d1003101 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -827,12 +827,9 @@ static int __mem_open(struct inode *inode, struct file *file, unsigned int mode)
>
> static int mem_open(struct inode *inode, struct file *file)
> {
> - int ret = __mem_open(inode, file, PTRACE_MODE_ATTACH);
> -
> - /* OK to pass negative loff_t, we can catch out-of-range */
> - file->f_mode |= FMODE_UNSIGNED_OFFSET;
> -
> - return ret;
> + if (WARN_ON_ONCE(!(file->f_op->fop_flags & FOP_UNSIGNED_OFFSET)))
> + return -EINVAL;
> + return __mem_open(inode, file, PTRACE_MODE_ATTACH);
> }
>
> static ssize_t mem_rw(struct file *file, char __user *buf,
> @@ -932,6 +929,7 @@ static const struct file_operations proc_mem_operations = {
> .write = mem_write,
> .open = mem_open,
> .release = mem_release,
> + .fop_flags = FOP_UNSIGNED_OFFSET,
> };
>
> static int environ_open(struct inode *inode, struct file *file)
> diff --git a/fs/read_write.c b/fs/read_write.c
> index 90e283b31ca1..89d4af0e3b93 100644
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -36,7 +36,7 @@ EXPORT_SYMBOL(generic_ro_fops);
>
> static inline bool unsigned_offsets(struct file *file)
> {
> - return file->f_mode & FMODE_UNSIGNED_OFFSET;
> + return file->f_op->fop_flags & FOP_UNSIGNED_OFFSET;
> }
>
> /**
> diff --git a/include/drm/drm_accel.h b/include/drm/drm_accel.h
> index f4d3784b1dce..41c78b7d712c 100644
> --- a/include/drm/drm_accel.h
> +++ b/include/drm/drm_accel.h
> @@ -28,7 +28,8 @@
> .poll = drm_poll,\
> .read = drm_read,\
> .llseek = noop_llseek, \
> - .mmap = drm_gem_mmap
> + .mmap = drm_gem_mmap, \
> + .fop_flags = FOP_UNSIGNED_OFFSET
>
> /**
> * DEFINE_DRM_ACCEL_FOPS() - macro to generate file operations for accelerators drivers
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index bae4865b2101..d8b86df2ec0d 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -447,7 +447,8 @@ struct drm_gem_object {
> .poll = drm_poll,\
> .read = drm_read,\
> .llseek = noop_llseek,\
> - .mmap = drm_gem_mmap
> + .mmap = drm_gem_mmap, \
> + .fop_flags = FOP_UNSIGNED_OFFSET
>
> /**
> * DEFINE_DRM_GEM_FOPS() - macro to generate file operations for GEM drivers
> diff --git a/include/drm/drm_gem_dma_helper.h b/include/drm/drm_gem_dma_helper.h
> index a827bde494f6..f2678e7ecb98 100644
> --- a/include/drm/drm_gem_dma_helper.h
> +++ b/include/drm/drm_gem_dma_helper.h
> @@ -267,6 +267,7 @@ unsigned long drm_gem_dma_get_unmapped_area(struct file *filp,
> .read = drm_read,\
> .llseek = noop_llseek,\
> .mmap = drm_gem_mmap,\
> + .fop_flags = FOP_UNSIGNED_OFFSET, \
> DRM_GEM_DMA_UNMAPPED_AREA_FOPS \
> }
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index fd34b5755c0b..40ebfa09112c 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -146,8 +146,7 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
> /* Expect random access pattern */
> #define FMODE_RANDOM ((__force fmode_t)(1 << 12))
>
> -/* File is huge (eg. /dev/mem): treat loff_t as unsigned */
> -#define FMODE_UNSIGNED_OFFSET ((__force fmode_t)(1 << 13))
> +/* FMODE_* bit 13 */
>
> /* File is opened with O_PATH; almost nothing can be done with it */
> #define FMODE_PATH ((__force fmode_t)(1 << 14))
> @@ -2073,6 +2072,8 @@ struct file_operations {
> #define FOP_DIO_PARALLEL_WRITE ((__force fop_flags_t)(1 << 3))
> /* Contains huge pages */
> #define FOP_HUGE_PAGES ((__force fop_flags_t)(1 << 4))
> +/* Treat loff_t as unsigned (e.g., /dev/mem) */
> +#define FOP_UNSIGNED_OFFSET ((__force fop_flags_t)(1 << 5))
>
> /* Wrap a directory iterator that needs exclusive inode access */
> int wrap_directory_iterator(struct file *, struct dir_context *,
> diff --git a/mm/mmap.c b/mm/mmap.c
> index d0dfc85b209b..6ddb278a5ee8 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1229,7 +1229,7 @@ static inline u64 file_mmap_size_max(struct file *file, struct inode *inode)
> return MAX_LFS_FILESIZE;
>
> /* Special "we do even unsigned file positions" case */
> - if (file->f_mode & FMODE_UNSIGNED_OFFSET)
> + if (file->f_op->fop_flags & FOP_UNSIGNED_OFFSET)
> return 0;
>
> /* Yes, random drivers might want more. But I'm tired of buggy drivers */
>
> ---
> base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b
> change-id: 20240809-work-fop_unsigned-5f6f7734cb7b
>
Nice.
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fs: move FMODE_UNSIGNED_OFFSET to fop_flags
2024-08-09 10:38 Christian Brauner
2024-08-09 12:23 ` Jeff Layton
@ 2024-08-09 17:14 ` Jan Kara
1 sibling, 0 replies; 6+ messages in thread
From: Jan Kara @ 2024-08-09 17:14 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-fsdevel, Jan Kara, Jeff Layton, Al Viro, Josef Bacik
On Fri 09-08-24 12:38:56, Christian Brauner wrote:
> This is another flag that is statically set and doesn't need to use up
> an FMODE_* bit. Move it to ->fop_flags and free up another FMODE_* bit.
>
> (1) mem_open() used from proc_mem_operations
> (2) adi_open() used from adi_fops
> (3) drm_open_helper():
> (3.1) accel_open() used from DRM_ACCEL_FOPS
> (3.2) drm_open() used from
> (3.2.1) amdgpu_driver_kms_fops
> (3.2.2) psb_gem_fops
> (3.2.3) i915_driver_fops
> (3.2.4) nouveau_driver_fops
> (3.2.5) panthor_drm_driver_fops
> (3.2.6) radeon_driver_kms_fops
> (3.2.7) tegra_drm_fops
> (3.2.8) vmwgfx_driver_fops
> (3.2.9) xe_driver_fops
> (3.2.10) DRM_GEM_FOPS
> (3.2.11) DEFINE_DRM_GEM_DMA_FOPS
> (4) struct memdev sets fmode flags based on type of device opened. For
> devices using struct mem_fops unsigned offset is used.
>
> Mark all these file operations as FOP_UNSIGNED_OFFSET and add asserts
> into the open helper to ensure that the flag is always set.
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> ---
> drivers/char/adi.c | 8 +-------
> drivers/char/mem.c | 3 ++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 +
> drivers/gpu/drm/drm_file.c | 3 ++-
> drivers/gpu/drm/gma500/psb_drv.c | 1 +
> drivers/gpu/drm/i915/i915_driver.c | 1 +
> drivers/gpu/drm/nouveau/nouveau_drm.c | 1 +
> drivers/gpu/drm/radeon/radeon_drv.c | 1 +
> drivers/gpu/drm/tegra/drm.c | 1 +
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 1 +
> drivers/gpu/drm/xe/xe_device.c | 1 +
> fs/proc/base.c | 10 ++++------
> fs/read_write.c | 2 +-
> include/drm/drm_accel.h | 3 ++-
> include/drm/drm_gem.h | 3 ++-
> include/drm/drm_gem_dma_helper.h | 1 +
> include/linux/fs.h | 5 +++--
> mm/mmap.c | 2 +-
> 18 files changed, 27 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/char/adi.c b/drivers/char/adi.c
> index 751d7cc0da1b..1c76c8758f0f 100644
> --- a/drivers/char/adi.c
> +++ b/drivers/char/adi.c
> @@ -14,12 +14,6 @@
>
> #define MAX_BUF_SZ PAGE_SIZE
>
> -static int adi_open(struct inode *inode, struct file *file)
> -{
> - file->f_mode |= FMODE_UNSIGNED_OFFSET;
> - return 0;
> -}
> -
> static int read_mcd_tag(unsigned long addr)
> {
> long err;
> @@ -206,9 +200,9 @@ static loff_t adi_llseek(struct file *file, loff_t offset, int whence)
> static const struct file_operations adi_fops = {
> .owner = THIS_MODULE,
> .llseek = adi_llseek,
> - .open = adi_open,
> .read = adi_read,
> .write = adi_write,
> + .fop_flags = FOP_UNSIGNED_OFFSET,
> };
>
> static struct miscdevice adi_miscdev = {
> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> index 7c359cc406d5..169eed162a7f 100644
> --- a/drivers/char/mem.c
> +++ b/drivers/char/mem.c
> @@ -643,6 +643,7 @@ static const struct file_operations __maybe_unused mem_fops = {
> .get_unmapped_area = get_unmapped_area_mem,
> .mmap_capabilities = memory_mmap_capabilities,
> #endif
> + .fop_flags = FOP_UNSIGNED_OFFSET,
> };
>
> static const struct file_operations null_fops = {
> @@ -693,7 +694,7 @@ static const struct memdev {
> umode_t mode;
> } devlist[] = {
> #ifdef CONFIG_DEVMEM
> - [DEVMEM_MINOR] = { "mem", &mem_fops, FMODE_UNSIGNED_OFFSET, 0 },
> + [DEVMEM_MINOR] = { "mem", &mem_fops, 0, 0 },
> #endif
> [3] = { "null", &null_fops, FMODE_NOWAIT, 0666 },
> #ifdef CONFIG_DEVPORT
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 094498a0964b..d7ef8cbecf6c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2908,6 +2908,7 @@ static const struct file_operations amdgpu_driver_kms_fops = {
> #ifdef CONFIG_PROC_FS
> .show_fdinfo = drm_show_fdinfo,
> #endif
> + .fop_flags = FOP_UNSIGNED_OFFSET,
> };
>
> int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv)
> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> index 714e42b05108..f8de3cba1a08 100644
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -318,6 +318,8 @@ int drm_open_helper(struct file *filp, struct drm_minor *minor)
> if (dev->switch_power_state != DRM_SWITCH_POWER_ON &&
> dev->switch_power_state != DRM_SWITCH_POWER_DYNAMIC_OFF)
> return -EINVAL;
> + if (WARN_ON_ONCE(!(filp->f_op->fop_flags & FOP_UNSIGNED_OFFSET)))
> + return -EINVAL;
>
> drm_dbg_core(dev, "comm=\"%s\", pid=%d, minor=%d\n",
> current->comm, task_pid_nr(current), minor->index);
> @@ -335,7 +337,6 @@ int drm_open_helper(struct file *filp, struct drm_minor *minor)
> }
>
> filp->private_data = priv;
> - filp->f_mode |= FMODE_UNSIGNED_OFFSET;
> priv->filp = filp;
>
> mutex_lock(&dev->filelist_mutex);
> diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
> index 8b64f61ffaf9..d67c2b3ad901 100644
> --- a/drivers/gpu/drm/gma500/psb_drv.c
> +++ b/drivers/gpu/drm/gma500/psb_drv.c
> @@ -498,6 +498,7 @@ static const struct file_operations psb_gem_fops = {
> .mmap = drm_gem_mmap,
> .poll = drm_poll,
> .read = drm_read,
> + .fop_flags = FOP_UNSIGNED_OFFSET,
> };
>
> static const struct drm_driver driver = {
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index fb8e9c2fcea5..cf276299bccb 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -1671,6 +1671,7 @@ static const struct file_operations i915_driver_fops = {
> #ifdef CONFIG_PROC_FS
> .show_fdinfo = drm_show_fdinfo,
> #endif
> + .fop_flags = FOP_UNSIGNED_OFFSET,
> };
>
> static int
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index a58c31089613..e243b42f8582 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -1274,6 +1274,7 @@ nouveau_driver_fops = {
> .compat_ioctl = nouveau_compat_ioctl,
> #endif
> .llseek = noop_llseek,
> + .fop_flags = FOP_UNSIGNED_OFFSET,
> };
>
> static struct drm_driver
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> index 7bf08164140e..ac49779ed03d 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -520,6 +520,7 @@ static const struct file_operations radeon_driver_kms_fops = {
> #ifdef CONFIG_COMPAT
> .compat_ioctl = radeon_kms_compat_ioctl,
> #endif
> + .fop_flags = FOP_UNSIGNED_OFFSET,
> };
>
> static const struct drm_ioctl_desc radeon_ioctls_kms[] = {
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index 03d1c76aec2d..108c26a33edb 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -801,6 +801,7 @@ static const struct file_operations tegra_drm_fops = {
> .read = drm_read,
> .compat_ioctl = drm_compat_ioctl,
> .llseek = noop_llseek,
> + .fop_flags = FOP_UNSIGNED_OFFSET,
> };
>
> static int tegra_drm_context_cleanup(int id, void *p, void *data)
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index 50ad3105c16e..2825dd3149ed 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -1609,6 +1609,7 @@ static const struct file_operations vmwgfx_driver_fops = {
> .compat_ioctl = vmw_compat_ioctl,
> #endif
> .llseek = noop_llseek,
> + .fop_flags = FOP_UNSIGNED_OFFSET,
> };
>
> static const struct drm_driver driver = {
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 76109415eba6..ea7e3ff6feba 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -197,6 +197,7 @@ static const struct file_operations xe_driver_fops = {
> #ifdef CONFIG_PROC_FS
> .show_fdinfo = drm_show_fdinfo,
> #endif
> + .fop_flags = FOP_UNSIGNED_OFFSET,
> };
>
> static struct drm_driver driver = {
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 72a1acd03675..1409d1003101 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -827,12 +827,9 @@ static int __mem_open(struct inode *inode, struct file *file, unsigned int mode)
>
> static int mem_open(struct inode *inode, struct file *file)
> {
> - int ret = __mem_open(inode, file, PTRACE_MODE_ATTACH);
> -
> - /* OK to pass negative loff_t, we can catch out-of-range */
> - file->f_mode |= FMODE_UNSIGNED_OFFSET;
> -
> - return ret;
> + if (WARN_ON_ONCE(!(file->f_op->fop_flags & FOP_UNSIGNED_OFFSET)))
> + return -EINVAL;
> + return __mem_open(inode, file, PTRACE_MODE_ATTACH);
> }
>
> static ssize_t mem_rw(struct file *file, char __user *buf,
> @@ -932,6 +929,7 @@ static const struct file_operations proc_mem_operations = {
> .write = mem_write,
> .open = mem_open,
> .release = mem_release,
> + .fop_flags = FOP_UNSIGNED_OFFSET,
> };
>
> static int environ_open(struct inode *inode, struct file *file)
> diff --git a/fs/read_write.c b/fs/read_write.c
> index 90e283b31ca1..89d4af0e3b93 100644
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -36,7 +36,7 @@ EXPORT_SYMBOL(generic_ro_fops);
>
> static inline bool unsigned_offsets(struct file *file)
> {
> - return file->f_mode & FMODE_UNSIGNED_OFFSET;
> + return file->f_op->fop_flags & FOP_UNSIGNED_OFFSET;
> }
>
> /**
> diff --git a/include/drm/drm_accel.h b/include/drm/drm_accel.h
> index f4d3784b1dce..41c78b7d712c 100644
> --- a/include/drm/drm_accel.h
> +++ b/include/drm/drm_accel.h
> @@ -28,7 +28,8 @@
> .poll = drm_poll,\
> .read = drm_read,\
> .llseek = noop_llseek, \
> - .mmap = drm_gem_mmap
> + .mmap = drm_gem_mmap, \
> + .fop_flags = FOP_UNSIGNED_OFFSET
>
> /**
> * DEFINE_DRM_ACCEL_FOPS() - macro to generate file operations for accelerators drivers
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index bae4865b2101..d8b86df2ec0d 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -447,7 +447,8 @@ struct drm_gem_object {
> .poll = drm_poll,\
> .read = drm_read,\
> .llseek = noop_llseek,\
> - .mmap = drm_gem_mmap
> + .mmap = drm_gem_mmap, \
> + .fop_flags = FOP_UNSIGNED_OFFSET
>
> /**
> * DEFINE_DRM_GEM_FOPS() - macro to generate file operations for GEM drivers
> diff --git a/include/drm/drm_gem_dma_helper.h b/include/drm/drm_gem_dma_helper.h
> index a827bde494f6..f2678e7ecb98 100644
> --- a/include/drm/drm_gem_dma_helper.h
> +++ b/include/drm/drm_gem_dma_helper.h
> @@ -267,6 +267,7 @@ unsigned long drm_gem_dma_get_unmapped_area(struct file *filp,
> .read = drm_read,\
> .llseek = noop_llseek,\
> .mmap = drm_gem_mmap,\
> + .fop_flags = FOP_UNSIGNED_OFFSET, \
> DRM_GEM_DMA_UNMAPPED_AREA_FOPS \
> }
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index fd34b5755c0b..40ebfa09112c 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -146,8 +146,7 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
> /* Expect random access pattern */
> #define FMODE_RANDOM ((__force fmode_t)(1 << 12))
>
> -/* File is huge (eg. /dev/mem): treat loff_t as unsigned */
> -#define FMODE_UNSIGNED_OFFSET ((__force fmode_t)(1 << 13))
> +/* FMODE_* bit 13 */
>
> /* File is opened with O_PATH; almost nothing can be done with it */
> #define FMODE_PATH ((__force fmode_t)(1 << 14))
> @@ -2073,6 +2072,8 @@ struct file_operations {
> #define FOP_DIO_PARALLEL_WRITE ((__force fop_flags_t)(1 << 3))
> /* Contains huge pages */
> #define FOP_HUGE_PAGES ((__force fop_flags_t)(1 << 4))
> +/* Treat loff_t as unsigned (e.g., /dev/mem) */
> +#define FOP_UNSIGNED_OFFSET ((__force fop_flags_t)(1 << 5))
>
> /* Wrap a directory iterator that needs exclusive inode access */
> int wrap_directory_iterator(struct file *, struct dir_context *,
> diff --git a/mm/mmap.c b/mm/mmap.c
> index d0dfc85b209b..6ddb278a5ee8 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1229,7 +1229,7 @@ static inline u64 file_mmap_size_max(struct file *file, struct inode *inode)
> return MAX_LFS_FILESIZE;
>
> /* Special "we do even unsigned file positions" case */
> - if (file->f_mode & FMODE_UNSIGNED_OFFSET)
> + if (file->f_op->fop_flags & FOP_UNSIGNED_OFFSET)
> return 0;
>
> /* Yes, random drivers might want more. But I'm tired of buggy drivers */
>
> ---
> base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b
> change-id: 20240809-work-fop_unsigned-5f6f7734cb7b
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fs: move FMODE_UNSIGNED_OFFSET to fop_flags
@ 2024-09-18 13:40 Liviu Dudau
2024-09-19 13:03 ` Christian Brauner
0 siblings, 1 reply; 6+ messages in thread
From: Liviu Dudau @ 2024-09-18 13:40 UTC (permalink / raw)
To: Christian Brauner
Cc: Jan Kara, Jeff Layton, Al Viro, Josef Bacik, linux-fsdevel
Hi Christian,
> This is another flag that is statically set and doesn't need to use up
> an FMODE_* bit. Move it to ->fop_flags and free up another FMODE_* bit.
>
> (1) mem_open() used from proc_mem_operations
> (2) adi_open() used from adi_fops
> (3) drm_open_helper():
> (3.1) accel_open() used from DRM_ACCEL_FOPS
> (3.2) drm_open() used from
> (3.2.1) amdgpu_driver_kms_fops
> (3.2.2) psb_gem_fops
> (3.2.3) i915_driver_fops
> (3.2.4) nouveau_driver_fops
> (3.2.5) panthor_drm_driver_fops
> (3.2.6) radeon_driver_kms_fops
> (3.2.7) tegra_drm_fops
> (3.2.8) vmwgfx_driver_fops
> (3.2.9) xe_driver_fops
> (3.2.10) DRM_GEM_FOPS
> (3.2.11) DEFINE_DRM_GEM_DMA_FOPS
> (4) struct memdev sets fmode flags based on type of device opened. For
> devices using struct mem_fops unsigned offset is used.
>
> Mark all these file operations as FOP_UNSIGNED_OFFSET and add asserts
> into the open helper to ensure that the flag is always set.
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
Your patch seems to be missing the panthor_drm_driver_fops changes. I've
discovered that on linux-next your patch triggers a WARN() during my testing.
I've added the following change to my local tree to fix the warning.
-- 8< -------------------------------------------
diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
index 34182f67136c1..c520f156e2d73 100644
--- a/drivers/gpu/drm/panthor/panthor_drv.c
+++ b/drivers/gpu/drm/panthor/panthor_drv.c
@@ -1383,6 +1383,7 @@ static const struct file_operations panthor_drm_driver_fops = {
.read = drm_read,
.llseek = noop_llseek,
.mmap = panthor_mmap,
+ .fop_flags = FOP_UNSIGNED_OFFSET,
};
#ifdef CONFIG_DEBUG_FS
-- 8< -------------------------------------------
Best regards,
Liviu
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] fs: move FMODE_UNSIGNED_OFFSET to fop_flags
2024-09-18 13:40 [PATCH] fs: move FMODE_UNSIGNED_OFFSET to fop_flags Liviu Dudau
@ 2024-09-19 13:03 ` Christian Brauner
2024-09-19 23:55 ` Liviu Dudau
0 siblings, 1 reply; 6+ messages in thread
From: Christian Brauner @ 2024-09-19 13:03 UTC (permalink / raw)
To: Liviu Dudau; +Cc: Jan Kara, Jeff Layton, Al Viro, Josef Bacik, linux-fsdevel
On Wed, Sep 18, 2024 at 02:40:46PM GMT, Liviu Dudau wrote:
> Hi Christian,
>
> > This is another flag that is statically set and doesn't need to use up
> > an FMODE_* bit. Move it to ->fop_flags and free up another FMODE_* bit.
> >
> > (1) mem_open() used from proc_mem_operations
> > (2) adi_open() used from adi_fops
> > (3) drm_open_helper():
> > (3.1) accel_open() used from DRM_ACCEL_FOPS
> > (3.2) drm_open() used from
> > (3.2.1) amdgpu_driver_kms_fops
> > (3.2.2) psb_gem_fops
> > (3.2.3) i915_driver_fops
> > (3.2.4) nouveau_driver_fops
> > (3.2.5) panthor_drm_driver_fops
> > (3.2.6) radeon_driver_kms_fops
> > (3.2.7) tegra_drm_fops
> > (3.2.8) vmwgfx_driver_fops
> > (3.2.9) xe_driver_fops
> > (3.2.10) DRM_GEM_FOPS
> > (3.2.11) DEFINE_DRM_GEM_DMA_FOPS
> > (4) struct memdev sets fmode flags based on type of device opened. For
> > devices using struct mem_fops unsigned offset is used.
> >
> > Mark all these file operations as FOP_UNSIGNED_OFFSET and add asserts
> > into the open helper to ensure that the flag is always set.
> >
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
>
> Your patch seems to be missing the panthor_drm_driver_fops changes. I've
> discovered that on linux-next your patch triggers a WARN() during my testing.
Yeah, I've added a WARN_ON_ONCE() to catch such cases. Good that it worked.
>
> I've added the following change to my local tree to fix the warning.
I would send a fixes PR soon. Do you want me to send a fix for this
along with this or are you taking this upstream soon yourself?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fs: move FMODE_UNSIGNED_OFFSET to fop_flags
2024-09-19 13:03 ` Christian Brauner
@ 2024-09-19 23:55 ` Liviu Dudau
0 siblings, 0 replies; 6+ messages in thread
From: Liviu Dudau @ 2024-09-19 23:55 UTC (permalink / raw)
To: Christian Brauner
Cc: Jan Kara, Jeff Layton, Al Viro, Josef Bacik, linux-fsdevel
On Thu, Sep 19, 2024 at 03:03:19PM +0200, Christian Brauner wrote:
> On Wed, Sep 18, 2024 at 02:40:46PM GMT, Liviu Dudau wrote:
> > Hi Christian,
> >
> > > This is another flag that is statically set and doesn't need to use up
> > > an FMODE_* bit. Move it to ->fop_flags and free up another FMODE_* bit.
> > >
> > > (1) mem_open() used from proc_mem_operations
> > > (2) adi_open() used from adi_fops
> > > (3) drm_open_helper():
> > > (3.1) accel_open() used from DRM_ACCEL_FOPS
> > > (3.2) drm_open() used from
> > > (3.2.1) amdgpu_driver_kms_fops
> > > (3.2.2) psb_gem_fops
> > > (3.2.3) i915_driver_fops
> > > (3.2.4) nouveau_driver_fops
> > > (3.2.5) panthor_drm_driver_fops
> > > (3.2.6) radeon_driver_kms_fops
> > > (3.2.7) tegra_drm_fops
> > > (3.2.8) vmwgfx_driver_fops
> > > (3.2.9) xe_driver_fops
> > > (3.2.10) DRM_GEM_FOPS
> > > (3.2.11) DEFINE_DRM_GEM_DMA_FOPS
> > > (4) struct memdev sets fmode flags based on type of device opened. For
> > > devices using struct mem_fops unsigned offset is used.
> > >
> > > Mark all these file operations as FOP_UNSIGNED_OFFSET and add asserts
> > > into the open helper to ensure that the flag is always set.
> > >
> > > Signed-off-by: Christian Brauner <brauner@kernel.org>
> >
> > Your patch seems to be missing the panthor_drm_driver_fops changes. I've
> > discovered that on linux-next your patch triggers a WARN() during my testing.
>
> Yeah, I've added a WARN_ON_ONCE() to catch such cases. Good that it worked.
>
> >
> > I've added the following change to my local tree to fix the warning.
>
> I would send a fixes PR soon. Do you want me to send a fix for this
> along with this or are you taking this upstream soon yourself?
Was your series pulled into v6.12?
I will send a patch and once reviewed I will push it into the relevant drm-misc branch.
Best regards,
Liviu
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-09-19 23:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-18 13:40 [PATCH] fs: move FMODE_UNSIGNED_OFFSET to fop_flags Liviu Dudau
2024-09-19 13:03 ` Christian Brauner
2024-09-19 23:55 ` Liviu Dudau
-- strict thread matches above, loose matches on Subject: below --
2024-08-09 10:38 Christian Brauner
2024-08-09 12:23 ` Jeff Layton
2024-08-09 17:14 ` Jan Kara
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).