From: Frank Zago Date: Fri, 5 May 2017 21:10:47 +0000 (-0400) Subject: LU-9468 llite: fix for stat under kthread and X86_X32 X-Git-Tag: 2.10.0-RC1~29 X-Git-Url: https://git.whamcloud.com/?a=commitdiff_plain;h=86d26552f91e4b8cfe79a55cae8746aa1c18001e;p=fs%2Flustre-release.git LU-9468 llite: fix for stat under kthread and X86_X32 Under the following conditions, ll_getattr will flatten the inode number when it shouldn't: - the X86_X32 architecture is defined CONFIG_X86_X32, and not even used, - ll_getattr is called from a kernel thread (though vfs_getattr for instance.) This has the result that inode numbers are different whether the same file is stat'ed from a kernel thread, or from a syscall. For instance, 4198401 vs. 144115205272502273. ll_getattr calls ll_need_32bit_api to determine whether the task is 32 bits. When the combination is kthread+X86_X32, that function returns that the task is 32 bits, which is incorrect, as the kernel is 64 bits. The solution is to check whether the call is from a kernel thread (which is 64 bits) and act consequently. Added test_410 to test the condition. A failed run will have a message like this: lustre_kinode_54354: CONFIG_X86_X32 is set lustre_kinode_54354: inode is 144115205272502273 lustre_kinode_54354: inode is 4198401 lustre_kinode_54354: inode numbers are different: 144115205272502273 4198401 while a successfull one will be: lustre_kinode_10519: CONFIG_X86_X32 is set lustre_kinode_10519: inode is 144115205272502275 lustre_kinode_10519: inode is 144115205272502275 lustre_kinode_10519: inode numbers are identical: 144115205272502275 Signed-off-by: Frank Zago Change-Id: Ib8f07de47eaa29046a61c488368d466f9096a994 Reviewed-on: https://review.whamcloud.com/26992 Reviewed-by: Andreas Dilger Reviewed-by: Dmitry Eremin Tested-by: Jenkins Tested-by: Maloo --- diff --git a/lustre.spec.in b/lustre.spec.in index 6ed1996..59a3352 100644 --- a/lustre.spec.in +++ b/lustre.spec.in @@ -331,6 +331,8 @@ mv $basemodpath/fs/osd_zfs.ko $basemodpath-osd-zfs/fs/osd_zfs.ko %if %{with lustre_tests} mkdir -p $basemodpath-tests/fs mv $basemodpath/fs/llog_test.ko $basemodpath-tests/fs/llog_test.ko +mkdir -p $RPM_BUILD_ROOT%{_libdir}/lustre/tests/kernel/ +mv $basemodpath/fs/kinode.ko $RPM_BUILD_ROOT%{_libdir}/lustre/tests/kernel/ %endif :> lustre.files diff --git a/lustre/Makefile.in b/lustre/Makefile.in index 2794a62..319a405 100644 --- a/lustre/Makefile.in +++ b/lustre/Makefile.in @@ -3,6 +3,7 @@ subdir-m += obdclass subdir-m += ptlrpc subdir-m += obdecho subdir-m += mgc +subdir-m += tests/kernel @SERVER_TRUE@subdir-m += ost mgs mdt mdd ofd quota osp lod lfsck @CLIENT_TRUE@subdir-m += lov osc mdc lmv llite fld diff --git a/lustre/autoMakefile.am b/lustre/autoMakefile.am index 3b8a55e..e94ce3d 100644 --- a/lustre/autoMakefile.am +++ b/lustre/autoMakefile.am @@ -36,7 +36,7 @@ AUTOMAKE_OPTIONS = foreign # also update lustre/autoconf/lustre-core.m4 AC_CONFIG_FILES ALWAYS_SUBDIRS = include obdclass ldlm ptlrpc obdecho \ - mgc fid fld doc utils tests scripts autoconf contrib conf + mgc fid fld doc utils tests tests/kernel scripts autoconf contrib conf SERVER_SUBDIRS = ost mgs mdt mdd ofd osd-zfs osd-ldiskfs \ quota osp lod target lfsck diff --git a/lustre/autoconf/lustre-core.m4 b/lustre/autoconf/lustre-core.m4 index 77d6a98..c6f2377 100644 --- a/lustre/autoconf/lustre-core.m4 +++ b/lustre/autoconf/lustre-core.m4 @@ -3068,6 +3068,8 @@ lustre/scripts/Makefile lustre/scripts/systemd/Makefile lustre/tests/Makefile lustre/tests/mpi/Makefile +lustre/tests/kernel/Makefile +lustre/tests/kernel/autoMakefile lustre/utils/Makefile lustre/utils/gss/Makefile lustre/osp/Makefile diff --git a/lustre/llite/llite_internal.h b/lustre/llite/llite_internal.h index eb9298d..03ab7e6 100644 --- a/lustre/llite/llite_internal.h +++ b/lustre/llite/llite_internal.h @@ -661,7 +661,19 @@ static inline int ll_need_32bit_api(struct ll_sb_info *sbi) #if BITS_PER_LONG == 32 return 1; #elif defined(CONFIG_COMPAT) - return unlikely(in_compat_syscall() || (sbi->ll_flags & LL_SBI_32BIT_API)); + if (unlikely(sbi->ll_flags & LL_SBI_32BIT_API)) + return true; + +# ifdef CONFIG_X86_X32 + /* in_compat_syscall() returns true when called from a kthread + * and CONFIG_X86_X32 is enabled, which is wrong. So check + * whether the caller comes from a syscall (ie. not a kthread) + * before calling in_compat_syscall(). */ + if (current->flags & PF_KTHREAD) + return false; +# endif + + return unlikely(in_compat_syscall()); #else return unlikely(sbi->ll_flags & LL_SBI_32BIT_API); #endif diff --git a/lustre/tests/kernel/Makefile.in b/lustre/tests/kernel/Makefile.in new file mode 100644 index 0000000..9cba127 --- /dev/null +++ b/lustre/tests/kernel/Makefile.in @@ -0,0 +1,5 @@ +MODULES := kinode + +EXTRA_DIST = kinode.c + +@INCLUDE_RULES@ diff --git a/lustre/tests/kernel/autoMakefile.am b/lustre/tests/kernel/autoMakefile.am new file mode 100644 index 0000000..9309582 --- /dev/null +++ b/lustre/tests/kernel/autoMakefile.am @@ -0,0 +1,34 @@ +# +# GPL HEADER START +# +# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 2 only, +# as published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License version 2 for more details (a copy is included +# in the LICENSE file that accompanied this code). +# +# You should have received a copy of the GNU General Public License +# version 2 along with this program; If not, see +# http://www.gnu.org/licenses/gpl-2.0.html +# +# GPL HEADER END +# + +# +# This file is part of Lustre, http://www.lustre.org/ +# Lustre is a trademark of Sun Microsystems, Inc. +# + +if MODULES +if TESTS +modulefs_DATA = kinode$(KMODEXT) +endif +endif + +MOSTLYCLEANFILES := @MOSTLYCLEANFILES@ diff --git a/lustre/tests/kernel/kinode.c b/lustre/tests/kernel/kinode.c new file mode 100644 index 0000000..0283c8e --- /dev/null +++ b/lustre/tests/kernel/kinode.c @@ -0,0 +1,169 @@ +/* + * GPL HEADER START + * + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 only, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License version 2 for more details (a copy is included + * in the LICENSE file that accompanied this code). + * + * You should have received a copy of the GNU General Public License + * version 2 along with this program; If not, see + * http://www.gnu.org/licenses/gpl-2.0.html + * + * GPL HEADER END + */ + +/* + * Copyright 2017 Cray Inc. All rights reserved. + * Author: Frank Zago. + */ + +/* Check that the inode number is the same whether the call to + * vfs_getattr is coming from a system call or from a kthread. When + * CONFIG_X86_X32 was set, the result used to be different for + * Lustre. In addition, a user can also check that the same inode + * number is also seen from the kernel and userspace. */ + +#include +#include +#include +#include +#include + +/* Random ID passed by userspace, and printed in messages, used to + * separate different runs of that module. */ +static int run_id; +module_param(run_id, int, 0644); +MODULE_PARM_DESC(run_id, "run ID"); + +/* Name of the file to stat. */ +static char fname[4096]; +module_param_string(fname, fname, sizeof(fname), 0644); +MODULE_PARM_DESC(fname, "name of file to stat"); + +struct completion thr_start; + +#define PREFIX "lustre_kinode_%u:" + +static int stat_file(struct kstat *stbuf) +{ + struct file *fd; + int rc; + + fd = filp_open(fname, O_RDONLY, 0); + if (IS_ERR(fd)) { + pr_err(PREFIX " can't open file %s\n", run_id, fname); + return -EIO; + } + +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 9, 0) + rc = vfs_getattr(&fd->f_path, stbuf); +#else + rc = vfs_getattr(fd->f_path.mnt, fd->f_path.dentry, stbuf); +#endif + if (rc != 0) { + pr_err(PREFIX " vfs_getattr failed: %d\n", run_id, rc); + goto out; + } + + pr_err(PREFIX " inode is %llu\n", run_id, stbuf->ino); + rc = 0; + +out: + filp_close(fd, NULL); + + return rc; +} + +static int stat_thread(void *data) +{ + struct kstat *stbuf = data; + int rc; + + /* Signal caller that thread has started. */ + complete(&thr_start); + + rc = stat_file(stbuf); + + /* Wait for call to kthread_stop. */ + set_current_state(TASK_INTERRUPTIBLE); + while (!kthread_should_stop()) { + schedule(); + set_current_state(TASK_INTERRUPTIBLE); + } + set_current_state(TASK_RUNNING); + + return rc; +} + +static int __init kinode_init(void) +{ + struct task_struct *thr; + struct kstat stbuf1; + struct kstat stbuf2; + int rc; + +#ifdef CONFIG_X86_X32 + pr_err(PREFIX " CONFIG_X86_X32 is set\n", run_id); +#else + pr_err(PREFIX " CONFIG_X86_X32 is not set\n", run_id); +#endif + + if (strlen(fname) < 1) { + pr_err(PREFIX " invalid file name '%s'\n", run_id, fname); + goto out; + } + + rc = stat_file(&stbuf1); + if (rc) { + pr_err(PREFIX " direct stat failed: %d\n", run_id, rc); + goto out; + } + + /* Run the same from a kthread. */ + init_completion(&thr_start); + thr = kthread_run(stat_thread, &stbuf2, "kinode_%u", run_id); + if (IS_ERR(thr)) { + pr_err(PREFIX " Cannot create kthread\n", run_id); + goto out; + } + + /* Wait for the thread to start, then wait for it to + * terminate. */ + wait_for_completion(&thr_start); + rc = kthread_stop(thr); + if (rc) { + pr_err(PREFIX " indirect stat failed: %d\n", run_id, rc); + goto out; + } + + if (stbuf1.ino != stbuf2.ino) + pr_err(PREFIX " inode numbers are different: %llu %llu\n", + run_id, stbuf1.ino, stbuf2.ino); + else + pr_err(PREFIX " inode numbers are identical: %llu\n", + run_id, stbuf1.ino); + +out: + /* Don't load. */ + return -EINVAL; +} + +static void __exit kinode_exit(void) +{ +} + +MODULE_AUTHOR("OpenSFS, Inc. "); +MODULE_DESCRIPTION("Lustre inode stat test module"); +MODULE_VERSION(LUSTRE_VERSION_STRING); +MODULE_LICENSE("GPL"); + +module_init(kinode_init); +module_exit(kinode_exit); diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh index c120a2e..49114e9 100755 --- a/lustre/tests/sanity.sh +++ b/lustre/tests/sanity.sh @@ -16419,6 +16419,27 @@ test_409() } run_test 409 "Large amount of cross-MDTs hard links on the same file" +test_410() +{ + # Create a file, and stat it from the kernel + local testfile=$DIR/$tfile + touch $testfile + + local run_id=$RANDOM + local my_ino=$(stat --format "%i" $testfile) + + # Try to insert the module. This will always fail as the + # module is designed to not be inserted. + insmod $LUSTRE/tests/kernel/kinode.ko run_id=$run_id fname=$testfile \ + &> /dev/null + + # Anything but success is a test failure + dmesg | grep -q \ + "lustre_kinode_$run_id: inode numbers are identical: $my_ino" || + error "no inode match" +} +run_test 410 "Test inode number returned from kernel thread" + prep_801() { [[ $(lustre_version_code mds1) -lt $(version_code 2.9.55) ]] || [[ $(lustre_version_code ost1) -lt $(version_code 2.9.55) ]] &&