* [PATCH 7/7] exec: open code copy_string_kernel
From: Christoph Hellwig @ 2020-04-21 15:42 UTC (permalink / raw)
To: Andrew Morton, Alexander Viro
Cc: Arnd Bergmann, linux-kernel, Jeremy Kerr, linux-fsdevel,
linuxppc-dev, Eric W . Biederman
In-Reply-To: <20200421154204.252921-1-hch@lst.de>
Currently copy_string_kernel is just a wrapper around copy_strings that
simplifies the calling conventions and uses set_fs to allow passing a
kernel pointer. But due to the fact the we only need to handle a single
kernel argument pointer, the logic can be sigificantly simplified while
getting rid of the set_fs.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/exec.c | 43 ++++++++++++++++++++++++++++++++++---------
1 file changed, 34 insertions(+), 9 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index b2a77d5acede..ea90af1fb236 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -592,17 +592,42 @@ static int copy_strings(int argc, struct user_arg_ptr argv,
*/
int copy_string_kernel(const char *arg, struct linux_binprm *bprm)
{
- int r;
- mm_segment_t oldfs = get_fs();
- struct user_arg_ptr argv = {
- .ptr.native = (const char __user *const __user *)&arg,
- };
+ int len = strnlen(arg, MAX_ARG_STRLEN) + 1 /* terminating NUL */;
+ unsigned long pos = bprm->p;
+
+ if (len == 0)
+ return -EFAULT;
+ if (!valid_arg_len(bprm, len))
+ return -E2BIG;
+
+ /* We're going to work our way backwards. */
+ arg += len;
+ bprm->p -= len;
+ if (IS_ENABLED(CONFIG_MMU) && bprm->p < bprm->argmin)
+ return -E2BIG;
+
+ while (len > 0) {
+ unsigned int bytes_to_copy = min_t(unsigned int, len,
+ min_not_zero(offset_in_page(pos), PAGE_SIZE));
+ struct page *page;
+ char *kaddr;
- set_fs(KERNEL_DS);
- r = copy_strings(1, argv, bprm);
- set_fs(oldfs);
+ pos -= bytes_to_copy;
+ arg -= bytes_to_copy;
+ len -= bytes_to_copy;
- return r;
+ page = get_arg_page(bprm, pos, 1);
+ if (!page)
+ return -E2BIG;
+ kaddr = kmap_atomic(page);
+ flush_arg_page(bprm, pos & PAGE_MASK, page);
+ memcpy(kaddr + offset_in_page(pos), arg, bytes_to_copy);
+ flush_kernel_dcache_page(page);
+ kunmap_atomic(kaddr);
+ put_arg_page(page);
+ }
+
+ return 0;
}
EXPORT_SYMBOL(copy_string_kernel);
--
2.26.1
^ permalink raw reply related
* [PATCH 6/7] exec: simplify the copy_strings_kernel calling convention
From: Christoph Hellwig @ 2020-04-21 15:42 UTC (permalink / raw)
To: Andrew Morton, Alexander Viro
Cc: Arnd Bergmann, linux-kernel, Jeremy Kerr, linux-fsdevel,
linuxppc-dev, Eric W . Biederman
In-Reply-To: <20200421154204.252921-1-hch@lst.de>
copy_strings_kernel is always used with a single argument,
adjust the calling convention to that.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/binfmt_em86.c | 6 +++---
fs/binfmt_misc.c | 4 ++--
fs/binfmt_script.c | 6 +++---
fs/exec.c | 13 ++++++-------
include/linux/binfmts.h | 3 +--
5 files changed, 15 insertions(+), 17 deletions(-)
diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c
index 466497860c62..f33fa668c91f 100644
--- a/fs/binfmt_em86.c
+++ b/fs/binfmt_em86.c
@@ -68,15 +68,15 @@ static int load_em86(struct linux_binprm *bprm)
* user environment and arguments are stored.
*/
remove_arg_zero(bprm);
- retval = copy_strings_kernel(1, &bprm->filename, bprm);
+ retval = copy_string_kernel(bprm->filename, bprm);
if (retval < 0) return retval;
bprm->argc++;
if (i_arg) {
- retval = copy_strings_kernel(1, &i_arg, bprm);
+ retval = copy_string_kernel(i_arg, bprm);
if (retval < 0) return retval;
bprm->argc++;
}
- retval = copy_strings_kernel(1, &i_name, bprm);
+ retval = copy_string_kernel(i_name, bprm);
if (retval < 0) return retval;
bprm->argc++;
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index cdb45829354d..b15257d8ff5e 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -190,13 +190,13 @@ static int load_misc_binary(struct linux_binprm *bprm)
bprm->file = NULL;
}
/* make argv[1] be the path to the binary */
- retval = copy_strings_kernel(1, &bprm->interp, bprm);
+ retval = copy_string_kernel(bprm->interp, bprm);
if (retval < 0)
goto error;
bprm->argc++;
/* add the interp as argv[0] */
- retval = copy_strings_kernel(1, &fmt->interpreter, bprm);
+ retval = copy_string_kernel(fmt->interpreter, bprm);
if (retval < 0)
goto error;
bprm->argc++;
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index e9e6a6f4a35f..c4fb7f52a46e 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -117,17 +117,17 @@ static int load_script(struct linux_binprm *bprm)
retval = remove_arg_zero(bprm);
if (retval)
return retval;
- retval = copy_strings_kernel(1, &bprm->interp, bprm);
+ retval = copy_string_kernel(bprm->interp, bprm);
if (retval < 0)
return retval;
bprm->argc++;
if (i_arg) {
- retval = copy_strings_kernel(1, &i_arg, bprm);
+ retval = copy_string_kernel(i_arg, bprm);
if (retval < 0)
return retval;
bprm->argc++;
}
- retval = copy_strings_kernel(1, &i_name, bprm);
+ retval = copy_string_kernel(i_name, bprm);
if (retval)
return retval;
bprm->argc++;
diff --git a/fs/exec.c b/fs/exec.c
index 06b4c550af5d..b2a77d5acede 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -588,24 +588,23 @@ static int copy_strings(int argc, struct user_arg_ptr argv,
}
/*
- * Like copy_strings, but get argv and its values from kernel memory.
+ * Copy and argument/environment string from the kernel to the processes stack.
*/
-int copy_strings_kernel(int argc, const char *const *__argv,
- struct linux_binprm *bprm)
+int copy_string_kernel(const char *arg, struct linux_binprm *bprm)
{
int r;
mm_segment_t oldfs = get_fs();
struct user_arg_ptr argv = {
- .ptr.native = (const char __user *const __user *)__argv,
+ .ptr.native = (const char __user *const __user *)&arg,
};
set_fs(KERNEL_DS);
- r = copy_strings(argc, argv, bprm);
+ r = copy_strings(1, argv, bprm);
set_fs(oldfs);
return r;
}
-EXPORT_SYMBOL(copy_strings_kernel);
+EXPORT_SYMBOL(copy_string_kernel);
#ifdef CONFIG_MMU
@@ -1863,7 +1862,7 @@ static int __do_execve_file(int fd, struct filename *filename,
if (retval < 0)
goto out;
- retval = copy_strings_kernel(1, &bprm->filename, bprm);
+ retval = copy_string_kernel(bprm->filename, bprm);
if (retval < 0)
goto out;
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index a345d9fed3d8..3d3afe094c97 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -144,8 +144,7 @@ extern int setup_arg_pages(struct linux_binprm * bprm,
extern int transfer_args_to_stack(struct linux_binprm *bprm,
unsigned long *sp_location);
extern int bprm_change_interp(const char *interp, struct linux_binprm *bprm);
-extern int copy_strings_kernel(int argc, const char *const *argv,
- struct linux_binprm *bprm);
+int copy_string_kernel(const char *arg, struct linux_binprm *bprm);
extern void install_exec_creds(struct linux_binprm *bprm);
extern void set_binfmt(struct linux_binfmt *new);
extern ssize_t read_code(struct file *, unsigned long, loff_t, size_t);
--
2.26.1
^ permalink raw reply related
* [PATCH 5/7] binfmt_elf_fdpic: remove the set_fs(KERNEL_DS) in elf_fdpic_core_dump
From: Christoph Hellwig @ 2020-04-21 15:42 UTC (permalink / raw)
To: Andrew Morton, Alexander Viro
Cc: Arnd Bergmann, linux-kernel, Jeremy Kerr, linux-fsdevel,
linuxppc-dev, Eric W . Biederman
In-Reply-To: <20200421154204.252921-1-hch@lst.de>
There is no logic in elf_fdpic_core_dump itself, or in the various arch
helpers called from it which use uaccess routines on kernel pointers
except for the file writes thate are nicely encapsulated by using
__kernel_write in dump_emit.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/binfmt_elf_fdpic.c | 31 ++++++++++++-------------------
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 240f66663543..c62c17a5c34a 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -1549,7 +1549,6 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
{
#define NUM_NOTES 6
int has_dumped = 0;
- mm_segment_t fs;
int segs;
int i;
struct vm_area_struct *vma;
@@ -1678,9 +1677,6 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
"LINUX", ELF_CORE_XFPREG_TYPE, sizeof(*xfpu), xfpu);
#endif
- fs = get_fs();
- set_fs(KERNEL_DS);
-
offset += sizeof(*elf); /* Elf header */
offset += segs * sizeof(struct elf_phdr); /* Program headers */
@@ -1695,7 +1691,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
phdr4note = kmalloc(sizeof(*phdr4note), GFP_KERNEL);
if (!phdr4note)
- goto end_coredump;
+ goto cleanup;
fill_elf_note_phdr(phdr4note, sz, offset);
offset += sz;
@@ -1711,17 +1707,17 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
if (e_phnum == PN_XNUM) {
shdr4extnum = kmalloc(sizeof(*shdr4extnum), GFP_KERNEL);
if (!shdr4extnum)
- goto end_coredump;
+ goto cleanup;
fill_extnum_info(elf, shdr4extnum, e_shoff, segs);
}
offset = dataoff;
if (!dump_emit(cprm, elf, sizeof(*elf)))
- goto end_coredump;
+ goto cleanup;
if (!dump_emit(cprm, phdr4note, sizeof(*phdr4note)))
- goto end_coredump;
+ goto cleanup;
/* write program headers for segments dump */
for (vma = current->mm->mmap; vma; vma = vma->vm_next) {
@@ -1745,16 +1741,16 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
phdr.p_align = ELF_EXEC_PAGESIZE;
if (!dump_emit(cprm, &phdr, sizeof(phdr)))
- goto end_coredump;
+ goto cleanup;
}
if (!elf_core_write_extra_phdrs(cprm, offset))
- goto end_coredump;
+ goto cleanup;
/* write out the notes section */
for (i = 0; i < numnote; i++)
if (!writenote(notes + i, cprm))
- goto end_coredump;
+ goto cleanup;
/* write out the thread status notes section */
list_for_each(t, &thread_list) {
@@ -1763,21 +1759,21 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
for (i = 0; i < tmp->num_notes; i++)
if (!writenote(&tmp->notes[i], cprm))
- goto end_coredump;
+ goto cleanup;
}
if (!dump_skip(cprm, dataoff - cprm->pos))
- goto end_coredump;
+ goto cleanup;
if (!elf_fdpic_dump_segments(cprm))
- goto end_coredump;
+ goto cleanup;
if (!elf_core_write_extra_data(cprm))
- goto end_coredump;
+ goto cleanup;
if (e_phnum == PN_XNUM) {
if (!dump_emit(cprm, shdr4extnum, sizeof(*shdr4extnum)))
- goto end_coredump;
+ goto cleanup;
}
if (cprm->file->f_pos != offset) {
@@ -1787,9 +1783,6 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
cprm->file->f_pos, offset);
}
-end_coredump:
- set_fs(fs);
-
cleanup:
while (!list_empty(&thread_list)) {
struct list_head *tmp = thread_list.next;
--
2.26.1
^ permalink raw reply related
* [PATCH 4/7] binfmt_elf: remove the set_fs(KERNEL_DS) in elf_core_dump
From: Christoph Hellwig @ 2020-04-21 15:42 UTC (permalink / raw)
To: Andrew Morton, Alexander Viro
Cc: Arnd Bergmann, linux-kernel, Jeremy Kerr, linux-fsdevel,
linuxppc-dev, Eric W . Biederman
In-Reply-To: <20200421154204.252921-1-hch@lst.de>
There is no logic in elf_core_dump itself, or in the various arch helpers
called from it which use uaccess routines on kernel pointers except for
the file writes thate are nicely encapsulated by using __kernel_write in
dump_emit.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/binfmt_elf.c | 40 +++++++++++++---------------------------
1 file changed, 13 insertions(+), 27 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index a1f57e20c3cf..b29b84595b09 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1355,7 +1355,6 @@ static unsigned long vma_dump_size(struct vm_area_struct *vma,
vma->vm_pgoff == 0 && (vma->vm_flags & VM_READ)) {
u32 __user *header = (u32 __user *) vma->vm_start;
u32 word;
- mm_segment_t fs = get_fs();
/*
* Doing it this way gets the constant folded by GCC.
*/
@@ -1368,14 +1367,8 @@ static unsigned long vma_dump_size(struct vm_area_struct *vma,
magic.elfmag[EI_MAG1] = ELFMAG1;
magic.elfmag[EI_MAG2] = ELFMAG2;
magic.elfmag[EI_MAG3] = ELFMAG3;
- /*
- * Switch to the user "segment" for get_user(),
- * then put back what elf_core_dump() had in place.
- */
- set_fs(USER_DS);
if (unlikely(get_user(word, header)))
word = 0;
- set_fs(fs);
if (word == magic.cmp)
return PAGE_SIZE;
}
@@ -2183,7 +2176,6 @@ static void fill_extnum_info(struct elfhdr *elf, struct elf_shdr *shdr4extnum,
static int elf_core_dump(struct coredump_params *cprm)
{
int has_dumped = 0;
- mm_segment_t fs;
int segs, i;
size_t vma_data_size = 0;
struct vm_area_struct *vma, *gate_vma;
@@ -2236,9 +2228,6 @@ static int elf_core_dump(struct coredump_params *cprm)
has_dumped = 1;
- fs = get_fs();
- set_fs(KERNEL_DS);
-
offset += sizeof(elf); /* Elf header */
offset += segs * sizeof(struct elf_phdr); /* Program headers */
@@ -2250,7 +2239,7 @@ static int elf_core_dump(struct coredump_params *cprm)
phdr4note = kmalloc(sizeof(*phdr4note), GFP_KERNEL);
if (!phdr4note)
- goto end_coredump;
+ goto cleanup;
fill_elf_note_phdr(phdr4note, sz, offset);
offset += sz;
@@ -2265,7 +2254,7 @@ static int elf_core_dump(struct coredump_params *cprm)
vma_filesz = kvmalloc(array_size(sizeof(*vma_filesz), (segs - 1)),
GFP_KERNEL);
if (!vma_filesz)
- goto end_coredump;
+ goto cleanup;
for (i = 0, vma = first_vma(current, gate_vma); vma != NULL;
vma = next_vma(vma, gate_vma)) {
@@ -2283,17 +2272,17 @@ static int elf_core_dump(struct coredump_params *cprm)
if (e_phnum == PN_XNUM) {
shdr4extnum = kmalloc(sizeof(*shdr4extnum), GFP_KERNEL);
if (!shdr4extnum)
- goto end_coredump;
+ goto cleanup;
fill_extnum_info(&elf, shdr4extnum, e_shoff, segs);
}
offset = dataoff;
if (!dump_emit(cprm, &elf, sizeof(elf)))
- goto end_coredump;
+ goto cleanup;
if (!dump_emit(cprm, phdr4note, sizeof(*phdr4note)))
- goto end_coredump;
+ goto cleanup;
/* Write program headers for segments dump */
for (i = 0, vma = first_vma(current, gate_vma); vma != NULL;
@@ -2315,22 +2304,22 @@ static int elf_core_dump(struct coredump_params *cprm)
phdr.p_align = ELF_EXEC_PAGESIZE;
if (!dump_emit(cprm, &phdr, sizeof(phdr)))
- goto end_coredump;
+ goto cleanup;
}
if (!elf_core_write_extra_phdrs(cprm, offset))
- goto end_coredump;
+ goto cleanup;
/* write out the notes section */
if (!write_note_info(&info, cprm))
- goto end_coredump;
+ goto cleanup;
if (elf_coredump_extra_notes_write(cprm))
- goto end_coredump;
+ goto cleanup;
/* Align to page */
if (!dump_skip(cprm, dataoff - cprm->pos))
- goto end_coredump;
+ goto cleanup;
for (i = 0, vma = first_vma(current, gate_vma); vma != NULL;
vma = next_vma(vma, gate_vma)) {
@@ -2352,22 +2341,19 @@ static int elf_core_dump(struct coredump_params *cprm)
} else
stop = !dump_skip(cprm, PAGE_SIZE);
if (stop)
- goto end_coredump;
+ goto cleanup;
}
}
dump_truncate(cprm);
if (!elf_core_write_extra_data(cprm))
- goto end_coredump;
+ goto cleanup;
if (e_phnum == PN_XNUM) {
if (!dump_emit(cprm, shdr4extnum, sizeof(*shdr4extnum)))
- goto end_coredump;
+ goto cleanup;
}
-end_coredump:
- set_fs(fs);
-
cleanup:
free_note_info(&info);
kfree(shdr4extnum);
--
2.26.1
^ permalink raw reply related
* [PATCH 3/7] binfmt_elf: femove the set_fs in fill_siginfo_note
From: Christoph Hellwig @ 2020-04-21 15:42 UTC (permalink / raw)
To: Andrew Morton, Alexander Viro
Cc: Arnd Bergmann, linux-kernel, Jeremy Kerr, linux-fsdevel,
linuxppc-dev, Eric W . Biederman
In-Reply-To: <20200421154204.252921-1-hch@lst.de>
From: "Eric W. Biederman" <ebiederm@xmission.com>
The code in binfmt_elf.c is differnt from the rest of the code that
processes siginfo, as it sends siginfo from a kernel buffer to a file
rather than from kernel memory to userspace buffers. To remove it's
use of set_fs the code needs some different siginfo helpers.
Add the helper copy_siginfo_to_external to copy from the kernel's
internal siginfo layout to a buffer in the siginfo layout that
userspace expects.
Modify fill_siginfo_note to use copy_siginfo_to_external instead of
set_fs and copy_siginfo_to_user.
Update compat_binfmt_elf.c to use the previously added
copy_siginfo_to_external32 to handle the compat case.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/binfmt_elf.c | 5 +----
fs/compat_binfmt_elf.c | 2 +-
include/linux/signal.h | 8 ++++++++
3 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 13f25e241ac4..a1f57e20c3cf 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1556,10 +1556,7 @@ static void fill_auxv_note(struct memelfnote *note, struct mm_struct *mm)
static void fill_siginfo_note(struct memelfnote *note, user_siginfo_t *csigdata,
const kernel_siginfo_t *siginfo)
{
- mm_segment_t old_fs = get_fs();
- set_fs(KERNEL_DS);
- copy_siginfo_to_user((user_siginfo_t __user *) csigdata, siginfo);
- set_fs(old_fs);
+ copy_siginfo_to_external(csigdata, siginfo);
fill_note(note, "CORE", NT_SIGINFO, sizeof(*csigdata), csigdata);
}
diff --git a/fs/compat_binfmt_elf.c b/fs/compat_binfmt_elf.c
index aaad4ca1217e..fa0e24e1b726 100644
--- a/fs/compat_binfmt_elf.c
+++ b/fs/compat_binfmt_elf.c
@@ -39,7 +39,7 @@
*/
#define user_long_t compat_long_t
#define user_siginfo_t compat_siginfo_t
-#define copy_siginfo_to_user copy_siginfo_to_user32
+#define copy_siginfo_to_external copy_siginfo_to_external32
/*
* The machine-dependent core note format types are defined in elfcore-compat.h,
diff --git a/include/linux/signal.h b/include/linux/signal.h
index 05bacd2ab135..6bb1a3f0258c 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -24,6 +24,14 @@ static inline void clear_siginfo(kernel_siginfo_t *info)
#define SI_EXPANSION_SIZE (sizeof(struct siginfo) - sizeof(struct kernel_siginfo))
+static inline void copy_siginfo_to_external(siginfo_t *to,
+ const kernel_siginfo_t *from)
+{
+ memcpy(to, from, sizeof(*from));
+ memset(((char *)to) + sizeof(struct kernel_siginfo), 0,
+ SI_EXPANSION_SIZE);
+}
+
int copy_siginfo_to_user(siginfo_t __user *to, const kernel_siginfo_t *from);
int copy_siginfo_from_user(kernel_siginfo_t *to, const siginfo_t __user *from);
--
2.26.1
^ permalink raw reply related
* remove set_fs calls from the exec and coredump code v3
From: Christoph Hellwig @ 2020-04-21 15:41 UTC (permalink / raw)
To: Andrew Morton, Alexander Viro
Cc: Arnd Bergmann, linux-kernel, Jeremy Kerr, linux-fsdevel,
linuxppc-dev, Eric W . Biederman
Hi all,
this series gets rid of playing with the address limit in the exec and
coredump code. Most of this was fairly trivial, the biggest changes are
those to the spufs coredump code.
Changes since v2:
- don't cleanup the compat siginfo calling conventions, use the patch
variant from Eric with slight coding style fixes instead.
Changes since v1:
- properly spell NUL
- properly handle the compat siginfo case in ELF coredumps
^ permalink raw reply
* [PATCH 2/7] signal: factor copy_siginfo_to_external32 from copy_siginfo_to_user32
From: Christoph Hellwig @ 2020-04-21 15:41 UTC (permalink / raw)
To: Andrew Morton, Alexander Viro
Cc: Arnd Bergmann, linux-kernel, Jeremy Kerr, linux-fsdevel,
linuxppc-dev, Eric W . Biederman
In-Reply-To: <20200421154204.252921-1-hch@lst.de>
From: "Eric W. Biederman" <ebiederm@xmission.com>
To remove the use of set_fs in the coredump code there needs to be a
way to convert a kernel siginfo to a userspace compat siginfo.
Call that function copy_siginfo_to_compat and factor it out of
copy_siginfo_to_user32.
The existence of x32 complicates this code. On x32 SIGCHLD uses 64bit
times for utime and stime. As only SIGCHLD is affected and SIGCHLD
never causes a coredump I have avoided handling that case.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
include/linux/compat.h | 2 +
kernel/signal.c | 109 +++++++++++++++++++++++------------------
2 files changed, 64 insertions(+), 47 deletions(-)
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 0480ba4db592..adbfe8f688d9 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -402,6 +402,8 @@ long compat_get_bitmap(unsigned long *mask, const compat_ulong_t __user *umask,
unsigned long bitmap_size);
long compat_put_bitmap(compat_ulong_t __user *umask, unsigned long *mask,
unsigned long bitmap_size);
+void copy_siginfo_to_external32(struct compat_siginfo *to,
+ const struct kernel_siginfo *from);
int copy_siginfo_from_user32(kernel_siginfo_t *to, const struct compat_siginfo __user *from);
int copy_siginfo_to_user32(struct compat_siginfo __user *to, const kernel_siginfo_t *from);
int get_compat_sigevent(struct sigevent *event,
diff --git a/kernel/signal.c b/kernel/signal.c
index 713104884414..d8eb30914771 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3231,94 +3231,109 @@ int copy_siginfo_from_user(kernel_siginfo_t *to, const siginfo_t __user *from)
}
#ifdef CONFIG_COMPAT
-int copy_siginfo_to_user32(struct compat_siginfo __user *to,
- const struct kernel_siginfo *from)
-#if defined(CONFIG_X86_X32_ABI) || defined(CONFIG_IA32_EMULATION)
-{
- return __copy_siginfo_to_user32(to, from, in_x32_syscall());
-}
-int __copy_siginfo_to_user32(struct compat_siginfo __user *to,
- const struct kernel_siginfo *from, bool x32_ABI)
-#endif
+void copy_siginfo_to_external32(struct compat_siginfo *to,
+ const struct kernel_siginfo *from)
{
- struct compat_siginfo new;
- memset(&new, 0, sizeof(new));
+ /*
+ * This function does not work properly for SIGCHLD on x32,
+ * but it does not need to as SIGCHLD never causes a coredump.
+ */
+ memset(to, 0, sizeof(*to));
- new.si_signo = from->si_signo;
- new.si_errno = from->si_errno;
- new.si_code = from->si_code;
+ to->si_signo = from->si_signo;
+ to->si_errno = from->si_errno;
+ to->si_code = from->si_code;
switch(siginfo_layout(from->si_signo, from->si_code)) {
case SIL_KILL:
- new.si_pid = from->si_pid;
- new.si_uid = from->si_uid;
+ to->si_pid = from->si_pid;
+ to->si_uid = from->si_uid;
break;
case SIL_TIMER:
- new.si_tid = from->si_tid;
- new.si_overrun = from->si_overrun;
- new.si_int = from->si_int;
+ to->si_tid = from->si_tid;
+ to->si_overrun = from->si_overrun;
+ to->si_int = from->si_int;
break;
case SIL_POLL:
- new.si_band = from->si_band;
- new.si_fd = from->si_fd;
+ to->si_band = from->si_band;
+ to->si_fd = from->si_fd;
break;
case SIL_FAULT:
- new.si_addr = ptr_to_compat(from->si_addr);
+ to->si_addr = ptr_to_compat(from->si_addr);
#ifdef __ARCH_SI_TRAPNO
- new.si_trapno = from->si_trapno;
+ to->si_trapno = from->si_trapno;
#endif
break;
case SIL_FAULT_MCEERR:
- new.si_addr = ptr_to_compat(from->si_addr);
+ to->si_addr = ptr_to_compat(from->si_addr);
#ifdef __ARCH_SI_TRAPNO
- new.si_trapno = from->si_trapno;
+ to->si_trapno = from->si_trapno;
#endif
- new.si_addr_lsb = from->si_addr_lsb;
+ to->si_addr_lsb = from->si_addr_lsb;
break;
case SIL_FAULT_BNDERR:
- new.si_addr = ptr_to_compat(from->si_addr);
+ to->si_addr = ptr_to_compat(from->si_addr);
#ifdef __ARCH_SI_TRAPNO
- new.si_trapno = from->si_trapno;
+ to->si_trapno = from->si_trapno;
#endif
- new.si_lower = ptr_to_compat(from->si_lower);
- new.si_upper = ptr_to_compat(from->si_upper);
+ to->si_lower = ptr_to_compat(from->si_lower);
+ to->si_upper = ptr_to_compat(from->si_upper);
break;
case SIL_FAULT_PKUERR:
- new.si_addr = ptr_to_compat(from->si_addr);
+ to->si_addr = ptr_to_compat(from->si_addr);
#ifdef __ARCH_SI_TRAPNO
- new.si_trapno = from->si_trapno;
+ to->si_trapno = from->si_trapno;
#endif
- new.si_pkey = from->si_pkey;
+ to->si_pkey = from->si_pkey;
break;
case SIL_CHLD:
- new.si_pid = from->si_pid;
- new.si_uid = from->si_uid;
- new.si_status = from->si_status;
+ to->si_pid = from->si_pid;
+ to->si_uid = from->si_uid;
+ to->si_status = from->si_status;
+ to->si_utime = from->si_utime;
+ to->si_stime = from->si_stime;
#ifdef CONFIG_X86_X32_ABI
if (x32_ABI) {
- new._sifields._sigchld_x32._utime = from->si_utime;
- new._sifields._sigchld_x32._stime = from->si_stime;
+ to->_sifields._sigchld_x32._utime = from->si_utime;
+ to->_sifields._sigchld_x32._stime = from->si_stime;
} else
#endif
{
- new.si_utime = from->si_utime;
- new.si_stime = from->si_stime;
}
break;
case SIL_RT:
- new.si_pid = from->si_pid;
- new.si_uid = from->si_uid;
- new.si_int = from->si_int;
+ to->si_pid = from->si_pid;
+ to->si_uid = from->si_uid;
+ to->si_int = from->si_int;
break;
case SIL_SYS:
- new.si_call_addr = ptr_to_compat(from->si_call_addr);
- new.si_syscall = from->si_syscall;
- new.si_arch = from->si_arch;
+ to->si_call_addr = ptr_to_compat(from->si_call_addr);
+ to->si_syscall = from->si_syscall;
+ to->si_arch = from->si_arch;
break;
}
+}
+
+int copy_siginfo_to_user32(struct compat_siginfo __user *to,
+ const struct kernel_siginfo *from)
+#if defined(CONFIG_X86_X32_ABI) || defined(CONFIG_IA32_EMULATION)
+{
+ return __copy_siginfo_to_user32(to, from, in_x32_syscall());
+}
+int __copy_siginfo_to_user32(struct compat_siginfo __user *to,
+ const struct kernel_siginfo *from, bool x32_ABI)
+#endif
+{
+ struct compat_siginfo new;
+ copy_siginfo_to_external32(&new, from);
+#ifdef CONFIG_X86_X32_ABI
+ if (x32_ABI && from->si_signo == SIGCHLD) {
+ new._sifields._sigchld_x32._utime = from->si_utime;
+ new._sifields._sigchld_x32._stime = from->si_stime;
+ }
+#endif
if (copy_to_user(to, &new, sizeof(struct compat_siginfo)))
return -EFAULT;
-
return 0;
}
--
2.26.1
^ permalink raw reply related
* [PATCH 1/7] powerpc/spufs: simplify spufs core dumping
From: Christoph Hellwig @ 2020-04-21 15:41 UTC (permalink / raw)
To: Andrew Morton, Alexander Viro
Cc: Arnd Bergmann, linux-kernel, Jeremy Kerr, linux-fsdevel,
linuxppc-dev, Eric W . Biederman
In-Reply-To: <20200421154204.252921-1-hch@lst.de>
Replace the coredump ->read method with a ->dump method that must call
dump_emit itself. That way we avoid a buffer allocation an messing with
set_fs() to call into code that is intended to deal with user buffers.
For the ->get case we can now use a small on-stack buffer and avoid
memory allocations as well.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Jeremy Kerr <jk@ozlabs.org>
---
arch/powerpc/platforms/cell/spufs/coredump.c | 87 ++----
arch/powerpc/platforms/cell/spufs/file.c | 273 ++++++++++---------
arch/powerpc/platforms/cell/spufs/spufs.h | 3 +-
3 files changed, 170 insertions(+), 193 deletions(-)
diff --git a/arch/powerpc/platforms/cell/spufs/coredump.c b/arch/powerpc/platforms/cell/spufs/coredump.c
index 8b3296b62f65..3b75e8f60609 100644
--- a/arch/powerpc/platforms/cell/spufs/coredump.c
+++ b/arch/powerpc/platforms/cell/spufs/coredump.c
@@ -21,22 +21,6 @@
#include "spufs.h"
-static ssize_t do_coredump_read(int num, struct spu_context *ctx, void *buffer,
- size_t size, loff_t *off)
-{
- u64 data;
- int ret;
-
- if (spufs_coredump_read[num].read)
- return spufs_coredump_read[num].read(ctx, buffer, size, off);
-
- data = spufs_coredump_read[num].get(ctx);
- ret = snprintf(buffer, size, "0x%.16llx", data);
- if (ret >= size)
- return size;
- return ++ret; /* count trailing NULL */
-}
-
static int spufs_ctx_note_size(struct spu_context *ctx, int dfd)
{
int i, sz, total = 0;
@@ -118,58 +102,43 @@ int spufs_coredump_extra_notes_size(void)
static int spufs_arch_write_note(struct spu_context *ctx, int i,
struct coredump_params *cprm, int dfd)
{
- loff_t pos = 0;
- int sz, rc, total = 0;
- const int bufsz = PAGE_SIZE;
- char *name;
- char fullname[80], *buf;
+ size_t sz = spufs_coredump_read[i].size;
+ char fullname[80];
struct elf_note en;
- size_t skip;
-
- buf = (void *)get_zeroed_page(GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
+ size_t ret;
- name = spufs_coredump_read[i].name;
- sz = spufs_coredump_read[i].size;
-
- sprintf(fullname, "SPU/%d/%s", dfd, name);
+ sprintf(fullname, "SPU/%d/%s", dfd, spufs_coredump_read[i].name);
en.n_namesz = strlen(fullname) + 1;
en.n_descsz = sz;
en.n_type = NT_SPU;
if (!dump_emit(cprm, &en, sizeof(en)))
- goto Eio;
-
+ return -EIO;
if (!dump_emit(cprm, fullname, en.n_namesz))
- goto Eio;
-
+ return -EIO;
if (!dump_align(cprm, 4))
- goto Eio;
-
- do {
- rc = do_coredump_read(i, ctx, buf, bufsz, &pos);
- if (rc > 0) {
- if (!dump_emit(cprm, buf, rc))
- goto Eio;
- total += rc;
- }
- } while (rc == bufsz && total < sz);
-
- if (rc < 0)
- goto out;
-
- skip = roundup(cprm->pos - total + sz, 4) - cprm->pos;
- if (!dump_skip(cprm, skip))
- goto Eio;
-
- rc = 0;
-out:
- free_page((unsigned long)buf);
- return rc;
-Eio:
- free_page((unsigned long)buf);
- return -EIO;
+ return -EIO;
+
+ if (spufs_coredump_read[i].dump) {
+ ret = spufs_coredump_read[i].dump(ctx, cprm);
+ if (ret < 0)
+ return ret;
+ } else {
+ char buf[32];
+
+ ret = snprintf(buf, sizeof(buf), "0x%.16llx",
+ spufs_coredump_read[i].get(ctx));
+ if (ret >= sizeof(buf))
+ return sizeof(buf);
+
+ /* count trailing the NULL: */
+ if (!dump_emit(cprm, buf, ret + 1))
+ return -EIO;
+ }
+
+ if (!dump_skip(cprm, roundup(cprm->pos - ret + sz, 4) - cprm->pos))
+ return -EIO;
+ return 0;
}
int spufs_coredump_extra_notes_write(struct coredump_params *cprm)
diff --git a/arch/powerpc/platforms/cell/spufs/file.c b/arch/powerpc/platforms/cell/spufs/file.c
index c0f950a3f4e1..0f8c3d692af0 100644
--- a/arch/powerpc/platforms/cell/spufs/file.c
+++ b/arch/powerpc/platforms/cell/spufs/file.c
@@ -9,6 +9,7 @@
#undef DEBUG
+#include <linux/coredump.h>
#include <linux/fs.h>
#include <linux/ioctl.h>
#include <linux/export.h>
@@ -129,6 +130,14 @@ static ssize_t spufs_attr_write(struct file *file, const char __user *buf,
return ret;
}
+static ssize_t spufs_dump_emit(struct coredump_params *cprm, void *buf,
+ size_t size)
+{
+ if (!dump_emit(cprm, buf, size))
+ return -EIO;
+ return size;
+}
+
#define DEFINE_SPUFS_SIMPLE_ATTRIBUTE(__fops, __get, __set, __fmt) \
static int __fops ## _open(struct inode *inode, struct file *file) \
{ \
@@ -172,12 +181,9 @@ spufs_mem_release(struct inode *inode, struct file *file)
}
static ssize_t
-__spufs_mem_read(struct spu_context *ctx, char __user *buffer,
- size_t size, loff_t *pos)
+spufs_mem_dump(struct spu_context *ctx, struct coredump_params *cprm)
{
- char *local_store = ctx->ops->get_ls(ctx);
- return simple_read_from_buffer(buffer, size, pos, local_store,
- LS_SIZE);
+ return spufs_dump_emit(cprm, ctx->ops->get_ls(ctx), LS_SIZE);
}
static ssize_t
@@ -190,7 +196,8 @@ spufs_mem_read(struct file *file, char __user *buffer,
ret = spu_acquire(ctx);
if (ret)
return ret;
- ret = __spufs_mem_read(ctx, buffer, size, pos);
+ ret = simple_read_from_buffer(buffer, size, pos, ctx->ops->get_ls(ctx),
+ LS_SIZE);
spu_release(ctx);
return ret;
@@ -459,12 +466,10 @@ spufs_regs_open(struct inode *inode, struct file *file)
}
static ssize_t
-__spufs_regs_read(struct spu_context *ctx, char __user *buffer,
- size_t size, loff_t *pos)
+spufs_regs_dump(struct spu_context *ctx, struct coredump_params *cprm)
{
- struct spu_lscsa *lscsa = ctx->csa.lscsa;
- return simple_read_from_buffer(buffer, size, pos,
- lscsa->gprs, sizeof lscsa->gprs);
+ return spufs_dump_emit(cprm, ctx->csa.lscsa->gprs,
+ sizeof(ctx->csa.lscsa->gprs));
}
static ssize_t
@@ -482,7 +487,8 @@ spufs_regs_read(struct file *file, char __user *buffer,
ret = spu_acquire_saved(ctx);
if (ret)
return ret;
- ret = __spufs_regs_read(ctx, buffer, size, pos);
+ ret = simple_read_from_buffer(buffer, size, pos, ctx->csa.lscsa->gprs,
+ sizeof(ctx->csa.lscsa->gprs));
spu_release_saved(ctx);
return ret;
}
@@ -517,12 +523,10 @@ static const struct file_operations spufs_regs_fops = {
};
static ssize_t
-__spufs_fpcr_read(struct spu_context *ctx, char __user * buffer,
- size_t size, loff_t * pos)
+spufs_fpcr_dump(struct spu_context *ctx, struct coredump_params *cprm)
{
- struct spu_lscsa *lscsa = ctx->csa.lscsa;
- return simple_read_from_buffer(buffer, size, pos,
- &lscsa->fpcr, sizeof(lscsa->fpcr));
+ return spufs_dump_emit(cprm, &ctx->csa.lscsa->fpcr,
+ sizeof(ctx->csa.lscsa->fpcr));
}
static ssize_t
@@ -535,7 +539,8 @@ spufs_fpcr_read(struct file *file, char __user * buffer,
ret = spu_acquire_saved(ctx);
if (ret)
return ret;
- ret = __spufs_fpcr_read(ctx, buffer, size, pos);
+ ret = simple_read_from_buffer(buffer, size, pos, &ctx->csa.lscsa->fpcr,
+ sizeof(ctx->csa.lscsa->fpcr));
spu_release_saved(ctx);
return ret;
}
@@ -967,28 +972,26 @@ spufs_signal1_release(struct inode *inode, struct file *file)
return 0;
}
-static ssize_t __spufs_signal1_read(struct spu_context *ctx, char __user *buf,
- size_t len, loff_t *pos)
+static ssize_t spufs_signal1_dump(struct spu_context *ctx,
+ struct coredump_params *cprm)
{
- int ret = 0;
- u32 data;
+ if (!ctx->csa.spu_chnlcnt_RW[3])
+ return 0;
+ return spufs_dump_emit(cprm, &ctx->csa.spu_chnldata_RW[3],
+ sizeof(ctx->csa.spu_chnldata_RW[3]));
+}
- if (len < 4)
+static ssize_t __spufs_signal1_read(struct spu_context *ctx, char __user *buf,
+ size_t len)
+{
+ if (len < sizeof(ctx->csa.spu_chnldata_RW[3]))
return -EINVAL;
-
- if (ctx->csa.spu_chnlcnt_RW[3]) {
- data = ctx->csa.spu_chnldata_RW[3];
- ret = 4;
- }
-
- if (!ret)
- goto out;
-
- if (copy_to_user(buf, &data, 4))
+ if (!ctx->csa.spu_chnlcnt_RW[3])
+ return 0;
+ if (copy_to_user(buf, &ctx->csa.spu_chnldata_RW[3],
+ sizeof(ctx->csa.spu_chnldata_RW[3])))
return -EFAULT;
-
-out:
- return ret;
+ return sizeof(ctx->csa.spu_chnldata_RW[3]);
}
static ssize_t spufs_signal1_read(struct file *file, char __user *buf,
@@ -1000,7 +1003,7 @@ static ssize_t spufs_signal1_read(struct file *file, char __user *buf,
ret = spu_acquire_saved(ctx);
if (ret)
return ret;
- ret = __spufs_signal1_read(ctx, buf, len, pos);
+ ret = __spufs_signal1_read(ctx, buf, len);
spu_release_saved(ctx);
return ret;
@@ -1104,28 +1107,26 @@ spufs_signal2_release(struct inode *inode, struct file *file)
return 0;
}
-static ssize_t __spufs_signal2_read(struct spu_context *ctx, char __user *buf,
- size_t len, loff_t *pos)
+static ssize_t spufs_signal2_dump(struct spu_context *ctx,
+ struct coredump_params *cprm)
{
- int ret = 0;
- u32 data;
+ if (!ctx->csa.spu_chnlcnt_RW[4])
+ return 0;
+ return spufs_dump_emit(cprm, &ctx->csa.spu_chnldata_RW[4],
+ sizeof(ctx->csa.spu_chnldata_RW[4]));
+}
- if (len < 4)
+static ssize_t __spufs_signal2_read(struct spu_context *ctx, char __user *buf,
+ size_t len)
+{
+ if (len < sizeof(ctx->csa.spu_chnldata_RW[4]))
return -EINVAL;
-
- if (ctx->csa.spu_chnlcnt_RW[4]) {
- data = ctx->csa.spu_chnldata_RW[4];
- ret = 4;
- }
-
- if (!ret)
- goto out;
-
- if (copy_to_user(buf, &data, 4))
+ if (!ctx->csa.spu_chnlcnt_RW[4])
+ return 0;
+ if (copy_to_user(buf, &ctx->csa.spu_chnldata_RW[4],
+ sizeof(ctx->csa.spu_chnldata_RW[4])))
return -EFAULT;
-
-out:
- return ret;
+ return sizeof(ctx->csa.spu_chnldata_RW[4]);
}
static ssize_t spufs_signal2_read(struct file *file, char __user *buf,
@@ -1137,7 +1138,7 @@ static ssize_t spufs_signal2_read(struct file *file, char __user *buf,
ret = spu_acquire_saved(ctx);
if (ret)
return ret;
- ret = __spufs_signal2_read(ctx, buf, len, pos);
+ ret = __spufs_signal2_read(ctx, buf, len);
spu_release_saved(ctx);
return ret;
@@ -1961,18 +1962,13 @@ static const struct file_operations spufs_caps_fops = {
.release = single_release,
};
-static ssize_t __spufs_mbox_info_read(struct spu_context *ctx,
- char __user *buf, size_t len, loff_t *pos)
+static ssize_t spufs_mbox_info_dump(struct spu_context *ctx,
+ struct coredump_params *cprm)
{
- u32 data;
-
- /* EOF if there's no entry in the mbox */
if (!(ctx->csa.prob.mb_stat_R & 0x0000ff))
return 0;
-
- data = ctx->csa.prob.pu_mb_R;
-
- return simple_read_from_buffer(buf, len, pos, &data, sizeof data);
+ return spufs_dump_emit(cprm, &ctx->csa.prob.pu_mb_R,
+ sizeof(ctx->csa.prob.pu_mb_R));
}
static ssize_t spufs_mbox_info_read(struct file *file, char __user *buf,
@@ -1988,7 +1984,12 @@ static ssize_t spufs_mbox_info_read(struct file *file, char __user *buf,
if (ret)
return ret;
spin_lock(&ctx->csa.register_lock);
- ret = __spufs_mbox_info_read(ctx, buf, len, pos);
+ /* EOF if there's no entry in the mbox */
+ if (ctx->csa.prob.mb_stat_R & 0x0000ff) {
+ ret = simple_read_from_buffer(buf, len, pos,
+ &ctx->csa.prob.pu_mb_R,
+ sizeof(ctx->csa.prob.pu_mb_R));
+ }
spin_unlock(&ctx->csa.register_lock);
spu_release_saved(ctx);
@@ -2001,18 +2002,13 @@ static const struct file_operations spufs_mbox_info_fops = {
.llseek = generic_file_llseek,
};
-static ssize_t __spufs_ibox_info_read(struct spu_context *ctx,
- char __user *buf, size_t len, loff_t *pos)
+static ssize_t spufs_ibox_info_dump(struct spu_context *ctx,
+ struct coredump_params *cprm)
{
- u32 data;
-
- /* EOF if there's no entry in the ibox */
if (!(ctx->csa.prob.mb_stat_R & 0xff0000))
return 0;
-
- data = ctx->csa.priv2.puint_mb_R;
-
- return simple_read_from_buffer(buf, len, pos, &data, sizeof data);
+ return spufs_dump_emit(cprm, &ctx->csa.priv2.puint_mb_R,
+ sizeof(ctx->csa.priv2.puint_mb_R));
}
static ssize_t spufs_ibox_info_read(struct file *file, char __user *buf,
@@ -2028,7 +2024,12 @@ static ssize_t spufs_ibox_info_read(struct file *file, char __user *buf,
if (ret)
return ret;
spin_lock(&ctx->csa.register_lock);
- ret = __spufs_ibox_info_read(ctx, buf, len, pos);
+ /* EOF if there's no entry in the ibox */
+ if (ctx->csa.prob.mb_stat_R & 0xff0000) {
+ ret = simple_read_from_buffer(buf, len, pos,
+ &ctx->csa.priv2.puint_mb_R,
+ sizeof(ctx->csa.priv2.puint_mb_R));
+ }
spin_unlock(&ctx->csa.register_lock);
spu_release_saved(ctx);
@@ -2041,21 +2042,16 @@ static const struct file_operations spufs_ibox_info_fops = {
.llseek = generic_file_llseek,
};
-static ssize_t __spufs_wbox_info_read(struct spu_context *ctx,
- char __user *buf, size_t len, loff_t *pos)
+static size_t spufs_wbox_info_cnt(struct spu_context *ctx)
{
- int i, cnt;
- u32 data[4];
- u32 wbox_stat;
-
- wbox_stat = ctx->csa.prob.mb_stat_R;
- cnt = 4 - ((wbox_stat & 0x00ff00) >> 8);
- for (i = 0; i < cnt; i++) {
- data[i] = ctx->csa.spu_mailbox_data[i];
- }
+ return (4 - ((ctx->csa.prob.mb_stat_R & 0x00ff00) >> 8)) * sizeof(u32);
+}
- return simple_read_from_buffer(buf, len, pos, &data,
- cnt * sizeof(u32));
+static ssize_t spufs_wbox_info_dump(struct spu_context *ctx,
+ struct coredump_params *cprm)
+{
+ return spufs_dump_emit(cprm, &ctx->csa.spu_mailbox_data,
+ spufs_wbox_info_cnt(ctx));
}
static ssize_t spufs_wbox_info_read(struct file *file, char __user *buf,
@@ -2071,7 +2067,8 @@ static ssize_t spufs_wbox_info_read(struct file *file, char __user *buf,
if (ret)
return ret;
spin_lock(&ctx->csa.register_lock);
- ret = __spufs_wbox_info_read(ctx, buf, len, pos);
+ ret = simple_read_from_buffer(buf, len, pos, &ctx->csa.spu_mailbox_data,
+ spufs_wbox_info_cnt(ctx));
spin_unlock(&ctx->csa.register_lock);
spu_release_saved(ctx);
@@ -2084,36 +2081,42 @@ static const struct file_operations spufs_wbox_info_fops = {
.llseek = generic_file_llseek,
};
-static ssize_t __spufs_dma_info_read(struct spu_context *ctx,
- char __user *buf, size_t len, loff_t *pos)
+static void __spufs_dma_info_read(struct spu_context *ctx,
+ struct spu_dma_info *info)
{
- struct spu_dma_info info;
- struct mfc_cq_sr *qp, *spuqp;
int i;
- info.dma_info_type = ctx->csa.priv2.spu_tag_status_query_RW;
- info.dma_info_mask = ctx->csa.lscsa->tag_mask.slot[0];
- info.dma_info_status = ctx->csa.spu_chnldata_RW[24];
- info.dma_info_stall_and_notify = ctx->csa.spu_chnldata_RW[25];
- info.dma_info_atomic_command_status = ctx->csa.spu_chnldata_RW[27];
+ info->dma_info_type = ctx->csa.priv2.spu_tag_status_query_RW;
+ info->dma_info_mask = ctx->csa.lscsa->tag_mask.slot[0];
+ info->dma_info_status = ctx->csa.spu_chnldata_RW[24];
+ info->dma_info_stall_and_notify = ctx->csa.spu_chnldata_RW[25];
+ info->dma_info_atomic_command_status = ctx->csa.spu_chnldata_RW[27];
+
for (i = 0; i < 16; i++) {
- qp = &info.dma_info_command_data[i];
- spuqp = &ctx->csa.priv2.spuq[i];
+ struct mfc_cq_sr *qp = &info->dma_info_command_data[i];
+ struct mfc_cq_sr *spuqp = &ctx->csa.priv2.spuq[i];
qp->mfc_cq_data0_RW = spuqp->mfc_cq_data0_RW;
qp->mfc_cq_data1_RW = spuqp->mfc_cq_data1_RW;
qp->mfc_cq_data2_RW = spuqp->mfc_cq_data2_RW;
qp->mfc_cq_data3_RW = spuqp->mfc_cq_data3_RW;
}
+}
+
+static ssize_t spufs_dma_info_dump(struct spu_context *ctx,
+ struct coredump_params *cprm)
+{
+ struct spu_dma_info info;
- return simple_read_from_buffer(buf, len, pos, &info,
- sizeof info);
+ __spufs_dma_info_read(ctx, &info);
+ return spufs_dump_emit(cprm, &info, sizeof(info));
}
static ssize_t spufs_dma_info_read(struct file *file, char __user *buf,
size_t len, loff_t *pos)
{
struct spu_context *ctx = file->private_data;
+ struct spu_dma_info info;
int ret;
if (!access_ok(buf, len))
@@ -2123,7 +2126,8 @@ static ssize_t spufs_dma_info_read(struct file *file, char __user *buf,
if (ret)
return ret;
spin_lock(&ctx->csa.register_lock);
- ret = __spufs_dma_info_read(ctx, buf, len, pos);
+ __spufs_dma_info_read(ctx, &info);
+ ret = simple_read_from_buffer(buf, len, pos, &info, sizeof(info));
spin_unlock(&ctx->csa.register_lock);
spu_release_saved(ctx);
@@ -2136,48 +2140,53 @@ static const struct file_operations spufs_dma_info_fops = {
.llseek = no_llseek,
};
-static ssize_t __spufs_proxydma_info_read(struct spu_context *ctx,
- char __user *buf, size_t len, loff_t *pos)
+static void __spufs_proxydma_info_read(struct spu_context *ctx,
+ struct spu_proxydma_info *info)
{
- struct spu_proxydma_info info;
- struct mfc_cq_sr *qp, *puqp;
- int ret = sizeof info;
int i;
- if (len < ret)
- return -EINVAL;
-
- if (!access_ok(buf, len))
- return -EFAULT;
+ info->proxydma_info_type = ctx->csa.prob.dma_querytype_RW;
+ info->proxydma_info_mask = ctx->csa.prob.dma_querymask_RW;
+ info->proxydma_info_status = ctx->csa.prob.dma_tagstatus_R;
- info.proxydma_info_type = ctx->csa.prob.dma_querytype_RW;
- info.proxydma_info_mask = ctx->csa.prob.dma_querymask_RW;
- info.proxydma_info_status = ctx->csa.prob.dma_tagstatus_R;
for (i = 0; i < 8; i++) {
- qp = &info.proxydma_info_command_data[i];
- puqp = &ctx->csa.priv2.puq[i];
+ struct mfc_cq_sr *qp = &info->proxydma_info_command_data[i];
+ struct mfc_cq_sr *puqp = &ctx->csa.priv2.puq[i];
qp->mfc_cq_data0_RW = puqp->mfc_cq_data0_RW;
qp->mfc_cq_data1_RW = puqp->mfc_cq_data1_RW;
qp->mfc_cq_data2_RW = puqp->mfc_cq_data2_RW;
qp->mfc_cq_data3_RW = puqp->mfc_cq_data3_RW;
}
+}
+
+static ssize_t spufs_proxydma_info_dump(struct spu_context *ctx,
+ struct coredump_params *cprm)
+{
+ struct spu_proxydma_info info;
- return simple_read_from_buffer(buf, len, pos, &info,
- sizeof info);
+ __spufs_proxydma_info_read(ctx, &info);
+ return spufs_dump_emit(cprm, &info, sizeof(info));
}
static ssize_t spufs_proxydma_info_read(struct file *file, char __user *buf,
size_t len, loff_t *pos)
{
struct spu_context *ctx = file->private_data;
+ struct spu_proxydma_info info;
int ret;
+ if (len < sizeof(info))
+ return -EINVAL;
+ if (!access_ok(buf, len))
+ return -EFAULT;
+
ret = spu_acquire_saved(ctx);
if (ret)
return ret;
spin_lock(&ctx->csa.register_lock);
- ret = __spufs_proxydma_info_read(ctx, buf, len, pos);
+ __spufs_proxydma_info_read(ctx, &info);
+ ret = simple_read_from_buffer(buf, len, pos, &info, sizeof(info));
spin_unlock(&ctx->csa.register_lock);
spu_release_saved(ctx);
@@ -2625,23 +2634,23 @@ const struct spufs_tree_descr spufs_dir_debug_contents[] = {
};
const struct spufs_coredump_reader spufs_coredump_read[] = {
- { "regs", __spufs_regs_read, NULL, sizeof(struct spu_reg128[128])},
- { "fpcr", __spufs_fpcr_read, NULL, sizeof(struct spu_reg128) },
+ { "regs", spufs_regs_dump, NULL, sizeof(struct spu_reg128[128])},
+ { "fpcr", spufs_fpcr_dump, NULL, sizeof(struct spu_reg128) },
{ "lslr", NULL, spufs_lslr_get, 19 },
{ "decr", NULL, spufs_decr_get, 19 },
{ "decr_status", NULL, spufs_decr_status_get, 19 },
- { "mem", __spufs_mem_read, NULL, LS_SIZE, },
- { "signal1", __spufs_signal1_read, NULL, sizeof(u32) },
+ { "mem", spufs_mem_dump, NULL, LS_SIZE, },
+ { "signal1", spufs_signal1_dump, NULL, sizeof(u32) },
{ "signal1_type", NULL, spufs_signal1_type_get, 19 },
- { "signal2", __spufs_signal2_read, NULL, sizeof(u32) },
+ { "signal2", spufs_signal2_dump, NULL, sizeof(u32) },
{ "signal2_type", NULL, spufs_signal2_type_get, 19 },
{ "event_mask", NULL, spufs_event_mask_get, 19 },
{ "event_status", NULL, spufs_event_status_get, 19 },
- { "mbox_info", __spufs_mbox_info_read, NULL, sizeof(u32) },
- { "ibox_info", __spufs_ibox_info_read, NULL, sizeof(u32) },
- { "wbox_info", __spufs_wbox_info_read, NULL, 4 * sizeof(u32)},
- { "dma_info", __spufs_dma_info_read, NULL, sizeof(struct spu_dma_info)},
- { "proxydma_info", __spufs_proxydma_info_read,
+ { "mbox_info", spufs_mbox_info_dump, NULL, sizeof(u32) },
+ { "ibox_info", spufs_ibox_info_dump, NULL, sizeof(u32) },
+ { "wbox_info", spufs_wbox_info_dump, NULL, 4 * sizeof(u32)},
+ { "dma_info", spufs_dma_info_dump, NULL, sizeof(struct spu_dma_info)},
+ { "proxydma_info", spufs_proxydma_info_dump,
NULL, sizeof(struct spu_proxydma_info)},
{ "object-id", NULL, spufs_object_id_get, 19 },
{ "npc", NULL, spufs_npc_get, 19 },
diff --git a/arch/powerpc/platforms/cell/spufs/spufs.h b/arch/powerpc/platforms/cell/spufs/spufs.h
index 413c89afe112..1ba4d884febf 100644
--- a/arch/powerpc/platforms/cell/spufs/spufs.h
+++ b/arch/powerpc/platforms/cell/spufs/spufs.h
@@ -337,8 +337,7 @@ void spufs_dma_callback(struct spu *spu, int type);
extern struct spu_coredump_calls spufs_coredump_calls;
struct spufs_coredump_reader {
char *name;
- ssize_t (*read)(struct spu_context *ctx,
- char __user *buffer, size_t size, loff_t *pos);
+ ssize_t (*dump)(struct spu_context *ctx, struct coredump_params *cprm);
u64 (*get)(struct spu_context *ctx);
size_t size;
};
--
2.26.1
^ permalink raw reply related
* RE: [musl] Powerpc Linux 'scv' system call ABI proposal take 2
From: David Laight @ 2020-04-21 15:31 UTC (permalink / raw)
To: 'Adhemerval Zanella', Rich Felker
Cc: libc-dev@lists.llvm.org, libc-alpha@sourceware.org,
linuxppc-dev@lists.ozlabs.org, 'Nicholas Piggin',
musl@lists.openwall.com
In-Reply-To: <960127e0-57a0-55b4-f309-ae0a675c7756@linaro.org>
From: Adhemerval Zanella
> Sent: 21 April 2020 16:01
>
> On 21/04/2020 11:39, Rich Felker wrote:
> > On Tue, Apr 21, 2020 at 12:28:25PM +0000, David Laight wrote:
> >> From: Nicholas Piggin
> >>> Sent: 20 April 2020 02:10
> >> ...
> >>>>> Yes, but does it really matter to optimize this specific usage case
> >>>>> for size? glibc, for instance, tries to leverage the syscall mechanism
> >>>>> by adding some complex pre-processor asm directives. It optimizes
> >>>>> the syscall code size in most cases. For instance, kill in static case
> >>>>> generates on x86_64:
> >>>>>
> >>>>> 0000000000000000 <__kill>:
> >>>>> 0: b8 3e 00 00 00 mov $0x3e,%eax
> >>>>> 5: 0f 05 syscall
> >>>>> 7: 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax
> >>>>> d: 0f 83 00 00 00 00 jae 13 <__kill+0x13>
> >>
> >> Hmmm... that cmp + jae is unnecessary here.
> >
> > It's not.. Rather the objdump was just mistakenly done without -r so
> > it looks like a nop jump rather than a conditional tail call to the
> > function that sets errno.
> >
>
> Indeed, the output with -r is:
>
> 0000000000000000 <__kill>:
> 0: b8 3e 00 00 00 mov $0x3e,%eax
> 5: 0f 05 syscall
> 7: 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax
> d: 0f 83 00 00 00 00 jae 13 <__kill+0x13>
> f: R_X86_64_PLT32 __syscall_error-0x4
> 13: c3 retq
Yes, I probably should have remembered it looked like that :-)
...
> >> I also suspect it gets predicted very badly.
> >
> > I doubt that. This is a very standard idiom and the size of the offset
> > (which is necessarily 32-bit because it has a relocation on it) is
> > orthogonal to the condition on the jump.
Yes, it only gets mispredicted as badly as any other conditional jump.
I believe modern intel x86 will randomly predict it taken (regardless
of the direction) and then hit a TLB fault on text.unlikely :-)
> > FWIW a syscall like kill takes global kernel-side locks to be able to
> > address a target process by pid, and the rate of meaningful calls you
> > can make to it is very low (since it's bounded by time for target
> > process to act on the signal). Trying to optimize it for speed is
> > pointless, and even size isn't important locally (although in
> > aggregate, lots of wasted small size can add up to more pages = more
> > TLB entries = ...).
>
> I agree and I would prefer to focus on code simplicity to have a
> platform neutral way to handle error and let the compiler optimize
> it than messy with assembly macros to squeeze this kind of
> micro-optimizations.
syscall entry does get micro-optimised.
Real speed-ups can probably be found by optimising other places.
I've a patch i need to resumbit that should improve the reading
of iov[] from user space.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [musl] Powerpc Linux 'scv' system call ABI proposal take 2
From: Adhemerval Zanella @ 2020-04-21 15:00 UTC (permalink / raw)
To: Rich Felker, David Laight
Cc: libc-dev@lists.llvm.org, libc-alpha@sourceware.org,
linuxppc-dev@lists.ozlabs.org, 'Nicholas Piggin',
musl@lists.openwall.com
In-Reply-To: <20200421143941.GJ11469@brightrain.aerifal.cx>
On 21/04/2020 11:39, Rich Felker wrote:
> On Tue, Apr 21, 2020 at 12:28:25PM +0000, David Laight wrote:
>> From: Nicholas Piggin
>>> Sent: 20 April 2020 02:10
>> ...
>>>>> Yes, but does it really matter to optimize this specific usage case
>>>>> for size? glibc, for instance, tries to leverage the syscall mechanism
>>>>> by adding some complex pre-processor asm directives. It optimizes
>>>>> the syscall code size in most cases. For instance, kill in static case
>>>>> generates on x86_64:
>>>>>
>>>>> 0000000000000000 <__kill>:
>>>>> 0: b8 3e 00 00 00 mov $0x3e,%eax
>>>>> 5: 0f 05 syscall
>>>>> 7: 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax
>>>>> d: 0f 83 00 00 00 00 jae 13 <__kill+0x13>
>>
>> Hmmm... that cmp + jae is unnecessary here.
>
> It's not.. Rather the objdump was just mistakenly done without -r so
> it looks like a nop jump rather than a conditional tail call to the
> function that sets errno.
>
Indeed, the output with -r is:
0000000000000000 <__kill>:
0: b8 3e 00 00 00 mov $0x3e,%eax
5: 0f 05 syscall
7: 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax
d: 0f 83 00 00 00 00 jae 13 <__kill+0x13>
f: R_X86_64_PLT32 __syscall_error-0x4
13: c3 retq
And for x86_64 __syscall_error is defined as:
0000000000000000 <__syscall_error>:
0: 48 f7 d8 neg %rax
0000000000000003 <__syscall_error_1>:
3: 64 89 04 25 00 00 00 mov %eax,%fs:0x0
a: 00
7: R_X86_64_TPOFF32 errno
b: 48 83 c8 ff or $0xffffffffffffffff,%rax
f: c3 retq
Different than musl, each architecture defines its own error handling
mechanism (some embedded errno setting in syscall itself, other branches
to a __syscall_error like function as x86_64).
This is due most likely from the glibc long history. One of my long
term plan is to just simplify, get rid of the assembly pre-processor,
implement all syscall in C code, and set error handling mechanism in
a platform neutral way using a tail call (most likely you do on musl).
>> It is also a 32bit offset jump.
>> I also suspect it gets predicted very badly.
>
> I doubt that. This is a very standard idiom and the size of the offset
> (which is necessarily 32-bit because it has a relocation on it) is
> orthogonal to the condition on the jump.
>
> FWIW a syscall like kill takes global kernel-side locks to be able to
> address a target process by pid, and the rate of meaningful calls you
> can make to it is very low (since it's bounded by time for target
> process to act on the signal). Trying to optimize it for speed is
> pointless, and even size isn't important locally (although in
> aggregate, lots of wasted small size can add up to more pages = more
> TLB entries = ...).
I agree and I would prefer to focus on code simplicity to have a
platform neutral way to handle error and let the compiler optimize
it than messy with assembly macros to squeeze this kind of
micro-optimizations.
>
>>>>> 13: c3 retq
>>>>>
>>>>> While on musl:
>>>>>
>>>>> 0000000000000000 <kill>:
>>>>> 0: 48 83 ec 08 sub $0x8,%rsp
>>>>> 4: 48 63 ff movslq %edi,%rdi
>>>>> 7: 48 63 f6 movslq %esi,%rsi
>>>>> a: b8 3e 00 00 00 mov $0x3e,%eax
>>>>> f: 0f 05 syscall
>>>>> 11: 48 89 c7 mov %rax,%rdi
>>>>> 14: e8 00 00 00 00 callq 19 <kill+0x19>
>>>>> 19: 5a pop %rdx
>>>>> 1a: c3 retq
>>>>
>>>> Wow that's some extraordinarily bad codegen going on by gcc... The
>>>> sign-extension is semantically needed and I don't see a good way
>>>> around it (glibc's asm is kinda a hack taking advantage of kernel not
>>>> looking at high bits, I think), but the gratuitous stack adjustment
>>>> and refusal to generate a tail call isn't. I'll see if we can track
>>>> down what's going on and get it fixed.
>>
>> A suitable cast might get rid of the sign extension.
>> Possibly just (unsigned int).
>
> No, it won't. The problem is that there is no representation of the
> fact that the kernel is only going to inspect the low 32 bits (by
> declaring the kernel-side function as taking an int argument). The
> external kill function receives arguments by the ABI, where the upper
> bits of int args can contain junk, and the asm register constraints
> for syscalls use longs (or rather an abstract syscall-arg type). It
> wouldn't even work to have macro magic detect that the expressions
> passed are ints and use hacks to avoid that, since it's perfectly
> valid to pass an int to a syscall that expects a long argument (e.g.
> offset to mmap), in which case it needs to be sign-extended.
>
> The only way to avoid this is encoding somewhere the syscall-specific
> knowledge of what arg size the kernel function expects. That's way too
> much redundant effort and too error-prone for the incredibly miniscule
> size benefit you'd get out of it.
>
> Rich
>
^ permalink raw reply
* Re: [musl] Powerpc Linux 'scv' system call ABI proposal take 2
From: Rich Felker @ 2020-04-21 14:39 UTC (permalink / raw)
To: David Laight
Cc: libc-alpha@sourceware.org, musl@lists.openwall.com,
'Nicholas Piggin', Adhemerval Zanella,
libc-dev@lists.llvm.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <b77fa2dc769d42e1a3e68f5edf90d250@AcuMS.aculab.com>
On Tue, Apr 21, 2020 at 12:28:25PM +0000, David Laight wrote:
> From: Nicholas Piggin
> > Sent: 20 April 2020 02:10
> ...
> > >> Yes, but does it really matter to optimize this specific usage case
> > >> for size? glibc, for instance, tries to leverage the syscall mechanism
> > >> by adding some complex pre-processor asm directives. It optimizes
> > >> the syscall code size in most cases. For instance, kill in static case
> > >> generates on x86_64:
> > >>
> > >> 0000000000000000 <__kill>:
> > >> 0: b8 3e 00 00 00 mov $0x3e,%eax
> > >> 5: 0f 05 syscall
> > >> 7: 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax
> > >> d: 0f 83 00 00 00 00 jae 13 <__kill+0x13>
>
> Hmmm... that cmp + jae is unnecessary here.
It's not.. Rather the objdump was just mistakenly done without -r so
it looks like a nop jump rather than a conditional tail call to the
function that sets errno.
> It is also a 32bit offset jump.
> I also suspect it gets predicted very badly.
I doubt that. This is a very standard idiom and the size of the offset
(which is necessarily 32-bit because it has a relocation on it) is
orthogonal to the condition on the jump.
FWIW a syscall like kill takes global kernel-side locks to be able to
address a target process by pid, and the rate of meaningful calls you
can make to it is very low (since it's bounded by time for target
process to act on the signal). Trying to optimize it for speed is
pointless, and even size isn't important locally (although in
aggregate, lots of wasted small size can add up to more pages = more
TLB entries = ...).
> > >> 13: c3 retq
> > >>
> > >> While on musl:
> > >>
> > >> 0000000000000000 <kill>:
> > >> 0: 48 83 ec 08 sub $0x8,%rsp
> > >> 4: 48 63 ff movslq %edi,%rdi
> > >> 7: 48 63 f6 movslq %esi,%rsi
> > >> a: b8 3e 00 00 00 mov $0x3e,%eax
> > >> f: 0f 05 syscall
> > >> 11: 48 89 c7 mov %rax,%rdi
> > >> 14: e8 00 00 00 00 callq 19 <kill+0x19>
> > >> 19: 5a pop %rdx
> > >> 1a: c3 retq
> > >
> > > Wow that's some extraordinarily bad codegen going on by gcc... The
> > > sign-extension is semantically needed and I don't see a good way
> > > around it (glibc's asm is kinda a hack taking advantage of kernel not
> > > looking at high bits, I think), but the gratuitous stack adjustment
> > > and refusal to generate a tail call isn't. I'll see if we can track
> > > down what's going on and get it fixed.
>
> A suitable cast might get rid of the sign extension.
> Possibly just (unsigned int).
No, it won't. The problem is that there is no representation of the
fact that the kernel is only going to inspect the low 32 bits (by
declaring the kernel-side function as taking an int argument). The
external kill function receives arguments by the ABI, where the upper
bits of int args can contain junk, and the asm register constraints
for syscalls use longs (or rather an abstract syscall-arg type). It
wouldn't even work to have macro magic detect that the expressions
passed are ints and use hacks to avoid that, since it's perfectly
valid to pass an int to a syscall that expects a long argument (e.g.
offset to mmap), in which case it needs to be sign-extended.
The only way to avoid this is encoding somewhere the syscall-specific
knowledge of what arg size the kernel function expects. That's way too
much redundant effort and too error-prone for the incredibly miniscule
size benefit you'd get out of it.
Rich
^ permalink raw reply
* Re: [PATCH 1/3] kexec: Prevent removal of memory in use by a loaded kexec image
From: David Hildenbrand @ 2020-04-21 14:30 UTC (permalink / raw)
To: Eric W. Biederman
Cc: piliu, Baoquan He, Anshuman Khandual, Catalin Marinas,
Bhupesh Sharma, linuxppc-dev, kexec,
Russell King - ARM Linux admin, linux-mm, James Morse,
Andrew Morton, Will Deacon, linux-arm-kernel
In-Reply-To: <87a735548w.fsf@x220.int.ebiederm.org>
>> b) "kexec -s -l" seems to work fine. For now, the kernel does not seem
>> to get placed on virtio-mem memory (pure luck due to the left-to-right
>> search). Memory added by virtio-mem is not getting added to the e820
>> map. Once the virtio-mem driver comes back up in the kexec kernel, the
>> right memory is readded.
>
> This sounds like a bug.
This is how virtio-mem wants its memory to get handled.
>
>> c) "kexec -c -l" does not work properly. All memory added by virtio-mem
>> is added to the e820 map, which is wrong. Memory that should not be
>> touched will be touched by the kexec kernel. I assume kexec-tools just
>> goes ahead and adds anything it can find in /proc/iomem (or
>> /sys/firmware/memmap/) to the e820 map of the new kernel.
>>
>> Due to c), I assume all hotplugged memory (e.g., ACPI DIMMs) is
>> similarly added to the e820 map and, therefore, won't be able to be
>> onlined MOVABLE easily.
>
> This sounds like correct behavior to me. If you add memory to the
> system it is treated as memory to the system.
Yeah, I would agree if we are talking about DIMMs, but this memory is
special. It's added via a paravirtualized interface and will contain
holes, especially after unplug. While memory in these holes can usually
be read, it should not be written. More on that below.
>
> If we need to make it a special kind of memory with special rules we can
> have some kind of special marking for the memory. But hotplugged is not
> in itself a sufficient criteria to say don't use this as normal memory.
Agreed. It is special, though.
>
> If take a huge server and I plug in an extra dimm it is just memory.
Agreed.
[...]
>
> Now perhaps virtualization needs a special tier of memory that should
> only be used for cases where the memory is easily movable.
>
> I am not familiar with virtio-mem but my skim of the initial design
> is that virtio-mem was not designed to be such a special tier of memory.
> Perhaps something has changed?
> https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg03870.html
Yes, a lot changed. See
https://lkml.kernel.org/r/20200311171422.10484-1-david@redhat.com for
the latest-greatest design overview.
>
>> b) Teach kexec-tools to leave virtio-mem added memory alone. E.g., by
>> indicating it in /proc/iomem in a special way ("System RAM
>> (hotplugged)"/"System RAM (virtio-mem)").
>
> How does the kernel memory allocator treat this memory?
So what virtio-mem does is add memory sections on demand and populate
within these sections the requested amount of memory. E.g., if 64MB are
requested, it will add a 128MB section/resource but only make the first
64MB accessible (via the hypervisor) and only give the first 64MB to the
buddy. This way of adding memory is similar to what XEN and hypver-v
balloon drivers do when hotplugging memory.
When requested to plug more memory, it might go ahead and make (parts
of) the remaining 64MB accessible and give them to the buddy. In case it
cannot "fill any holes", it will add a new section.
When requested to unplug memory, it will try to remove memory from the
added (here 64MB) memory from the buddy and tell the hypervisor about it.
So, it has some similarity to ballooning in virtual environment,
however, it manages its own device memory only and can therefore give
better guarantees and detect malicious guests.
Right now, I think the right approach would be to not create
/sys/firmware/memmap entries from memory virtio-mem added.
[...]
>
> p.s. Please excuse me for jumping in I may be missing some important
> context, but what I read when I saw this message in my inbox just seemed
> very wrong.
Yeah, still, thanks for having a look. Please let me know if you need
more information.
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH V4 1/5] selftests/powerpc: Add header files for GZIP engine test
From: Michael Ellerman @ 2020-04-21 14:09 UTC (permalink / raw)
To: Raphael Moreira Zinsly, linuxppc-dev, linux-crypto, dja
Cc: haren, abali, herbert, rzinsly
In-Reply-To: <20200420205538.25181-2-rzinsly@linux.ibm.com>
On Mon, 2020-04-20 at 20:55:34 UTC, Raphael Moreira Zinsly wrote:
> Add files to access the powerpc NX-GZIP engine in user space.
>
> Signed-off-by: Bulent Abali <abali@us.ibm.com>
> Signed-off-by: Raphael Moreira Zinsly <rzinsly@linux.ibm.com>
Series applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/d53979b589609d87036d8daf9500f7eccb0c6317
cheers
^ permalink raw reply
* Re: [PATCH v6 1/9] powerpc/vas: Initialize window attributes for GZIP coprocessor type
From: Michael Ellerman @ 2020-04-21 14:09 UTC (permalink / raw)
To: Haren Myneni
Cc: mikey, herbert, npiggin, linux-crypto, sukadev, linuxppc-dev, dja
In-Reply-To: <1587114029.2275.1103.camel@hbabu-laptop>
On Fri, 2020-04-17 at 09:00:29 UTC, Haren Myneni wrote:
>
> Initialize send and receive window attributes for GZIP high and
> normal priority types.
>
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
Series applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/a8c0c69b5e95e8f155480d5203a7bafb8024fd93
cheers
^ permalink raw reply
* Re: [PATCH v11 01/14] powerpc/xive: Define xive_native_alloc_irq_on_chip()
From: Michael Ellerman @ 2020-04-21 14:09 UTC (permalink / raw)
To: Haren Myneni
Cc: mikey, ajd, frederic.barrat, linux-kernel, npiggin, hch, oohall,
clg, herbert, sukadev, linuxppc-dev, srikar
In-Reply-To: <1587016720.2275.1047.camel@hbabu-laptop>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 416 bytes --]
On Thu, 2020-04-16 at 05:58:40 UTC, Haren Myneni wrote:
>
> This function allocates IRQ on a specific chip. VAS needs per chip
> IRQ allocation and will have IRQ handler per VAS instance.
>
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
Series applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/8d0ea29db5aefd0d94fa4b6ca6124c68998f3c6a
cheers
^ permalink raw reply
* Re: [PATCH 1/3] kexec: Prevent removal of memory in use by a loaded kexec image
From: Eric W. Biederman @ 2020-04-21 13:59 UTC (permalink / raw)
To: David Hildenbrand
Cc: piliu, Baoquan He, Anshuman Khandual, Catalin Marinas,
Bhupesh Sharma, linuxppc-dev, kexec,
Russell King - ARM Linux admin, linux-mm, James Morse,
Andrew Morton, Will Deacon, linux-arm-kernel
In-Reply-To: <9a4eb1d7-33bf-8707-9c0c-1ca657c3e502@redhat.com>
David Hildenbrand <david@redhat.com> writes:
>>> ACPI SRAT is embeded into efi, need read out the rsdp pointer. If we don't
>>> pass the efi, it won't get the SRAT table correctly, if I remember
>>> correctly. Yeah, I remeber kvm guest can get memory hotplugged with
>>> ACPI only, this won't happen on bare metal though. Need check carefully.
>>> I have been using kvm guest with uefi firmwire recently.
>>
>> Yeah, I can imagine that bare metal is different. kvm only uses ACPI.
>>
>> I'm also asking because of virtio-mem. Memory added via virtio-mem is
>> not part of any efi tables or whatsoever. So I assume the kexec kernel
>> will not detect it automatically (good!), instead load the virtio-mem
>> driver and let it add memory back to the system.
>>
>> I should probably play with kexec and virtio-mem once I have some spare
>> cycles ... to find out what's broken and needs to be addressed :)
>
> FWIW, I just gave virtio-mem and kexec/kdump a try.
>
> a) kdump seems to work. Memory added by virtio-mem is getting dumped.
> The kexec kernel only uses memory in the crash region. The virtio-mem
> driver properly bails out due to is_kdump_kernel().
>
> b) "kexec -s -l" seems to work fine. For now, the kernel does not seem
> to get placed on virtio-mem memory (pure luck due to the left-to-right
> search). Memory added by virtio-mem is not getting added to the e820
> map. Once the virtio-mem driver comes back up in the kexec kernel, the
> right memory is readded.
This sounds like a bug.
> c) "kexec -c -l" does not work properly. All memory added by virtio-mem
> is added to the e820 map, which is wrong. Memory that should not be
> touched will be touched by the kexec kernel. I assume kexec-tools just
> goes ahead and adds anything it can find in /proc/iomem (or
> /sys/firmware/memmap/) to the e820 map of the new kernel.
>
> Due to c), I assume all hotplugged memory (e.g., ACPI DIMMs) is
> similarly added to the e820 map and, therefore, won't be able to be
> onlined MOVABLE easily.
This sounds like correct behavior to me. If you add memory to the
system it is treated as memory to the system.
If we need to make it a special kind of memory with special rules we can
have some kind of special marking for the memory. But hotplugged is not
in itself a sufficient criteria to say don't use this as normal memory.
If take a huge server and I plug in an extra dimm it is just memory.
For a similarly huge server I might want to have memory that the system
booted with unpluggable, in case hardware error reporting notices
a dimm generating a lot of memory errors.
Now perhaps virtualization needs a special tier of memory that should
only be used for cases where the memory is easily movable.
I am not familiar with virtio-mem but my skim of the initial design
is that virtio-mem was not designed to be such a special tier of memory.
Perhaps something has changed?
https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg03870.html
> At least for virtio-mem, I would either have to
> a) Not support "kexec -c -l". A viable option if we would be planning on
> not supporting it either way in the long term. I could block this
> in-kernel somehow eventually.
No.
> b) Teach kexec-tools to leave virtio-mem added memory alone. E.g., by
> indicating it in /proc/iomem in a special way ("System RAM
> (hotplugged)"/"System RAM (virtio-mem)").
How does the kernel memory allocator treat this memory?
The logic is simple. If the kernel memory allocator treats that memory
as ordinary memory available for all uses it should be presented as
ordinary memory available for all uses.
If the kernel memory allocator treats that memory as special memory
only available for uses that we can easily free later and give back to
the system. AKA it is special and not oridinary memory we should mark
it as such.
Eric
p.s. Please excuse me for jumping in I may be missing some important
context, but what I read when I saw this message in my inbox just seemed
very wrong.
^ permalink raw reply
* Re: [PATCH v3 0/4] Clean up hugetlb boot command line processing
From: Gerald Schaefer @ 2020-04-21 14:02 UTC (permalink / raw)
To: Mike Kravetz
Cc: linux-doc, Catalin Marinas, Dave Hansen, Heiko Carstens, Peter Xu,
linux-mm, Paul Mackerras, sparclinux, linux-riscv, Will Deacon,
Mina Almasry, linux-s390, Jonathan Corbet, Christian Borntraeger,
Ingo Molnar, Gerald Schaefer, Longpeng, Albert Ou, Vasily Gorbik,
Paul Walmsley, Thomas Gleixner, linux-arm-kernel,
Nitesh Narayan Lal, Randy Dunlap, linux-kernel, Palmer Dabbelt,
Andrew Morton, linuxppc-dev, David S . Miller
In-Reply-To: <20200417185049.275845-1-mike.kravetz@oracle.com>
On Fri, 17 Apr 2020 11:50:45 -0700
Mike Kravetz <mike.kravetz@oracle.com> wrote:
> v3 -
> Used weak attribute method of defining arch_hugetlb_valid_size.
> This eliminates changes to arch specific hugetlb.h files (Peter)
> Updated documentation (Peter, Randy)
> Fixed handling of implicitly specified gigantic page preallocation
> in existing code and removed documentation of such. There is now
> no difference between handling of gigantic and non-gigantic pages.
> (Peter, Nitesh).
> This requires the most review as there is a small change to
> undocumented behavior. See patch 4 commit message for details.
> Added Acks and Reviews (Mina, Peter)
>
> v2 -
> Fix build errors with patch 1 (Will)
> Change arch_hugetlb_valid_size arg to unsigned long and remove
> irrelevant 'extern' keyword (Christophe)
> Documentation and other misc changes (Randy, Christophe, Mina)
> Do not process command line options if !hugepages_supported()
> (Dave, but it sounds like we may want to additional changes to
> hugepages_supported() for x86? If that is needed I would prefer
> a separate patch.)
>
> Longpeng(Mike) reported a weird message from hugetlb command line processing
> and proposed a solution [1]. While the proposed patch does address the
> specific issue, there are other related issues in command line processing.
> As hugetlbfs evolved, updates to command line processing have been made to
> meet immediate needs and not necessarily in a coordinated manner. The result
> is that some processing is done in arch specific code, some is done in arch
> independent code and coordination is problematic. Semantics can vary between
> architectures.
>
> The patch series does the following:
> - Define arch specific arch_hugetlb_valid_size routine used to validate
> passed huge page sizes.
> - Move hugepagesz= command line parsing out of arch specific code and into
> an arch independent routine.
> - Clean up command line processing to follow desired semantics and
> document those semantics.
>
> [1] https://lore.kernel.org/linux-mm/20200305033014.1152-1-longpeng2@huawei.com
>
> Mike Kravetz (4):
> hugetlbfs: add arch_hugetlb_valid_size
> hugetlbfs: move hugepagesz= parsing to arch independent code
> hugetlbfs: remove hugetlb_add_hstate() warning for existing hstate
> hugetlbfs: clean up command line processing
>
> .../admin-guide/kernel-parameters.txt | 40 ++--
> Documentation/admin-guide/mm/hugetlbpage.rst | 35 ++++
> arch/arm64/mm/hugetlbpage.c | 30 +--
> arch/powerpc/mm/hugetlbpage.c | 30 +--
> arch/riscv/mm/hugetlbpage.c | 24 +--
> arch/s390/mm/hugetlbpage.c | 24 +--
> arch/sparc/mm/init_64.c | 43 +---
> arch/x86/mm/hugetlbpage.c | 23 +--
> include/linux/hugetlb.h | 2 +-
> mm/hugetlb.c | 190 +++++++++++++++---
> 10 files changed, 271 insertions(+), 170 deletions(-)
>
Looks good and works fine for s390, thanks for cleaning up!
Acked-by: Gerald Schaefer <gerald.schaefer@de.ibm.com> # s390
^ permalink raw reply
* [PATCH v2 7/7] tracefs: switch to simplefs inode creation API
From: Emanuele Giuseppe Esposito @ 2020-04-21 13:57 UTC (permalink / raw)
To: linux-fsdevel
Cc: Rafael J. Wysocki, David Airlie, dri-devel, Christoph Hellwig,
Andrew Donnellan, Emanuele Giuseppe Esposito, linux-scsi,
James Morris, Serge E. Hallyn, Daniel Vetter, Arnd Bergmann,
James E.J. Bottomley, Maarten Lankhorst, Maxime Ripard,
Manoj N. Kumar, Alexander Viro, Matthew R. Ochs, Uma Krishnan,
John Johansen, Martin K. Petersen, Greg Kroah-Hartman,
linux-kernel, linux-security-module, Thomas Zimmermann,
Frederic Barrat, Paolo Bonzini, linuxppc-dev, Joel Becker
In-Reply-To: <20200421135119.30007-1-eesposit@redhat.com>
There is no semantic change intended; the code in the libfs.c
functions in fact was derived from debugfs and tracefs code.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
fs/tracefs/inode.c | 86 ++++------------------------------------------
1 file changed, 7 insertions(+), 79 deletions(-)
diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
index 370eb38ff1ad..bceaa4f45da2 100644
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -308,57 +308,6 @@ static struct file_system_type trace_fs_type = {
};
MODULE_ALIAS_FS("tracefs");
-static struct dentry *start_creating(const char *name, struct dentry *parent)
-{
- struct dentry *dentry;
- int error;
-
- pr_debug("tracefs: creating file '%s'\n",name);
-
- error = simple_pin_fs(&tracefs, &trace_fs_type);
- if (error)
- return ERR_PTR(error);
-
- /* If the parent is not specified, we create it in the root.
- * We need the root dentry to do this, which is in the super
- * block. A pointer to that is in the struct vfsmount that we
- * have around.
- */
- if (!parent)
- parent = tracefs.mount->mnt_root;
-
- inode_lock(parent->d_inode);
- if (unlikely(IS_DEADDIR(parent->d_inode)))
- dentry = ERR_PTR(-ENOENT);
- else
- dentry = lookup_one_len(name, parent, strlen(name));
- if (!IS_ERR(dentry) && dentry->d_inode) {
- dput(dentry);
- dentry = ERR_PTR(-EEXIST);
- }
-
- if (IS_ERR(dentry)) {
- inode_unlock(parent->d_inode);
- simple_release_fs(&tracefs);
- }
-
- return dentry;
-}
-
-static struct dentry *failed_creating(struct dentry *dentry)
-{
- inode_unlock(dentry->d_parent->d_inode);
- dput(dentry);
- simple_release_fs(&tracefs);
- return NULL;
-}
-
-static struct dentry *end_creating(struct dentry *dentry)
-{
- inode_unlock(dentry->d_parent->d_inode);
- return dentry;
-}
-
/**
* tracefs_create_file - create a file in the tracefs filesystem
* @name: a pointer to a string containing the name of the file to create.
@@ -395,49 +344,28 @@ struct dentry *tracefs_create_file(const char *name, umode_t mode,
if (security_locked_down(LOCKDOWN_TRACEFS))
return NULL;
- if (!(mode & S_IFMT))
- mode |= S_IFREG;
- BUG_ON(!S_ISREG(mode));
- dentry = start_creating(name, parent);
-
+ dentry = simplefs_create_file(&tracefs, &trace_fs_type,
+ name, mode, parent, data, &inode);
if (IS_ERR(dentry))
return NULL;
- inode = tracefs_get_inode(dentry->d_sb);
- if (unlikely(!inode))
- return failed_creating(dentry);
-
- inode->i_mode = mode;
inode->i_fop = fops ? fops : &tracefs_file_operations;
- inode->i_private = data;
- d_instantiate(dentry, inode);
- fsnotify_create(dentry->d_parent->d_inode, dentry);
- return end_creating(dentry);
+ return simplefs_finish_dentry(dentry, inode);
}
static struct dentry *__create_dir(const char *name, struct dentry *parent,
const struct inode_operations *ops)
{
- struct dentry *dentry = start_creating(name, parent);
+ struct dentry *dentry;
struct inode *inode;
+ dentry = simplefs_create_dir(&tracefs, &trace_fs_type,
+ name, 0755, parent, &inode);
if (IS_ERR(dentry))
return NULL;
- inode = tracefs_get_inode(dentry->d_sb);
- if (unlikely(!inode))
- return failed_creating(dentry);
-
- inode->i_mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO;
inode->i_op = ops;
- inode->i_fop = &simple_dir_operations;
-
- /* directory inodes start off with i_nlink == 2 (for "." entry) */
- inc_nlink(inode);
- d_instantiate(dentry, inode);
- inc_nlink(dentry->d_parent->d_inode);
- fsnotify_mkdir(dentry->d_parent->d_inode, dentry);
- return end_creating(dentry);
+ return simplefs_finish_dentry(dentry, inode);
}
/**
--
2.25.2
^ permalink raw reply related
* [PATCH v2 6/7] debugfs: switch to simplefs inode creation API
From: Emanuele Giuseppe Esposito @ 2020-04-21 13:57 UTC (permalink / raw)
To: linux-fsdevel
Cc: Rafael J. Wysocki, David Airlie, dri-devel, Christoph Hellwig,
Andrew Donnellan, Emanuele Giuseppe Esposito, linux-scsi,
James Morris, Serge E. Hallyn, Daniel Vetter, Arnd Bergmann,
James E.J. Bottomley, Maarten Lankhorst, Maxime Ripard,
Manoj N. Kumar, Alexander Viro, Matthew R. Ochs, Uma Krishnan,
John Johansen, Martin K. Petersen, Greg Kroah-Hartman,
linux-kernel, linux-security-module, Thomas Zimmermann,
Frederic Barrat, Paolo Bonzini, linuxppc-dev, Joel Becker
In-Reply-To: <20200421135119.30007-1-eesposit@redhat.com>
The only difference, compared to the pre-existing code, is that symlink
creation now triggers fsnotify_create. This was a bug in the debugfs
code, since for example vfs_symlink does call fsnotify_create.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
fs/debugfs/inode.c | 144 +++++----------------------------------------
1 file changed, 15 insertions(+), 129 deletions(-)
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 5dbb74a23e7c..ccbeea9e5f6c 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -305,68 +305,6 @@ struct dentry *debugfs_lookup(const char *name, struct dentry *parent)
}
EXPORT_SYMBOL_GPL(debugfs_lookup);
-static struct dentry *start_creating(const char *name, struct dentry *parent)
-{
- struct dentry *dentry;
- int error;
-
- pr_debug("creating file '%s'\n", name);
-
- if (IS_ERR(parent))
- return parent;
-
- error = simple_pin_fs(&debugfs, &debug_fs_type);
- if (error) {
- pr_err("Unable to pin filesystem for file '%s'\n", name);
- return ERR_PTR(error);
- }
-
- /* If the parent is not specified, we create it in the root.
- * We need the root dentry to do this, which is in the super
- * block. A pointer to that is in the struct vfsmount that we
- * have around.
- */
- if (!parent)
- parent = debugfs.mount->mnt_root;
-
- inode_lock(d_inode(parent));
- if (unlikely(IS_DEADDIR(d_inode(parent))))
- dentry = ERR_PTR(-ENOENT);
- else
- dentry = lookup_one_len(name, parent, strlen(name));
- if (!IS_ERR(dentry) && d_really_is_positive(dentry)) {
- if (d_is_dir(dentry))
- pr_err("Directory '%s' with parent '%s' already present!\n",
- name, parent->d_name.name);
- else
- pr_err("File '%s' in directory '%s' already present!\n",
- name, parent->d_name.name);
- dput(dentry);
- dentry = ERR_PTR(-EEXIST);
- }
-
- if (IS_ERR(dentry)) {
- inode_unlock(d_inode(parent));
- simple_release_fs(&debugfs);
- }
-
- return dentry;
-}
-
-static struct dentry *failed_creating(struct dentry *dentry)
-{
- inode_unlock(d_inode(dentry->d_parent));
- dput(dentry);
- simple_release_fs(&debugfs);
- return ERR_PTR(-ENOMEM);
-}
-
-static struct dentry *end_creating(struct dentry *dentry)
-{
- inode_unlock(d_inode(dentry->d_parent));
- return dentry;
-}
-
static struct dentry *__debugfs_create_file(const char *name, umode_t mode,
struct dentry *parent, void *data,
const struct file_operations *proxy_fops,
@@ -375,32 +313,17 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode,
struct dentry *dentry;
struct inode *inode;
- if (!(mode & S_IFMT))
- mode |= S_IFREG;
- BUG_ON(!S_ISREG(mode));
- dentry = start_creating(name, parent);
-
+ dentry = simplefs_create_file(&debugfs, &debug_fs_type,
+ name, mode, parent, data, &inode);
if (IS_ERR(dentry))
return dentry;
- inode = debugfs_get_inode(dentry->d_sb);
- if (unlikely(!inode)) {
- pr_err("out of free dentries, can not create file '%s'\n",
- name);
- return failed_creating(dentry);
- }
-
- inode->i_mode = mode;
- inode->i_private = data;
-
inode->i_op = &debugfs_file_inode_operations;
inode->i_fop = proxy_fops;
dentry->d_fsdata = (void *)((unsigned long)real_fops |
DEBUGFS_FSDATA_IS_REAL_FOPS_BIT);
- d_instantiate(dentry, inode);
- fsnotify_create(d_inode(dentry->d_parent), dentry);
- return end_creating(dentry);
+ return simplefs_finish_dentry(dentry, inode);
}
/**
@@ -533,29 +456,16 @@ EXPORT_SYMBOL_GPL(debugfs_create_file_size);
*/
struct dentry *debugfs_create_dir(const char *name, struct dentry *parent)
{
- struct dentry *dentry = start_creating(name, parent);
+ struct dentry *dentry;
struct inode *inode;
+ dentry = simplefs_create_dir(&debugfs, &debug_fs_type,
+ name, 0755, parent, &inode);
if (IS_ERR(dentry))
return dentry;
- inode = debugfs_get_inode(dentry->d_sb);
- if (unlikely(!inode)) {
- pr_err("out of free dentries, can not create directory '%s'\n",
- name);
- return failed_creating(dentry);
- }
-
- inode->i_mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO;
inode->i_op = &debugfs_dir_inode_operations;
- inode->i_fop = &simple_dir_operations;
-
- /* directory inodes start off with i_nlink == 2 (for "." entry) */
- inc_nlink(inode);
- d_instantiate(dentry, inode);
- inc_nlink(d_inode(dentry->d_parent));
- fsnotify_mkdir(d_inode(dentry->d_parent), dentry);
- return end_creating(dentry);
+ return simplefs_finish_dentry(dentry, inode);
}
EXPORT_SYMBOL_GPL(debugfs_create_dir);
@@ -575,29 +485,19 @@ struct dentry *debugfs_create_automount(const char *name,
debugfs_automount_t f,
void *data)
{
- struct dentry *dentry = start_creating(name, parent);
+ struct dentry *dentry;
struct inode *inode;
+ dentry = simplefs_create_dentry(&debugfs, &debug_fs_type, name, parent,
+ &inode);
if (IS_ERR(dentry))
return dentry;
- inode = debugfs_get_inode(dentry->d_sb);
- if (unlikely(!inode)) {
- pr_err("out of free dentries, can not create automount '%s'\n",
- name);
- return failed_creating(dentry);
- }
-
make_empty_dir_inode(inode);
inode->i_flags |= S_AUTOMOUNT;
inode->i_private = data;
dentry->d_fsdata = (void *)f;
- /* directory inodes start off with i_nlink == 2 (for "." entry) */
- inc_nlink(inode);
- d_instantiate(dentry, inode);
- inc_nlink(d_inode(dentry->d_parent));
- fsnotify_mkdir(d_inode(dentry->d_parent), dentry);
- return end_creating(dentry);
+ return simplefs_finish_dentry(dentry, inode);
}
EXPORT_SYMBOL(debugfs_create_automount);
@@ -629,28 +529,14 @@ struct dentry *debugfs_create_symlink(const char *name, struct dentry *parent,
{
struct dentry *dentry;
struct inode *inode;
- char *link = kstrdup(target, GFP_KERNEL);
- if (!link)
- return ERR_PTR(-ENOMEM);
- dentry = start_creating(name, parent);
- if (IS_ERR(dentry)) {
- kfree(link);
+ dentry = simplefs_create_symlink(&debugfs, &debug_fs_type,
+ name, parent, target, &inode);
+ if (IS_ERR(dentry))
return dentry;
- }
- inode = debugfs_get_inode(dentry->d_sb);
- if (unlikely(!inode)) {
- pr_err("out of free dentries, can not create symlink '%s'\n",
- name);
- kfree(link);
- return failed_creating(dentry);
- }
- inode->i_mode = S_IFLNK | S_IRWXUGO;
inode->i_op = &debugfs_symlink_inode_operations;
- inode->i_link = link;
- d_instantiate(dentry, inode);
- return end_creating(dentry);
+ return simplefs_finish_dentry(dentry, inode);
}
EXPORT_SYMBOL_GPL(debugfs_create_symlink);
--
2.25.2
^ permalink raw reply related
* [PATCH v2 5/7] libfs: add file creation functions
From: Emanuele Giuseppe Esposito @ 2020-04-21 13:57 UTC (permalink / raw)
To: linux-fsdevel
Cc: Rafael J. Wysocki, David Airlie, dri-devel, Christoph Hellwig,
Andrew Donnellan, Emanuele Giuseppe Esposito, linux-scsi,
James Morris, Serge E. Hallyn, Daniel Vetter, Arnd Bergmann,
James E.J. Bottomley, Maarten Lankhorst, Maxime Ripard,
Manoj N. Kumar, Alexander Viro, Matthew R. Ochs, Uma Krishnan,
John Johansen, Martin K. Petersen, Greg Kroah-Hartman,
linux-kernel, linux-security-module, Thomas Zimmermann,
Frederic Barrat, Paolo Bonzini, linuxppc-dev, Joel Becker
In-Reply-To: <20200421135119.30007-1-eesposit@redhat.com>
A bunch of code is duplicated between debugfs and tracefs, unify it to the
libfs library.
The code is very similar, except that dentry and inode creation are unified
into a single function (unlike start_creating in debugfs and tracefs, which
only takes care of dentries). This adds an output parameter to the
creation functions, but pushes all error recovery into fs/libfs.c.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
fs/libfs.c | 226 +++++++++++++++++++++++++++++++++++++++++++++
include/linux/fs.h | 18 ++++
2 files changed, 244 insertions(+)
diff --git a/fs/libfs.c b/fs/libfs.c
index 5c76e4c648dc..90b0c221d9a2 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -751,6 +751,232 @@ struct inode *simple_alloc_anon_inode(struct simple_fs *fs)
}
EXPORT_SYMBOL(simple_alloc_anon_inode);
+static struct dentry *failed_creating(struct simple_fs *fs, struct dentry *dentry)
+{
+ inode_unlock(d_inode(dentry->d_parent));
+ dput(dentry);
+ simple_release_fs(fs);
+ return ERR_PTR(-ENOMEM);
+}
+
+/**
+ * simplefs_create_dentry - creates a new dentry and inode
+ * @fs: a pointer to a struct simple_fs containing the reference counter
+ * and vfs_mount pointer
+ * @type: the fs type
+ * @name: dentry name
+ * @parent: parent dentry. If this parameter is NULL,
+ * then the dentry will be created in the root of the
+ * filesystem.
+ * @inode: pointer that will contain a newly created inode
+ *
+ * This function returns a new dentry, or NULL on error. On success, a
+ * new inode is created and stored into @inode. Also note that the inode
+ * for the parent directory is locked by simplefs_create_dentry(),
+ * and will be unlocked by simple_finish_dentry().
+ **/
+struct dentry *simplefs_create_dentry(struct simple_fs *fs, struct file_system_type *type,
+ const char *name, struct dentry *parent,
+ struct inode **inode)
+{
+ struct dentry *dentry;
+ int error;
+
+ pr_debug("creating file '%s'\n", name);
+
+ if (IS_ERR(parent))
+ return parent;
+
+ error = simple_pin_fs(fs, type);
+ if (error) {
+ pr_err("Unable to pin filesystem for file '%s'\n", name);
+ return ERR_PTR(error);
+ }
+
+ /* If the parent is not specified, we create it in the root.
+ * We need the root dentry to do this, which is in the super
+ * block. A pointer to that is in the struct vfsmount that we
+ * have around.
+ */
+ if (!parent)
+ parent = fs->mount->mnt_root;
+
+ inode_lock(d_inode(parent));
+ dentry = lookup_one_len(name, parent, strlen(name));
+ if (!IS_ERR(dentry) && d_really_is_positive(dentry)) {
+ if (d_is_dir(dentry))
+ pr_err("Directory '%s' with parent '%s' already present!\n",
+ name, parent->d_name.name);
+ else
+ pr_err("File '%s' in directory '%s' already present!\n",
+ name, parent->d_name.name);
+ dput(dentry);
+ dentry = ERR_PTR(-EEXIST);
+ }
+
+ if (IS_ERR(dentry)) {
+ inode_unlock(d_inode(parent));
+ simple_release_fs(fs);
+ }
+
+
+ if (IS_ERR(dentry))
+ return dentry;
+
+ *inode = new_inode_current_time(fs->mount->mnt_sb);
+ if (unlikely(!(*inode))) {
+ pr_err("out of free inodes, can not create file '%s'\n",
+ name);
+ return failed_creating(fs, dentry);
+ }
+
+ return dentry;
+}
+EXPORT_SYMBOL(simplefs_create_dentry);
+
+/**
+ * simplefs_create_file - creates a new file dentry and inode
+ * @fs: a pointer to a struct simple_fs containing the reference counter
+ * and vfs_mount pointer
+ * @type: the fs type
+ * @name: file name
+ * @mode: file mode
+ * @parent: parent dentry. If this parameter is NULL,
+ * then the file will be created in the root of the
+ * filesystem.
+ * @data: what will the file contain
+ * @inode: pointer that will contain a newly created inode
+ *
+ * This function returns a new dentry, or NULL on error. On success, a
+ * new inode is created and stored into @inode. Also note that the inode
+ * for the parent directory is locked by simplefs_create_dentry(),
+ * and will be unlocked by simple_finish_dentry().
+ **/
+struct dentry *simplefs_create_file(struct simple_fs *fs, struct file_system_type *type,
+ const char *name, umode_t mode,
+ struct dentry *parent, void *data,
+ struct inode **inode)
+{
+ struct dentry *dentry;
+
+ WARN_ON((mode & S_IFMT) && !S_ISREG(mode));
+ mode |= S_IFREG;
+
+ dentry = simplefs_create_dentry(fs, type, name, parent, inode);
+
+ if (IS_ERR(dentry))
+ return dentry;
+
+ (*inode)->i_mode = mode;
+ (*inode)->i_private = data;
+
+ return dentry;
+}
+EXPORT_SYMBOL(simplefs_create_file);
+
+
+/**
+ * simplefs_finish_dentry- complete creation of a new dentry
+ * @dentry: the dentry being created
+ * @inode: the inode associated to the dentry
+ *
+ * This function completes the creation of a dentry.
+ * This includes associating @inode with the dentry, ensuring the link
+ * counts are consistent and informing fsnotify.
+ **/
+struct dentry *simplefs_finish_dentry(struct dentry *dentry, struct inode *inode)
+{
+ d_instantiate(dentry, inode);
+ if (S_ISDIR(inode->i_mode)) {
+ inc_nlink(d_inode(dentry->d_parent));
+ fsnotify_mkdir(d_inode(dentry->d_parent), dentry);
+ } else {
+ fsnotify_create(d_inode(dentry->d_parent), dentry);
+ }
+ inode_unlock(d_inode(dentry->d_parent));
+ return dentry;
+}
+EXPORT_SYMBOL(simplefs_finish_dentry);
+
+/**
+ * simplefs_create_dir - creates a new directory dentry and inode
+ * @fs: a pointer to a struct simple_fs containing the reference counter
+ * and vfs_mount pointer
+ * @type: the fs type
+ * @name: dir name
+ * @mode: dir mode
+ * @parent: parent dentry. If this parameter is NULL,
+ * then the directory will be created in the root of the
+ * filesystem.
+ * @inode: pointer that will contain a newly created inode
+ *
+ * This function returns a new dentry, or NULL on error. On success, a
+ * new inode is created and stored into @inode. Also note that the inode
+ * for the parent directory is locked by simplefs_create_dentry(),
+ * and will be unlocked by simple_finish_dentry().
+ **/
+struct dentry *simplefs_create_dir(struct simple_fs *fs, struct file_system_type *type,
+ const char *name, umode_t mode, struct dentry *parent,
+ struct inode **inode)
+{
+ struct dentry *dentry;
+
+ WARN_ON((mode & S_IFMT) && !S_ISDIR(mode));
+ mode |= S_IFDIR;
+
+ dentry = simplefs_create_dentry(fs, type, name, parent, inode);
+ if (IS_ERR(dentry))
+ return dentry;
+
+ (*inode)->i_mode = mode;
+ (*inode)->i_op = &simple_dir_inode_operations;
+ (*inode)->i_fop = &simple_dir_operations;
+
+ /* directory inodes start off with i_nlink == 2 (for "." entry) */
+ inc_nlink(*inode);
+ return dentry;
+}
+EXPORT_SYMBOL(simplefs_create_dir);
+
+/**
+ * simplefs_create_symlink - creates a new symlink dentry and inode
+ * @fs: a pointer to a struct simple_fs containing the reference counter
+ * and vfs_mount pointer
+ * @type: the fs type
+ * @name: symlink name
+ * @parent: parent dentry. If this parameter is NULL,
+ * then the symbolic link will be created in the root of the
+ * filesystem.
+ * @inode: pointer that will contain a newly created inode
+ *
+ * This function returns a new dentry, or NULL on error. On success, a
+ * new inode is created and stored into @inode. Also note that the inode
+ * for the parent directory is locked by simplefs_create_dentry(),
+ * and will be unlocked by simple_finish_dentry().
+ **/
+struct dentry *simplefs_create_symlink(struct simple_fs *fs, struct file_system_type *type,
+ const char *name, struct dentry *parent,
+ const char *target, struct inode **inode)
+{
+ struct dentry *dentry;
+ char *link = kstrdup(target, GFP_KERNEL);
+
+ if (!link)
+ return ERR_PTR(-ENOMEM);
+
+ dentry = simplefs_create_dentry(fs, type, name, parent, inode);
+ if (IS_ERR(dentry)) {
+ kfree_link(link);
+ return dentry;
+ }
+
+ (*inode)->i_mode = S_IFLNK | S_IRWXUGO;
+ (*inode)->i_link = link;
+ (*inode)->i_op = &simple_symlink_inode_operations;
+ return dentry;
+}
+EXPORT_SYMBOL(simplefs_create_symlink);
+
/**
* simple_read_from_buffer - copy data from the buffer to user space
* @to: the user space buffer to read to
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 5e93de72118b..0569540fbe61 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3375,6 +3375,24 @@ extern void simple_release_fs(struct simple_fs *);
extern struct inode *simple_alloc_anon_inode(struct simple_fs *fs);
+extern struct dentry *simplefs_create_dentry(struct simple_fs *fs,
+ struct file_system_type *type,
+ const char *name, struct dentry *parent,
+ struct inode **inode);
+struct dentry *simplefs_finish_dentry(struct dentry *dentry, struct inode *inode);
+
+extern struct dentry *simplefs_create_file(struct simple_fs *fs,
+ struct file_system_type *type,
+ const char *name, umode_t mode,
+ struct dentry *parent, void *data,
+ struct inode **inode);
+extern struct dentry *simplefs_create_dir(struct simple_fs *fs, struct file_system_type *type,
+ const char *name, umode_t mode, struct dentry *parent,
+ struct inode **inode);
+extern struct dentry *simplefs_create_symlink(struct simple_fs *fs, struct file_system_type *type,
+ const char *name, struct dentry *parent,
+ const char *target, struct inode **inode);
+
extern ssize_t simple_read_from_buffer(void __user *to, size_t count,
loff_t *ppos, const void *from, size_t available);
extern ssize_t simple_write_to_buffer(void *to, size_t available, loff_t *ppos,
--
2.25.2
^ permalink raw reply related
* [PATCH v2 4/7] libfs: add alloc_anon_inode wrapper
From: Emanuele Giuseppe Esposito @ 2020-04-21 13:57 UTC (permalink / raw)
To: linux-fsdevel
Cc: Rafael J. Wysocki, David Airlie, dri-devel, Christoph Hellwig,
Andrew Donnellan, Emanuele Giuseppe Esposito, linux-scsi,
James Morris, Serge E. Hallyn, Daniel Vetter, Arnd Bergmann,
James E.J. Bottomley, Maarten Lankhorst, Maxime Ripard,
Manoj N. Kumar, Alexander Viro, Matthew R. Ochs, Uma Krishnan,
John Johansen, Martin K. Petersen, Greg Kroah-Hartman,
linux-kernel, linux-security-module, Thomas Zimmermann,
Frederic Barrat, Paolo Bonzini, linuxppc-dev, Joel Becker
In-Reply-To: <20200421135119.30007-1-eesposit@redhat.com>
libfs.c has many functions that are useful to implement dentry and inode
operations, but not many at the filesystem level. Start adding file
creation wrappers, the simplest returns an anonymous inode.
There is no functional change intended.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
drivers/gpu/drm/drm_drv.c | 2 +-
drivers/misc/cxl/api.c | 2 +-
drivers/scsi/cxlflash/ocxl_hw.c | 2 +-
fs/libfs.c | 10 +++++++++-
include/linux/fs.h | 2 ++
5 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index e29424d64874..1854f760ad39 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -539,7 +539,7 @@ static struct inode *drm_fs_inode_new(void)
return ERR_PTR(r);
}
- inode = alloc_anon_inode(drm_fs.mount->mnt_sb);
+ inode = simple_alloc_anon_inode(&drm_fs);
if (IS_ERR(inode))
simple_release_fs(&drm_fs);
diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
index 67e4808bce49..57672abb6223 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -72,7 +72,7 @@ static struct file *cxl_getfile(const char *name,
goto err_module;
}
- inode = alloc_anon_inode(cxl_fs.mount->mnt_sb);
+ inode = simple_alloc_anon_inode(&cxl_fs);
if (IS_ERR(inode)) {
file = ERR_CAST(inode);
goto err_fs;
diff --git a/drivers/scsi/cxlflash/ocxl_hw.c b/drivers/scsi/cxlflash/ocxl_hw.c
index 7fa98dd4fa28..0e9f2ae7eebf 100644
--- a/drivers/scsi/cxlflash/ocxl_hw.c
+++ b/drivers/scsi/cxlflash/ocxl_hw.c
@@ -85,7 +85,7 @@ static struct file *ocxlflash_getfile(struct device *dev, const char *name,
goto err2;
}
- inode = alloc_anon_inode(ocxlflash_fs.mount->mnt_sb);
+ inode = simple_alloc_anon_inode(&ocxlflash_fs);
if (IS_ERR(inode)) {
rc = PTR_ERR(inode);
dev_err(dev, "%s: alloc_anon_inode failed rc=%d\n",
diff --git a/fs/libfs.c b/fs/libfs.c
index 3fa0cd27ab06..5c76e4c648dc 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -741,7 +741,15 @@ void simple_release_fs(struct simple_fs *fs)
}
EXPORT_SYMBOL(simple_release_fs);
-
+/**
+ * simple_alloc_anon_inode - wrapper for alloc_anon_inode
+ * @fs: a pointer to a struct simple_fs containing a valid vfs_mount pointer
+ **/
+struct inode *simple_alloc_anon_inode(struct simple_fs *fs)
+{
+ return alloc_anon_inode(fs->mount->mnt_sb);
+}
+EXPORT_SYMBOL(simple_alloc_anon_inode);
/**
* simple_read_from_buffer - copy data from the buffer to user space
diff --git a/include/linux/fs.h b/include/linux/fs.h
index de2577df30ae..5e93de72118b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3373,6 +3373,8 @@ struct simple_fs {
extern int simple_pin_fs(struct simple_fs *, struct file_system_type *);
extern void simple_release_fs(struct simple_fs *);
+extern struct inode *simple_alloc_anon_inode(struct simple_fs *fs);
+
extern ssize_t simple_read_from_buffer(void __user *to, size_t count,
loff_t *ppos, const void *from, size_t available);
extern ssize_t simple_write_to_buffer(void *to, size_t available, loff_t *ppos,
--
2.25.2
^ permalink raw reply related
* [PATCH v2 3/7] libfs: introduce new_inode_current_time
From: Emanuele Giuseppe Esposito @ 2020-04-21 13:57 UTC (permalink / raw)
To: linux-fsdevel
Cc: Rafael J. Wysocki, David Airlie, dri-devel, Christoph Hellwig,
Andrew Donnellan, Emanuele Giuseppe Esposito, linux-scsi,
James Morris, Serge E. Hallyn, Daniel Vetter, Arnd Bergmann,
James E.J. Bottomley, Maarten Lankhorst, Maxime Ripard,
Manoj N. Kumar, Alexander Viro, Matthew R. Ochs, Uma Krishnan,
John Johansen, Martin K. Petersen, Greg Kroah-Hartman,
linux-kernel, linux-security-module, Thomas Zimmermann,
Frederic Barrat, Paolo Bonzini, linuxppc-dev, Joel Becker
In-Reply-To: <20200421135119.30007-1-eesposit@redhat.com>
It is a common special case for new_inode to initialize the
time to the current time and the inode to get_next_ino().
Introduce a core function that does it.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
fs/libfs.c | 20 ++++++++++++++++++++
include/linux/fs.h | 1 +
2 files changed, 21 insertions(+)
diff --git a/fs/libfs.c b/fs/libfs.c
index 54e07ae986ca..3fa0cd27ab06 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -594,6 +594,26 @@ int simple_write_end(struct file *file, struct address_space *mapping,
}
EXPORT_SYMBOL(simple_write_end);
+/**
+ * new_inode_current_time - create new inode by initializing the
+ * time to the current time and the inode to get_next_ino()
+ * @sb: pointer to super block of the file system
+ *
+ * Returns an inode pointer on success, NULL on failure.
+ */
+struct inode *new_inode_current_time(struct super_block *sb)
+{
+ struct inode *inode = new_inode(sb);
+
+ if (inode) {
+ inode->i_ino = get_next_ino();
+ inode->i_atime = inode->i_mtime =
+ inode->i_ctime = current_time(inode);
+ }
+ return inode;
+}
+EXPORT_SYMBOL(new_inode_current_time);
+
/*
* the inodes created here are not hashed. If you use iunique to generate
* unique inode values later for this filesystem, then you must take care
diff --git a/include/linux/fs.h b/include/linux/fs.h
index a3691c132b3a..de2577df30ae 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3088,6 +3088,7 @@ extern void clear_inode(struct inode *);
extern void __destroy_inode(struct inode *);
extern struct inode *new_inode_pseudo(struct super_block *sb);
extern struct inode *new_inode(struct super_block *sb);
+extern struct inode *new_inode_current_time(struct super_block *sb);
extern void free_inode_nonrcu(struct inode *inode);
extern int should_remove_suid(struct dentry *);
extern int file_remove_privs(struct file *);
--
2.25.2
^ permalink raw reply related
* Re: [PATCH 1/3] kexec: Prevent removal of memory in use by a loaded kexec image
From: David Hildenbrand @ 2020-04-21 13:57 UTC (permalink / raw)
To: Baoquan He, Andrew Morton
Cc: piliu, Anshuman Khandual, Catalin Marinas, Bhupesh Sharma,
linuxppc-dev, kexec, Russell King - ARM Linux admin, linux-mm,
James Morse, Eric W. Biederman, Will Deacon, linux-arm-kernel
In-Reply-To: <9a4eb1d7-33bf-8707-9c0c-1ca657c3e502@redhat.com>
On 21.04.20 15:29, David Hildenbrand wrote:
>>> ACPI SRAT is embeded into efi, need read out the rsdp pointer. If we don't
>>> pass the efi, it won't get the SRAT table correctly, if I remember
>>> correctly. Yeah, I remeber kvm guest can get memory hotplugged with
>>> ACPI only, this won't happen on bare metal though. Need check carefully.
>>> I have been using kvm guest with uefi firmwire recently.
>>
>> Yeah, I can imagine that bare metal is different. kvm only uses ACPI.
>>
>> I'm also asking because of virtio-mem. Memory added via virtio-mem is
>> not part of any efi tables or whatsoever. So I assume the kexec kernel
>> will not detect it automatically (good!), instead load the virtio-mem
>> driver and let it add memory back to the system.
>>
>> I should probably play with kexec and virtio-mem once I have some spare
>> cycles ... to find out what's broken and needs to be addressed :)
>
> FWIW, I just gave virtio-mem and kexec/kdump a try.
>
> a) kdump seems to work. Memory added by virtio-mem is getting dumped.
> The kexec kernel only uses memory in the crash region. The virtio-mem
> driver properly bails out due to is_kdump_kernel().
>
> b) "kexec -s -l" seems to work fine. For now, the kernel does not seem
> to get placed on virtio-mem memory (pure luck due to the left-to-right
> search). Memory added by virtio-mem is not getting added to the e820
> map. Once the virtio-mem driver comes back up in the kexec kernel, the
> right memory is readded.
>
> c) "kexec -c -l" does not work properly. All memory added by virtio-mem
> is added to the e820 map, which is wrong. Memory that should not be
> touched will be touched by the kexec kernel. I assume kexec-tools just
> goes ahead and adds anything it can find in /proc/iomem (or
> /sys/firmware/memmap/) to the e820 map of the new kernel.
>
> Due to c), I assume all hotplugged memory (e.g., ACPI DIMMs) is
> similarly added to the e820 map and, therefore, won't be able to be
> onlined MOVABLE easily.
>
>
> At least for virtio-mem, I would either have to
> a) Not support "kexec -c -l". A viable option if we would be planning on
> not supporting it either way in the long term. I could block this
> in-kernel somehow eventually.
>
> b) Teach kexec-tools to leave virtio-mem added memory alone. E.g., by
> indicating it in /proc/iomem in a special way ("System RAM
> (hotplugged)"/"System RAM (virtio-mem)").
I just realized, that *not* creating /sys/firmware/memmap/ entries for
virtio-mem memory seems to be the right thing to do.
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v2] sched/core: fix illegal RCU from offline CPUs
From: Peter Zijlstra @ 2020-04-21 13:56 UTC (permalink / raw)
To: Qian Cai
Cc: juri.lelli, James.Bottomley@hansenpartnership.com,
vincent.guittot, linux-parisc, paulmck, deller, Nicholas Piggin,
linux-kernel, Steven Rostedt, bsegall, linux-mm, Ingo Molnar,
mgorman, tglx, linuxppc-dev, dietmar.eggemann
In-Reply-To: <BBA124FA-7924-4782-AC9D-7B1B98BE817F@lca.pw>
On Fri, Apr 17, 2020 at 09:26:56AM -0400, Qian Cai wrote:
> > Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
>
> Peter, can you take a look at this patch when you have a chance?
Sorry, -ETOOMUCHEMAIL, got it now, thanks!
^ permalink raw reply
* [PATCH v2 2/7] libfs: wrap simple_pin_fs/simple_release_fs arguments in a struct
From: Emanuele Giuseppe Esposito @ 2020-04-21 13:51 UTC (permalink / raw)
To: linux-fsdevel
Cc: Rafael J. Wysocki, David Airlie, dri-devel, Christoph Hellwig,
Andrew Donnellan, Emanuele Giuseppe Esposito, linux-scsi,
James Morris, Serge E. Hallyn, Daniel Vetter, Arnd Bergmann,
James E.J. Bottomley, Maarten Lankhorst, Maxime Ripard,
Manoj N. Kumar, Alexander Viro, Matthew R. Ochs, Uma Krishnan,
John Johansen, Martin K. Petersen, Greg Kroah-Hartman,
linux-kernel, linux-security-module, Thomas Zimmermann,
Frederic Barrat, Paolo Bonzini, linuxppc-dev, Joel Becker
In-Reply-To: <20200421135119.30007-1-eesposit@redhat.com>
Simplify passing the count and mount to simple_pin_fs and
simple_release_fs by wrapping them in the simple_fs struct,
in preparation for adding more high level operations to
fs/libfs.c
There is no functional change intended.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
drivers/gpu/drm/drm_drv.c | 11 ++++----
drivers/misc/cxl/api.c | 13 +++++-----
drivers/scsi/cxlflash/ocxl_hw.c | 14 +++++-----
fs/binfmt_misc.c | 9 +++----
fs/configfs/mount.c | 10 +++-----
fs/debugfs/inode.c | 22 ++++++++--------
fs/libfs.c | 45 +++++++++++++++++++++++++--------
fs/tracefs/inode.c | 18 ++++++-------
include/linux/fs.h | 10 ++++++--
security/apparmor/apparmorfs.c | 25 +++++++++---------
security/inode.c | 11 ++++----
11 files changed, 103 insertions(+), 85 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 7b1a628d1f6e..e29424d64874 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -514,8 +514,7 @@ EXPORT_SYMBOL(drm_dev_unplug);
* iput(), but this way you'd end up with a new vfsmount for each inode.
*/
-static int drm_fs_cnt;
-static struct vfsmount *drm_fs_mnt;
+static struct simple_fs drm_fs;
static int drm_fs_init_fs_context(struct fs_context *fc)
{
@@ -534,15 +533,15 @@ static struct inode *drm_fs_inode_new(void)
struct inode *inode;
int r;
- r = simple_pin_fs(&drm_fs_type, &drm_fs_mnt, &drm_fs_cnt);
+ r = simple_pin_fs(&drm_fs, &drm_fs_type);
if (r < 0) {
DRM_ERROR("Cannot mount pseudo fs: %d\n", r);
return ERR_PTR(r);
}
- inode = alloc_anon_inode(drm_fs_mnt->mnt_sb);
+ inode = alloc_anon_inode(drm_fs.mount->mnt_sb);
if (IS_ERR(inode))
- simple_release_fs(&drm_fs_mnt, &drm_fs_cnt);
+ simple_release_fs(&drm_fs);
return inode;
}
@@ -551,7 +550,7 @@ static void drm_fs_inode_free(struct inode *inode)
{
if (inode) {
iput(inode);
- simple_release_fs(&drm_fs_mnt, &drm_fs_cnt);
+ simple_release_fs(&drm_fs);
}
}
diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
index b493de962153..67e4808bce49 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -31,8 +31,7 @@
#define CXL_PSEUDO_FS_MAGIC 0x1697697f
-static int cxl_fs_cnt;
-static struct vfsmount *cxl_vfs_mount;
+static struct simple_fs cxl_fs;
static int cxl_fs_init_fs_context(struct fs_context *fc)
{
@@ -50,7 +49,7 @@ static struct file_system_type cxl_fs_type = {
void cxl_release_mapping(struct cxl_context *ctx)
{
if (ctx->kernelapi && ctx->mapping)
- simple_release_fs(&cxl_vfs_mount, &cxl_fs_cnt);
+ simple_release_fs(&cxl_fs);
}
static struct file *cxl_getfile(const char *name,
@@ -66,20 +65,20 @@ static struct file *cxl_getfile(const char *name,
if (fops->owner && !try_module_get(fops->owner))
return ERR_PTR(-ENOENT);
- rc = simple_pin_fs(&cxl_fs_type, &cxl_vfs_mount, &cxl_fs_cnt);
+ rc = simple_pin_fs(&cxl_fs, &cxl_fs_type);
if (rc < 0) {
pr_err("Cannot mount cxl pseudo filesystem: %d\n", rc);
file = ERR_PTR(rc);
goto err_module;
}
- inode = alloc_anon_inode(cxl_vfs_mount->mnt_sb);
+ inode = alloc_anon_inode(cxl_fs.mount->mnt_sb);
if (IS_ERR(inode)) {
file = ERR_CAST(inode);
goto err_fs;
}
- file = alloc_file_pseudo(inode, cxl_vfs_mount, name,
+ file = alloc_file_pseudo(inode, cxl_fs.mount, name,
flags & (O_ACCMODE | O_NONBLOCK), fops);
if (IS_ERR(file))
goto err_inode;
@@ -91,7 +90,7 @@ static struct file *cxl_getfile(const char *name,
err_inode:
iput(inode);
err_fs:
- simple_release_fs(&cxl_vfs_mount, &cxl_fs_cnt);
+ simple_release_fs(&cxl_fs);
err_module:
module_put(fops->owner);
return file;
diff --git a/drivers/scsi/cxlflash/ocxl_hw.c b/drivers/scsi/cxlflash/ocxl_hw.c
index 7018cd802569..7fa98dd4fa28 100644
--- a/drivers/scsi/cxlflash/ocxl_hw.c
+++ b/drivers/scsi/cxlflash/ocxl_hw.c
@@ -29,8 +29,7 @@
#define OCXLFLASH_FS_MAGIC 0x1697698f
-static int ocxlflash_fs_cnt;
-static struct vfsmount *ocxlflash_vfs_mount;
+static struct simple_fs ocxlflash_fs;
static int ocxlflash_fs_init_fs_context(struct fs_context *fc)
{
@@ -51,7 +50,7 @@ static struct file_system_type ocxlflash_fs_type = {
static void ocxlflash_release_mapping(struct ocxlflash_context *ctx)
{
if (ctx->mapping)
- simple_release_fs(&ocxlflash_vfs_mount, &ocxlflash_fs_cnt);
+ simple_release_fs(&ocxlflash_fs);
ctx->mapping = NULL;
}
@@ -79,15 +78,14 @@ static struct file *ocxlflash_getfile(struct device *dev, const char *name,
goto err1;
}
- rc = simple_pin_fs(&ocxlflash_fs_type, &ocxlflash_vfs_mount,
- &ocxlflash_fs_cnt);
+ rc = simple_pin_fs(&ocxlflash_fs, &ocxlflash_fs_type);
if (unlikely(rc < 0)) {
dev_err(dev, "%s: Cannot mount ocxlflash pseudofs rc=%d\n",
__func__, rc);
goto err2;
}
- inode = alloc_anon_inode(ocxlflash_vfs_mount->mnt_sb);
+ inode = alloc_anon_inode(ocxlflash_fs.mount->mnt_sb);
if (IS_ERR(inode)) {
rc = PTR_ERR(inode);
dev_err(dev, "%s: alloc_anon_inode failed rc=%d\n",
@@ -95,7 +93,7 @@ static struct file *ocxlflash_getfile(struct device *dev, const char *name,
goto err3;
}
- file = alloc_file_pseudo(inode, ocxlflash_vfs_mount, name,
+ file = alloc_file_pseudo(inode, ocxlflash_fs.mount, name,
flags & (O_ACCMODE | O_NONBLOCK), fops);
if (IS_ERR(file)) {
rc = PTR_ERR(file);
@@ -110,7 +108,7 @@ static struct file *ocxlflash_getfile(struct device *dev, const char *name,
err4:
iput(inode);
err3:
- simple_release_fs(&ocxlflash_vfs_mount, &ocxlflash_fs_cnt);
+ simple_release_fs(&ocxlflash_fs);
err2:
module_put(fops->owner);
err1:
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index cdb45829354d..3cff446f222b 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -64,8 +64,7 @@ typedef struct {
static DEFINE_RWLOCK(entries_lock);
static struct file_system_type bm_fs_type;
-static struct vfsmount *bm_mnt;
-static int entry_count;
+static struct simple_fs bm_fs;
/*
* Max length of the register string. Determined by:
@@ -623,7 +622,7 @@ static void kill_node(Node *e)
drop_nlink(d_inode(dentry));
d_drop(dentry);
dput(dentry);
- simple_release_fs(&bm_mnt, &entry_count);
+ simple_release_fs(&bm_fs);
}
/* /<entry> */
@@ -718,7 +717,7 @@ static ssize_t bm_register_write(struct file *file, const char __user *buffer,
if (!inode)
goto out2;
- err = simple_pin_fs(&bm_fs_type, &bm_mnt, &entry_count);
+ err = simple_pin_fs(&bm_fs, &bm_fs_type);
if (err) {
iput(inode);
inode = NULL;
@@ -732,7 +731,7 @@ static ssize_t bm_register_write(struct file *file, const char __user *buffer,
if (IS_ERR(f)) {
err = PTR_ERR(f);
pr_notice("register: failed to install interpreter file %s\n", e->interpreter);
- simple_release_fs(&bm_mnt, &entry_count);
+ simple_release_fs(&bm_fs);
iput(inode);
inode = NULL;
goto out2;
diff --git a/fs/configfs/mount.c b/fs/configfs/mount.c
index 0c6e8cf61953..9fb2791e5eed 100644
--- a/fs/configfs/mount.c
+++ b/fs/configfs/mount.c
@@ -24,9 +24,8 @@
/* Random magic number */
#define CONFIGFS_MAGIC 0x62656570
-static struct vfsmount *configfs_mount = NULL;
+static struct simple_fs configfs_fs;
struct kmem_cache *configfs_dir_cachep;
-static int configfs_mnt_count = 0;
static void configfs_free_inode(struct inode *inode)
@@ -123,14 +122,13 @@ MODULE_ALIAS_FS("configfs");
struct dentry *configfs_pin_fs(void)
{
- int err = simple_pin_fs(&configfs_fs_type, &configfs_mount,
- &configfs_mnt_count);
- return err ? ERR_PTR(err) : configfs_mount->mnt_root;
+ int err = simple_pin_fs(&configfs_fs, &configfs_fs_type);
+ return err ? ERR_PTR(err) : configfs_fs.mount->mnt_root;
}
void configfs_release_fs(void)
{
- simple_release_fs(&configfs_mount, &configfs_mnt_count);
+ simple_release_fs(&configfs_fs);
}
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index b7f2e971ecbc..5dbb74a23e7c 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -32,8 +32,7 @@
#define DEBUGFS_DEFAULT_MODE 0700
-static struct vfsmount *debugfs_mount;
-static int debugfs_mount_count;
+static struct simple_fs debugfs;
static bool debugfs_registered;
/*
@@ -297,7 +296,7 @@ struct dentry *debugfs_lookup(const char *name, struct dentry *parent)
return NULL;
if (!parent)
- parent = debugfs_mount->mnt_root;
+ parent = debugfs.mount->mnt_root;
dentry = lookup_positive_unlocked(name, parent, strlen(name));
if (IS_ERR(dentry))
@@ -316,8 +315,7 @@ static struct dentry *start_creating(const char *name, struct dentry *parent)
if (IS_ERR(parent))
return parent;
- error = simple_pin_fs(&debug_fs_type, &debugfs_mount,
- &debugfs_mount_count);
+ error = simple_pin_fs(&debugfs, &debug_fs_type);
if (error) {
pr_err("Unable to pin filesystem for file '%s'\n", name);
return ERR_PTR(error);
@@ -329,7 +327,7 @@ static struct dentry *start_creating(const char *name, struct dentry *parent)
* have around.
*/
if (!parent)
- parent = debugfs_mount->mnt_root;
+ parent = debugfs.mount->mnt_root;
inode_lock(d_inode(parent));
if (unlikely(IS_DEADDIR(d_inode(parent))))
@@ -349,7 +347,7 @@ static struct dentry *start_creating(const char *name, struct dentry *parent)
if (IS_ERR(dentry)) {
inode_unlock(d_inode(parent));
- simple_release_fs(&debugfs_mount, &debugfs_mount_count);
+ simple_release_fs(&debugfs);
}
return dentry;
@@ -359,7 +357,7 @@ static struct dentry *failed_creating(struct dentry *dentry)
{
inode_unlock(d_inode(dentry->d_parent));
dput(dentry);
- simple_release_fs(&debugfs_mount, &debugfs_mount_count);
+ simple_release_fs(&debugfs);
return ERR_PTR(-ENOMEM);
}
@@ -676,9 +674,9 @@ static void __debugfs_file_removed(struct dentry *dentry)
static void remove_one(struct dentry *victim)
{
- if (d_is_reg(victim))
+ if (d_is_reg(victim))
__debugfs_file_removed(victim);
- simple_release_fs(&debugfs_mount, &debugfs_mount_count);
+ simple_release_fs(&debugfs);
}
/**
@@ -699,9 +697,9 @@ void debugfs_remove(struct dentry *dentry)
if (IS_ERR_OR_NULL(dentry))
return;
- simple_pin_fs(&debug_fs_type, &debugfs_mount, &debugfs_mount_count);
+ simple_pin_fs(&debugfs, &debug_fs_type);
simple_recursive_removal(dentry, remove_one);
- simple_release_fs(&debugfs_mount, &debugfs_mount_count);
+ simple_release_fs(&debugfs);
}
EXPORT_SYMBOL_GPL(debugfs_remove);
diff --git a/fs/libfs.c b/fs/libfs.c
index 3759fbacf522..54e07ae986ca 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -665,39 +665,64 @@ EXPORT_SYMBOL(simple_fill_super);
static DEFINE_SPINLOCK(pin_fs_lock);
-int simple_pin_fs(struct file_system_type *type, struct vfsmount **mount, int *count)
+/**
+ * simple_pin_fs - generic function to pin (mount if needed,
+ * otherwise add a reference to the mount) a filesystem
+ * @fs: a pointer to a the simple_fs struct containing a struct vfs_mount
+ * pointer (that can be NULL) and a counter.
+ * @type: a pointer to the file system type used by vfs_kern_mount.
+ *
+ * This function sets fs->mount if NULL, by calling vfs_kern_mount
+ * on @type.
+ * It also takes care of incrementing the reference counter.
+ *
+ * This function will return 0 in case of success, and PTR_ERR(-ERROR)
+ * if vfs_kern_mount fails.
+ **/
+int simple_pin_fs(struct simple_fs *fs, struct file_system_type *type)
{
struct vfsmount *mnt = NULL;
spin_lock(&pin_fs_lock);
- if (unlikely(!*mount)) {
+ if (unlikely(!fs->mount)) {
spin_unlock(&pin_fs_lock);
mnt = vfs_kern_mount(type, SB_KERNMOUNT, type->name, NULL);
if (IS_ERR(mnt))
return PTR_ERR(mnt);
spin_lock(&pin_fs_lock);
- if (!*mount)
- *mount = mnt;
+ if (!fs->mount)
+ fs->mount = mnt;
}
- mntget(*mount);
- ++*count;
+ mntget(fs->mount);
+ ++fs->count;
spin_unlock(&pin_fs_lock);
mntput(mnt);
return 0;
}
EXPORT_SYMBOL(simple_pin_fs);
-void simple_release_fs(struct vfsmount **mount, int *count)
+/**
+ * simple_release_fs - decrements the reference counter and unmounts the
+ * file system.
+ * @fs: a pointer to a struct simple_fs containing the reference counter
+ * and vfs_mount pointer
+ *
+ * This function decrements the refcount of the given file system and
+ * if 0 sets the mount pointer to NULL.
+ **/
+void simple_release_fs(struct simple_fs *fs)
{
struct vfsmount *mnt;
spin_lock(&pin_fs_lock);
- mnt = *mount;
- if (!--*count)
- *mount = NULL;
+ mnt = fs->mount;
+ if (!--fs->count)
+ fs->mount = NULL;
spin_unlock(&pin_fs_lock);
mntput(mnt);
}
EXPORT_SYMBOL(simple_release_fs);
+
+
/**
* simple_read_from_buffer - copy data from the buffer to user space
* @to: the user space buffer to read to
diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
index 0ee8c6dfb036..370eb38ff1ad 100644
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -24,8 +24,7 @@
#define TRACEFS_DEFAULT_MODE 0700
-static struct vfsmount *tracefs_mount;
-static int tracefs_mount_count;
+static struct simple_fs tracefs;
static bool tracefs_registered;
static ssize_t default_read_file(struct file *file, char __user *buf,
@@ -316,8 +315,7 @@ static struct dentry *start_creating(const char *name, struct dentry *parent)
pr_debug("tracefs: creating file '%s'\n",name);
- error = simple_pin_fs(&trace_fs_type, &tracefs_mount,
- &tracefs_mount_count);
+ error = simple_pin_fs(&tracefs, &trace_fs_type);
if (error)
return ERR_PTR(error);
@@ -327,7 +325,7 @@ static struct dentry *start_creating(const char *name, struct dentry *parent)
* have around.
*/
if (!parent)
- parent = tracefs_mount->mnt_root;
+ parent = tracefs.mount->mnt_root;
inode_lock(parent->d_inode);
if (unlikely(IS_DEADDIR(parent->d_inode)))
@@ -341,7 +339,7 @@ static struct dentry *start_creating(const char *name, struct dentry *parent)
if (IS_ERR(dentry)) {
inode_unlock(parent->d_inode);
- simple_release_fs(&tracefs_mount, &tracefs_mount_count);
+ simple_release_fs(&tracefs);
}
return dentry;
@@ -351,7 +349,7 @@ static struct dentry *failed_creating(struct dentry *dentry)
{
inode_unlock(dentry->d_parent->d_inode);
dput(dentry);
- simple_release_fs(&tracefs_mount, &tracefs_mount_count);
+ simple_release_fs(&tracefs);
return NULL;
}
@@ -504,7 +502,7 @@ __init struct dentry *tracefs_create_instance_dir(const char *name,
static void remove_one(struct dentry *victim)
{
- simple_release_fs(&tracefs_mount, &tracefs_mount_count);
+ simple_release_fs(&tracefs);
}
/**
@@ -520,9 +518,9 @@ void tracefs_remove(struct dentry *dentry)
if (IS_ERR_OR_NULL(dentry))
return;
- simple_pin_fs(&trace_fs_type, &tracefs_mount, &tracefs_mount_count);
+ simple_pin_fs(&tracefs, &trace_fs_type);
simple_recursive_removal(dentry, remove_one);
- simple_release_fs(&tracefs_mount, &tracefs_mount_count);
+ simple_release_fs(&tracefs);
}
/**
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 4f6f59b4f22a..a3691c132b3a 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3363,8 +3363,14 @@ struct tree_descr { const char *name; const struct file_operations *ops; int mod
struct dentry *d_alloc_name(struct dentry *, const char *);
extern int simple_fill_super(struct super_block *, unsigned long,
const struct tree_descr *);
-extern int simple_pin_fs(struct file_system_type *, struct vfsmount **mount, int *count);
-extern void simple_release_fs(struct vfsmount **mount, int *count);
+
+struct simple_fs {
+ struct vfsmount *mount;
+ int count;
+};
+
+extern int simple_pin_fs(struct simple_fs *, struct file_system_type *);
+extern void simple_release_fs(struct simple_fs *);
extern ssize_t simple_read_from_buffer(void __user *to, size_t count,
loff_t *ppos, const void *from, size_t available);
diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 36f848734902..00f0158fb1e1 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -140,8 +140,7 @@ static int mangle_name(const char *name, char *target)
*/
#define AAFS_NAME "apparmorfs"
-static struct vfsmount *aafs_mnt;
-static int aafs_count;
+static struct simple_fs aafs;
static int aafs_show_path(struct seq_file *seq, struct dentry *dentry)
@@ -273,7 +272,7 @@ static struct dentry *aafs_create(const char *name, umode_t mode,
if (!(mode & S_IFMT))
mode = (mode & S_IALLUGO) | S_IFREG;
- error = simple_pin_fs(&aafs_ops, &aafs_mnt, &aafs_count);
+ error = simple_pin_fs(&aafs, &aafs_ops);
if (error)
return ERR_PTR(error);
@@ -303,7 +302,7 @@ static struct dentry *aafs_create(const char *name, umode_t mode,
fail_lock:
inode_unlock(dir);
- simple_release_fs(&aafs_mnt, &aafs_count);
+ simple_release_fs(&aafs);
return ERR_PTR(error);
}
@@ -395,7 +394,7 @@ static void aafs_remove(struct dentry *dentry)
dput(dentry);
}
inode_unlock(dir);
- simple_release_fs(&aafs_mnt, &aafs_count);
+ simple_release_fs(&aafs);
}
@@ -1824,7 +1823,7 @@ static int ns_mkdir_op(struct inode *dir, struct dentry *dentry, umode_t mode)
* for pin_fs
*/
inode_unlock(dir);
- error = simple_pin_fs(&aafs_ops, &aafs_mnt, &aafs_count);
+ error = simple_pin_fs(&aafs, &aafs_ops);
mutex_lock_nested(&parent->lock, parent->level);
inode_lock_nested(dir, I_MUTEX_PARENT);
if (error)
@@ -1845,7 +1844,7 @@ static int ns_mkdir_op(struct inode *dir, struct dentry *dentry, umode_t mode)
aa_put_ns(ns); /* list ref remains */
out_pin:
if (error)
- simple_release_fs(&aafs_mnt, &aafs_count);
+ simple_release_fs(&aafs);
out:
mutex_unlock(&parent->lock);
aa_put_ns(parent);
@@ -2580,7 +2579,7 @@ static const char *policy_get_link(struct dentry *dentry,
return ERR_PTR(-ECHILD);
ns = aa_get_current_ns();
- path.mnt = mntget(aafs_mnt);
+ path.mnt = mntget(aafs.mount);
path.dentry = dget(ns_dir(ns));
error = nd_jump_link(&path);
aa_put_ns(ns);
@@ -2631,10 +2630,10 @@ static int __init aa_create_aafs(void)
}
/* setup apparmorfs used to virtualize policy/ */
- aafs_mnt = kern_mount(&aafs_ops);
- if (IS_ERR(aafs_mnt))
+ aafs.mount = kern_mount(&aafs_ops);
+ if (IS_ERR(aafs.mount))
panic("can't set apparmorfs up\n");
- aafs_mnt->mnt_sb->s_flags &= ~SB_NOUSER;
+ aafs.mount->mnt_sb->s_flags &= ~SB_NOUSER;
/* Populate fs tree. */
error = entry_create_dir(&aa_sfs_entry, NULL);
@@ -2667,8 +2666,8 @@ static int __init aa_create_aafs(void)
/* policy tree referenced by magic policy symlink */
mutex_lock_nested(&root_ns->lock, root_ns->level);
- error = __aafs_ns_mkdir(root_ns, aafs_mnt->mnt_root, ".policy",
- aafs_mnt->mnt_root);
+ error = __aafs_ns_mkdir(root_ns, aafs.mount->mnt_root, ".policy",
+ aafs.mount->mnt_root);
mutex_unlock(&root_ns->lock);
if (error)
goto error;
diff --git a/security/inode.c b/security/inode.c
index 6c326939750d..8a1bee35470a 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -22,8 +22,7 @@
#include <linux/lsm_hooks.h>
#include <linux/magic.h>
-static struct vfsmount *mount;
-static int mount_count;
+static struct simple_fs securityfs;
static void securityfs_free_inode(struct inode *inode)
{
@@ -118,12 +117,12 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
pr_debug("securityfs: creating file '%s'\n",name);
- error = simple_pin_fs(&fs_type, &mount, &mount_count);
+ error = simple_pin_fs(&securityfs, &fs_type);
if (error)
return ERR_PTR(error);
if (!parent)
- parent = mount->mnt_root;
+ parent = securityfs.mount->mnt_root;
dir = d_inode(parent);
@@ -168,7 +167,7 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
dentry = ERR_PTR(error);
out:
inode_unlock(dir);
- simple_release_fs(&mount, &mount_count);
+ simple_release_fs(&securityfs);
return dentry;
}
@@ -309,7 +308,7 @@ void securityfs_remove(struct dentry *dentry)
dput(dentry);
}
inode_unlock(dir);
- simple_release_fs(&mount, &mount_count);
+ simple_release_fs(&securityfs);
}
EXPORT_SYMBOL_GPL(securityfs_remove);
--
2.25.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox