2022-12-22 15:52:20

by Wen Yang

[permalink] [raw]
Subject: [PATCH] eventfd: use a generic helper instead of an open coded wait_event

From: Wen Yang <[email protected]>

Use wait_event_interruptible_locked_irq() in the eventfd_{write,read} to
avoid the longer, open coded equivalent.

Signed-off-by: Wen Yang <[email protected]>
Cc: Alexander Viro <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
fs/eventfd.c | 43 +++++++------------------------------------
1 file changed, 7 insertions(+), 36 deletions(-)

diff --git a/fs/eventfd.c b/fs/eventfd.c
index 249ca6c0b784..5ff944a17d6d 100644
--- a/fs/eventfd.c
+++ b/fs/eventfd.c
@@ -228,7 +228,6 @@ static ssize_t eventfd_read(struct kiocb *iocb, struct iov_iter *to)
struct file *file = iocb->ki_filp;
struct eventfd_ctx *ctx = file->private_data;
__u64 ucnt = 0;
- DECLARE_WAITQUEUE(wait, current);

if (iov_iter_count(to) < sizeof(ucnt))
return -EINVAL;
@@ -239,23 +238,9 @@ static ssize_t eventfd_read(struct kiocb *iocb, struct iov_iter *to)
spin_unlock_irq(&ctx->wqh.lock);
return -EAGAIN;
}
- __add_wait_queue(&ctx->wqh, &wait);
- for (;;) {
- set_current_state(TASK_INTERRUPTIBLE);
- if (ctx->count)
- break;
- if (signal_pending(current)) {
- __remove_wait_queue(&ctx->wqh, &wait);
- __set_current_state(TASK_RUNNING);
- spin_unlock_irq(&ctx->wqh.lock);
- return -ERESTARTSYS;
- }
- spin_unlock_irq(&ctx->wqh.lock);
- schedule();
- spin_lock_irq(&ctx->wqh.lock);
- }
- __remove_wait_queue(&ctx->wqh, &wait);
- __set_current_state(TASK_RUNNING);
+
+ if (wait_event_interruptible_locked_irq(ctx->wqh, ctx->count))
+ return -ERESTARTSYS;
}
eventfd_ctx_do_read(ctx, &ucnt);
current->in_eventfd = 1;
@@ -275,7 +260,6 @@ static ssize_t eventfd_write(struct file *file, const char __user *buf, size_t c
struct eventfd_ctx *ctx = file->private_data;
ssize_t res;
__u64 ucnt;
- DECLARE_WAITQUEUE(wait, current);

if (count < sizeof(ucnt))
return -EINVAL;
@@ -288,23 +272,10 @@ static ssize_t eventfd_write(struct file *file, const char __user *buf, size_t c
if (ULLONG_MAX - ctx->count > ucnt)
res = sizeof(ucnt);
else if (!(file->f_flags & O_NONBLOCK)) {
- __add_wait_queue(&ctx->wqh, &wait);
- for (res = 0;;) {
- set_current_state(TASK_INTERRUPTIBLE);
- if (ULLONG_MAX - ctx->count > ucnt) {
- res = sizeof(ucnt);
- break;
- }
- if (signal_pending(current)) {
- res = -ERESTARTSYS;
- break;
- }
- spin_unlock_irq(&ctx->wqh.lock);
- schedule();
- spin_lock_irq(&ctx->wqh.lock);
- }
- __remove_wait_queue(&ctx->wqh, &wait);
- __set_current_state(TASK_RUNNING);
+ res = wait_event_interruptible_locked_irq(ctx->wqh,
+ ULLONG_MAX - ctx->count > ucnt);
+ if (!res)
+ res = sizeof(ucnt);
}
if (likely(res > 0)) {
ctx->count += ucnt;
--
2.25.1


2022-12-24 08:41:13

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] eventfd: use a generic helper instead of an open coded wait_event

Hi,

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/wenyang-linux-foxmail-com/eventfd-use-a-generic-helper-instead-of-an-open-coded-wait_event/20221222-234947
patch link: https://lore.kernel.org/r/tencent_B38979DE0FF3B9B3EA887A37487B123BBD05%40qq.com
patch subject: [PATCH] eventfd: use a generic helper instead of an open coded wait_event
config: i386-randconfig-m021
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>

smatch warnings:
fs/eventfd.c:254 eventfd_read() warn: inconsistent returns '&ctx->wqh.lock'.

vim +254 fs/eventfd.c

12aceb89b0bce1 Jens Axboe 2020-05-01 226 static ssize_t eventfd_read(struct kiocb *iocb, struct iov_iter *to)
e1ad7468c77ddb Davide Libenzi 2007-05-10 227 {
12aceb89b0bce1 Jens Axboe 2020-05-01 228 struct file *file = iocb->ki_filp;
b6364572d641c8 Eric Biggers 2018-01-06 229 struct eventfd_ctx *ctx = file->private_data;
b6364572d641c8 Eric Biggers 2018-01-06 230 __u64 ucnt = 0;
e1ad7468c77ddb Davide Libenzi 2007-05-10 231
12aceb89b0bce1 Jens Axboe 2020-05-01 232 if (iov_iter_count(to) < sizeof(ucnt))
b6364572d641c8 Eric Biggers 2018-01-06 233 return -EINVAL;
d48eb233159522 Davide Libenzi 2007-05-18 234 spin_lock_irq(&ctx->wqh.lock);
12aceb89b0bce1 Jens Axboe 2020-05-01 235 if (!ctx->count) {
12aceb89b0bce1 Jens Axboe 2020-05-01 236 if ((file->f_flags & O_NONBLOCK) ||
12aceb89b0bce1 Jens Axboe 2020-05-01 237 (iocb->ki_flags & IOCB_NOWAIT)) {
12aceb89b0bce1 Jens Axboe 2020-05-01 238 spin_unlock_irq(&ctx->wqh.lock);
12aceb89b0bce1 Jens Axboe 2020-05-01 239 return -EAGAIN;
12aceb89b0bce1 Jens Axboe 2020-05-01 240 }
c908f8e6a3a1eb Wen Yang 2022-12-22 241
c908f8e6a3a1eb Wen Yang 2022-12-22 242 if (wait_event_interruptible_locked_irq(ctx->wqh, ctx->count))
12aceb89b0bce1 Jens Axboe 2020-05-01 243 return -ERESTARTSYS;

spin_unlock_irq(&ctx->wqh.lock);

e1ad7468c77ddb Davide Libenzi 2007-05-10 244 }
b6364572d641c8 Eric Biggers 2018-01-06 245 eventfd_ctx_do_read(ctx, &ucnt);
9f0deaa12d832f Dylan Yudaken 2022-08-16 246 current->in_eventfd = 1;
e1ad7468c77ddb Davide Libenzi 2007-05-10 247 if (waitqueue_active(&ctx->wqh))
a9a08845e9acbd Linus Torvalds 2018-02-11 248 wake_up_locked_poll(&ctx->wqh, EPOLLOUT);
9f0deaa12d832f Dylan Yudaken 2022-08-16 249 current->in_eventfd = 0;
d48eb233159522 Davide Libenzi 2007-05-18 250 spin_unlock_irq(&ctx->wqh.lock);
12aceb89b0bce1 Jens Axboe 2020-05-01 251 if (unlikely(copy_to_iter(&ucnt, sizeof(ucnt), to) != sizeof(ucnt)))
b6364572d641c8 Eric Biggers 2018-01-06 252 return -EFAULT;
cb289d6244a37c Davide Libenzi 2010-01-13 253
12aceb89b0bce1 Jens Axboe 2020-05-01 @254 return sizeof(ucnt);
cb289d6244a37c Davide Libenzi 2010-01-13 255 }

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-12-29 10:20:44

by Yujie Liu

[permalink] [raw]
Subject: Re: [PATCH] eventfd: use a generic helper instead of an open coded wait_event

Greeting,

FYI, we noticed BUG:sleeping_function_called_from_invalid_context_at_include/linux/freezer.h due to commit (built with gcc-11):

commit: c908f8e6a3a1eb1f39e27942402a28e9c4457779 ("[PATCH] eventfd: use a generic helper instead of an open coded wait_event")
url: https://github.com/intel-lab-lkp/linux/commits/wenyang-linux-foxmail-com/eventfd-use-a-generic-helper-instead-of-an-open-coded-wait_event/20221222-234947
base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git 9d2f6060fe4c3b49d0cdc1dce1c99296f33379c8
patch link: https://lore.kernel.org/all/[email protected]/
patch subject: [PATCH] eventfd: use a generic helper instead of an open coded wait_event

in testcase: trinity
version: trinity-x86_64-e63e4843-1_20220913
with following parameters:

runtime: 300s
group: group-00

test-description: Trinity is a linux system call fuzz tester.
test-url: http://codemonkey.org.uk/projects/trinity/

on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G

caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):


[ 92.570793][ T3869] BUG: sleeping function called from invalid context at include/linux/freezer.h:53
[ 92.578061][ T3869] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 3869, name: trinity-c4
[ 92.585234][ T3869] preempt_count: 1, expected: 0
[ 92.591738][ T3869] CPU: 1 PID: 3869 Comm: trinity-c4 Not tainted 6.1.0-14365-gc908f8e6a3a1 #1
[ 92.600749][ T3869] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-debian-1.16.0-5 04/01/2014
[ 92.607928][ T3869] Call Trace:
[ 92.614023][ T3869] <TASK>
[ 92.619977][ T3869] dump_stack_lvl (lib/dump_stack.c:107 (discriminator 1))
[ 92.626017][ T3869] __might_resched.cold (kernel/sched/core.c:9986)
[ 92.632260][ T3869] get_signal (include/linux/freezer.h:53 kernel/signal.c:2648)
[ 92.641175][ T3869] ? __hrtimer_start_range_ns (kernel/time/hrtimer.c:1258)
[ 92.647178][ T3869] ? ptrace_signal (kernel/signal.c:2628)
[ 92.652935][ T3869] arch_do_signal_or_restart (arch/x86/kernel/signal.c:306)
[ 92.658747][ T3869] ? get_sigframe_size (arch/x86/kernel/signal.c:303)
[ 92.664354][ T3869] ? perf_syscall_exit (arch/x86/include/asm/bitops.h:228 arch/x86/include/asm/bitops.h:240 include/asm-generic/bitops/instrumented-non-atomic.h:142 kernel/trace/trace_syscalls.c:684)
[ 92.669853][ T3869] ? ftrace_syscall_exit (kernel/trace/trace_syscalls.c:672)
[ 92.675305][ T3869] ? do_readv (fs/read_write.c:953)
[ 92.680533][ T3869] ? vfs_readv (fs/read_write.c:943)
[ 92.685810][ T3869] exit_to_user_mode_loop (kernel/entry/common.c:170)
[ 92.693298][ T3869] exit_to_user_mode_prepare (kernel/entry/common.c:203)
[ 92.698407][ T3869] syscall_exit_to_user_mode (arch/x86/include/asm/jump_label.h:27 include/linux/context_tracking_state.h:106 include/linux/context_tracking.h:41 kernel/entry/common.c:134 kernel/entry/common.c:298)
[ 92.703486][ T3869] do_syscall_64 (arch/x86/entry/common.c:87)
[ 92.708227][ T3869] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120)
[ 92.713040][ T3869] RIP: 0033:0x7f1103c539b9
[ 92.717608][ T3869] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d a7 54 0c 00 f7 d8 64 89 01 48
All code
========
0: 00 c3 add %al,%bl
2: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1)
9: 00 00 00
c: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
11: 48 89 f8 mov %rdi,%rax
14: 48 89 f7 mov %rsi,%rdi
17: 48 89 d6 mov %rdx,%rsi
1a: 48 89 ca mov %rcx,%rdx
1d: 4d 89 c2 mov %r8,%r10
20: 4d 89 c8 mov %r9,%r8
23: 4c 8b 4c 24 08 mov 0x8(%rsp),%r9
28: 0f 05 syscall
2a:* 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax <-- trapping instruction
30: 73 01 jae 0x33
32: c3 retq
33: 48 8b 0d a7 54 0c 00 mov 0xc54a7(%rip),%rcx # 0xc54e1
3a: f7 d8 neg %eax
3c: 64 89 01 mov %eax,%fs:(%rcx)
3f: 48 rex.W

Code starting with the faulting instruction
===========================================
0: 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax
6: 73 01 jae 0x9
8: c3 retq
9: 48 8b 0d a7 54 0c 00 mov 0xc54a7(%rip),%rcx # 0xc54b7
10: f7 d8 neg %eax
12: 64 89 01 mov %eax,%fs:(%rcx)
15: 48 rex.W
[ 92.730262][ T3869] RSP: 002b:00007fff6d1a3e68 EFLAGS: 00000246 ORIG_RAX: 0000000000000013
[ 92.739376][ T3869] RAX: fffffffffffffe00 RBX: 0000000000000002 RCX: 00007f1103c539b9
[ 92.744324][ T3869] RDX: 0000000000000001 RSI: 000055d96aa8b2d0 RDI: 000000000000000f
[ 92.749176][ T3869] RBP: 00007f11025d5000 R08: 00000000000000c6 R09: 000b352037dbdb35
[ 92.753934][ T3869] R10: 000000000000fffa R11: 0000000000000246 R12: 0000000000000013
[ 92.758599][ T3869] R13: 00007f1103d21580 R14: 00007f11025d5058 R15: 00007f11025d5000
[ 92.763188][ T3869] </TASK>
[ 92.767271][ T3869] BUG: scheduling while atomic: trinity-c4/3869/0x00000002
[ 92.775662][ T3869] Modules linked in: can_bcm can_raw can crypto_user ib_core nfnetlink scsi_transport_iscsi atm sctp ip6_udp_tunnel udp_tunnel libcrc32c sr_mod cdrom sg bochs intel_rapl_msr ppdev drm_vram_helper drm_ttm_helper ttm intel_rapl_common ata_generic crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel sha512_ssse3 rapl ata_piix drm_kms_helper parport_pc syscopyarea joydev libata sysfillrect parport ipmi_devintf ipmi_msghandler i2c_piix4 sysimgblt serio_raw fuse drm ip_tables
[ 92.801722][ T3869] CPU: 1 PID: 3869 Comm: trinity-c4 Tainted: G W 6.1.0-14365-gc908f8e6a3a1 #1
[ 92.810820][ T3869] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-debian-1.16.0-5 04/01/2014
[ 92.815481][ T3869] Call Trace:
[ 92.819245][ T3869] <TASK>
[ 92.822845][ T3869] dump_stack_lvl (lib/dump_stack.c:107 (discriminator 1))
[ 92.830325][ T3869] __schedule_bug.cold (kernel/sched/core.c:5788)
[ 92.833982][ T3869] schedule_debug (arch/x86/include/asm/preempt.h:35 kernel/sched/core.c:5815)
[ 92.838194][ T3869] __schedule (arch/x86/include/asm/jump_label.h:27 include/linux/jump_label.h:207 kernel/sched/features.h:40 kernel/sched/core.c:6451)
[ 92.841586][ T3869] ? handle_signal (arch/x86/kernel/signal.c:279)
[ 92.844904][ T3869] ? io_schedule_timeout (kernel/sched/core.c:6437)
[ 92.848218][ T3869] ? get_sigframe_size (arch/x86/kernel/signal.c:303)
[ 92.851537][ T3869] schedule (include/linux/instrumented.h:72 (discriminator 1) include/asm-generic/bitops/instrumented-non-atomic.h:141 (discriminator 1) include/linux/thread_info.h:118 (discriminator 1) include/linux/sched.h:2223 (discriminator 1) kernel/sched/core.c:6633 (discriminator 1))
[ 92.858814][ T3869] exit_to_user_mode_loop (kernel/entry/common.c:161)
[ 92.862865][ T3869] exit_to_user_mode_prepare (kernel/entry/common.c:203)
[ 92.869925][ T3869] syscall_exit_to_user_mode (arch/x86/include/asm/jump_label.h:27 include/linux/context_tracking_state.h:106 include/linux/context_tracking.h:41 kernel/entry/common.c:134 kernel/entry/common.c:298)
[ 92.873139][ T3869] do_syscall_64 (arch/x86/entry/common.c:87)
[ 92.878336][ T3869] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120)
[ 92.885825][ T3869] RIP: 0033:0x55d968929830
[ 92.888835][ T3869] Code: f5 ff ff be 01 00 00 00 48 8d 3d 8b 13 a3 00 80 80 a0 63 00 00 01 e8 9f 8a ff ff 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 <48> 83 ec 08 83 ff 0e 74 07 31 ff e8 90 88 ff ff bf 01 00 00 00 e8
All code
========
0: f5 cmc
1: ff (bad)
2: ff (bad)
3: be 01 00 00 00 mov $0x1,%esi
8: 48 8d 3d 8b 13 a3 00 lea 0xa3138b(%rip),%rdi # 0xa3139a
f: 80 80 a0 63 00 00 01 addb $0x1,0x63a0(%rax)
16: e8 9f 8a ff ff callq 0xffffffffffff8aba
1b: 66 66 2e 0f 1f 84 00 data16 nopw %cs:0x0(%rax,%rax,1)
22: 00 00 00 00
26: 0f 1f 40 00 nopl 0x0(%rax)
2a:* 48 83 ec 08 sub $0x8,%rsp <-- trapping instruction
2e: 83 ff 0e cmp $0xe,%edi
31: 74 07 je 0x3a
33: 31 ff xor %edi,%edi
35: e8 90 88 ff ff callq 0xffffffffffff88ca
3a: bf 01 00 00 00 mov $0x1,%edi
3f: e8 .byte 0xe8

Code starting with the faulting instruction
===========================================
0: 48 83 ec 08 sub $0x8,%rsp
4: 83 ff 0e cmp $0xe,%edi
7: 74 07 je 0x10
9: 31 ff xor %edi,%edi
b: e8 90 88 ff ff callq 0xffffffffffff88a0
10: bf 01 00 00 00 mov $0x1,%edi
15: e8 .byte 0xe8
[ 92.898852][ T3869] RSP: 002b:00007fff6d1a38b8 EFLAGS: 00000246 ORIG_RAX: 0000000000000013
[ 92.906374][ T3869] RAX: 0000000000000000 RBX: 0000000000000002 RCX: 00007f1103c539b9
[ 92.910054][ T3869] RDX: 00007fff6d1a38c0 RSI: 00007fff6d1a39f0 RDI: 000000000000000e
[ 92.917862][ T3869] RBP: 00007f11025d5000 R08: 00000000000000c6 R09: 000b352037dbdb35
[ 92.921591][ T3869] R10: 000000000000fffa R11: 0000000000000246 R12: 0000000000000013
[ 92.925314][ T3869] R13: 00007f1103d21580 R14: 00007f11025d5058 R15: 00007f11025d5000
[ 92.929051][ T3869] </TASK>


If you fix the issue, kindly add following tag
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-lkp/[email protected]


To reproduce:

# build kernel
cd linux
cp config-6.1.0-14365-gc908f8e6a3a1 .config
make HOSTCC=gcc-11 CC=gcc-11 ARCH=x86_64 olddefconfig prepare modules_prepare bzImage modules
make HOSTCC=gcc-11 CC=gcc-11 ARCH=x86_64 INSTALL_MOD_PATH=<mod-install-dir> modules_install
cd <mod-install-dir>
find lib/ | cpio -o -H newc --quiet | gzip > modules.cgz


git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
bin/lkp qemu -k <bzImage> -m modules.cgz job-script # job-script is attached in this email

# if come across any failure that blocks the test,
# please remove ~/.lkp and /lkp dir to run from a clean state.


--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests


Attachments:
(No filename) (10.48 kB)
config-6.1.0-14365-gc908f8e6a3a1 (174.61 kB)
job-script (5.11 kB)
dmesg.xz (31.62 kB)
Download all attachments