From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id D357DC0015E for ; Thu, 13 Jul 2023 14:52:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4E002900034; Thu, 13 Jul 2023 10:52:57 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 468C690001C; Thu, 13 Jul 2023 10:52:57 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 26DC2900034; Thu, 13 Jul 2023 10:52:57 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 0F2A790001C for ; Thu, 13 Jul 2023 10:52:57 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id B89BA160279 for ; Thu, 13 Jul 2023 14:52:56 +0000 (UTC) X-FDA: 81006880752.24.E5A88D0 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf12.hostedemail.com (Postfix) with ESMTP id 8AD9B40021; Thu, 13 Jul 2023 14:52:52 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="RS/6gRgq"; spf=pass (imf12.hostedemail.com: domain of brauner@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=brauner@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1689259974; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=sOlomhEhZcfQ2pureOSGzK4/KmeeEQMFo2ypJMHiWr0=; b=hMh5HGwWBGfKCJpiV3gXCz8gSeePa4KNdDS048UHJIWKyB30rR5kLn9KqSt/5yxcXqv7eK bjI6crHfDUVJ2QjBGmmHzNVpPyehOKqYS3ORBvLzksOwTk+DTo6JTLF5DuyPL+lasnNefN M58cEv2tFUvyyr71dbMbA7GC76+rpxw= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1689259974; a=rsa-sha256; cv=none; b=hg63pdg1E57K8COuTy1Nfzz3eRDgmnstv0kueQ2ikMhxj0KjUVG0wBC9DNQJJ0iWdR5Ewo aO37MP7WJQwNf80IsrdV3y0lw9mZwTGyD/WBsnY9w892JoJpwLGUXRp4541HaOiXkjN7nN SHrMHfQde1+XypN6lCmcWAOHCURoIG8= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="RS/6gRgq"; spf=pass (imf12.hostedemail.com: domain of brauner@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=brauner@kernel.org; dmarc=pass (policy=none) header.from=kernel.org Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 26627615BE; Thu, 13 Jul 2023 14:52:52 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id CA899C433C7; Thu, 13 Jul 2023 14:52:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1689259971; bh=R4Hkl4iroQ+C4d3AaFA6aa52uzI7aPnAyTDDN/lwc78=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=RS/6gRgqvF3ybwn8Ovqq43iLhI7Yv15SZZP2U5CNn6IhgXHQH9mo4nByA/g4W736R AkMi1WqcMsHhNrtO7dGPqCwKIYlgvLK+7FW5vJj2elL3FTubJgM81//BHDuM8QqyRP Csacjb3yQ3aRtIn7A1i9sInqtS8VRY/O0QwqwwSlh+wJ04gvPtGAbDZaSnmdfd209l vt3KpAe4HIE13hYrO1C7jusvVYug5pHUh1tQDDmm+XtqhW6CbXRqbqDYOCl1W9QxOQ VXNT8IMNIH9hLd1wzLndR4KoVQB79H/FYLq25P8lOlildm0R+bpsuSo9TwClPA0kLV csu2vX1i0ECSg== Date: Thu, 13 Jul 2023 16:52:34 +0200 From: Christian Brauner To: Sean Christopherson Cc: linux-fsdevel@vger.kernel.org, Vitaly Kuznetsov , Paolo Bonzini , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, David Woodhouse , Paul Durrant , Oded Gabbay , Wu Hao , Tom Rix , Moritz Fischer , Xu Yilun , Zhenyu Wang , Zhi Wang , Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , Tvrtko Ursulin , David Airlie , Daniel Vetter , Leon Romanovsky , Jason Gunthorpe , Frederic Barrat , Andrew Donnellan , Arnd Bergmann , Greg Kroah-Hartman , Eric Farman , Matthew Rosato , Halil Pasic , Vineeth Vijayan , Peter Oberparleiter , Heiko Carstens , Vasily Gorbik , Alexander Gordeev , Christian Borntraeger , Sven Schnelle , Tony Krowiak , Jason Herne , Harald Freudenberger , "Michael S. Tsirkin" , Jason Wang , Xuan Zhuo , Diana Craciun , Alex Williamson , Eric Auger , Fei Li , Benjamin LaHaise , Johannes Weiner , Michal Hocko , Roman Gushchin , Shakeel Butt , Muchun Song , Kirti Wankhede , kvm@vger.kernel.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-fpga@vger.kernel.org, intel-gvt-dev@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, linux-rdma@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-s390@vger.kernel.org, linux-usb@vger.kernel.org, virtualization@lists.linux-foundation.org, netdev@vger.kernel.org, linux-aio@kvack.org, cgroups@vger.kernel.org, linux-mm@kvack.org, Jens Axboe , Pavel Begunkov , io-uring@vger.kernel.org Subject: Re: [PATCH 2/2] eventfd: simplify eventfd_signal_mask() Message-ID: <20230713-mahnen-drosseln-fa717117e827@brauner> References: <20230713-vfs-eventfd-signal-v1-0-7fda6c5d212b@kernel.org> <20230713-vfs-eventfd-signal-v1-2-7fda6c5d212b@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-Stat-Signature: nr1w8imhp174jcnh6kimgtxeiot3fkgw X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 8AD9B40021 X-Rspam-User: X-HE-Tag: 1689259972-772701 X-HE-Meta: U2FsdGVkX18Vvn8h7fZgC+V4S8HbSkQtlHYgKQyMnmprKSiJUWfpNceLZ+Zzf4sJ0tb/wFVdqexDLAQ9O7+1+Ygx4xWTHxJGH/WNpcVXL5Um3+T3bedVL1JQNwkUJOuF1XqvZxcAKwmIeqaCvh7WQJSgSSxSTfhu3dg9cnbALnomHAgUArwhmcPM7yjeEMSOsmGRGTSCHRqBaGEFDSiBallmnd8yzJj4dv4FO3XY20ueFjKbmC5MY3uvau6W+hcakitm80oK2kazNS8nESHvRALf9tH353jUeKjMRYKTYAtkI734Anw8ghGXBaoleawgsgSPVKeE02HeGjlO8l7nvX7vgo4+qHtq5UGCwbTNWiRBRT8Z1pyfT+g2W0JCbjixwfuFGwlz3Jsw3EQYGM6vwf8xaJs5VQLeXr9Rm5EILBc6uLT0UZ3KdErMxeZ9KEp+Q5H4OXhd6n5on3AzG8eieAfIIThwkKrn2xo/7rqh5vdHzI6S+2Ic+4To6D6ddJKnG7tojbEDsDmNidns0hFVWUe6MrsbVRdfZtsDjzJr5zoreRMiMIdQN6b0nUyQxRIaxKNZ7gOcz9UJb7spgnX2jIndziE9Ojl3UTAxuPFZmhh8Y2JiWgYgugQZyjTLIAZ/vRu23GPi/rxiBaNLnI8VsxLpDvntr6xSGA7w0X5xq3HIw3gfKvwkMYg+PKoBGxk3l9G8cEbVdeJyNqRWdc7z+E2W9sJD78vpZV0WU5trxfME5zhIDZWkkUzqmouuMy+Zu/Xcbb5MGWqsj41+rMTM7+BLM3Tzzd8Nw53gy1Lj+H9wcCrTUmVATJpqa4SknQ1T7TtSgBohfP2EozxNUqLNbHoPP3+Ep1FdnIGlsu/vxU8Z/CCUHZj533WEDqK9JKKAczqncPoMk4sfBN91t/SAMPgJgGc4LzbwYtmEyi6EhSWZuQFeT/3Q306MVYN2OjoTrlMGS2NnTesduZbiFmh KI1k5w1j lHp6oH+BHlDQZQv0uG4p8R4yhCR5twQwJxcNyAKNyFINe4G0/mJfZLwCgqYZXFdD0RDxWtNrQR1tP7l+Q0gyj7RBS0qTsR3ckjwgh6gMiHNzK4jGvoM1neq/86IFj8kT6L8sDskIzCzn9RZ/Cv/KcvAzho1gN4Zl6vhEf/O0Rl95/ypARtdgQ3gdTsuuVxC6A6E2k/NEU3Q7vJ9hhPLdzIfjFPqUEE1dFBzbWw2iK/ki/6Gt284LKFlgHQAhiBEzDrcR+U61gebSo8RJkAARwD2YDVrd48JlRZ4kXykmqY55SUZwzG68uIIxbnRFpdANyuoVgfMdUMMogvadyZppONtM9hS7vESIz/HyGMGJYkvr2TdsOE5phRlVUNnfoLiGr8Zu1D33C6n24ugaWuIVvTy7VDA== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Jul 13, 2023 at 07:33:05AM -0700, Sean Christopherson wrote: > On Thu, Jul 13, 2023, Christian Brauner wrote: > > diff --git a/fs/eventfd.c b/fs/eventfd.c > > index dc9e01053235..077be5da72bd 100644 > > --- a/fs/eventfd.c > > +++ b/fs/eventfd.c > > @@ -43,9 +43,10 @@ struct eventfd_ctx { > > int id; > > }; > > > > -__u64 eventfd_signal_mask(struct eventfd_ctx *ctx, __u64 n, __poll_t mask) > > +bool eventfd_signal_mask(struct eventfd_ctx *ctx, __poll_t mask) > > { > > unsigned long flags; > > + __u64 n = 1; > > > > /* > > * Deadlock or stack overflow issues can happen if we recurse here > > @@ -68,7 +69,7 @@ __u64 eventfd_signal_mask(struct eventfd_ctx *ctx, __u64 n, __poll_t mask) > > current->in_eventfd = 0; > > spin_unlock_irqrestore(&ctx->wqh.lock, flags); > > > > - return n; > > + return n == 1; > > } > > ... > > > @@ -58,13 +58,12 @@ static inline struct eventfd_ctx *eventfd_ctx_fdget(int fd) > > return ERR_PTR(-ENOSYS); > > } > > > > -static inline int eventfd_signal(struct eventfd_ctx *ctx) > > +static inline bool eventfd_signal(struct eventfd_ctx *ctx) > > { > > return -ENOSYS; > > } > > > > -static inline int eventfd_signal_mask(struct eventfd_ctx *ctx, __u64 n, > > - unsigned mask) > > +static inline bool eventfd_signal_mask(struct eventfd_ctx *ctx, unsigned mask) > > { > > return -ENOSYS; > > This will morph to "true" for what should be an error case. One option would be Ewww, that means it did return -ENOSYS before any of this. > to have eventfd_signal_mask() return 0/-errno instead of the count, but looking > at all the callers, nothing ever actually consumes the result. > > KVMGT morphs failure into -EFAULT > > if (vgpu->msi_trigger && eventfd_signal(vgpu->msi_trigger, 1) != 1) > return -EFAULT; > > but the only caller of that user ignores the return value. > > if (vgpu_vreg(vgpu, i915_mmio_reg_offset(GEN8_MASTER_IRQ)) > & ~GEN8_MASTER_IRQ_CONTROL) > inject_virtual_interrupt(vgpu); > > The sample driver in samples/vfio-mdev/mtty.c uses a similar pattern: prints an > error but otherwise ignores the result. > > So why not return nothing? That will simplify eventfd_signal_mask() a wee bit > more, and eliminate that bizarre return value confusion for the ugly stubs, e.g. Yeah, it used to return an int in the non-eventfd and a __u64 in the eventfd case. > > void eventfd_signal_mask(struct eventfd_ctx *ctx, unsigned mask) > { > unsigned long flags; > > /* > * Deadlock or stack overflow issues can happen if we recurse here > * through waitqueue wakeup handlers. If the caller users potentially > * nested waitqueues with custom wakeup handlers, then it should > * check eventfd_signal_allowed() before calling this function. If > * it returns false, the eventfd_signal() call should be deferred to a > * safe context. > */ > if (WARN_ON_ONCE(current->in_eventfd)) > return; > > spin_lock_irqsave(&ctx->wqh.lock, flags); > current->in_eventfd = 1; > if (ctx->count < ULLONG_MAX) > ctx->count++; > if (waitqueue_active(&ctx->wqh)) > wake_up_locked_poll(&ctx->wqh, EPOLLIN | mask); > current->in_eventfd = 0; > spin_unlock_irqrestore(&ctx->wqh.lock, flags); > } > > You could even go further and unify the real and stub versions of eventfd_signal(). The reason I didn't make eventfd_signal_mask() return void was that it was called from eventfd_signal() which did, I didn't realize the caller didn't actually consume the return value. If we can let both return void it gets simpler. Thanks for that.