From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Christopherson Subject: Re: [PATCH 2/2] eventfd: simplify eventfd_signal_mask() Date: Thu, 13 Jul 2023 07:33:05 -0700 Message-ID: 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="us-ascii" Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1689258787; x=1691850787; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=fr2vYa/Ch6xYV487nmzxcm8iM/FlpnkWVHQ5jynQTbg=; b=jacaK9M2p8ae07DYtf3WTwW4sJ88AeR0PMoTM/K8Zom80G9CNsRED8/7OkNACpL4FI vSrK+poAQrpW9b9XXAEX3N+/eJqGG0fl2uSilp/V2zhMmynFGmkNp19rQMAPCUIgro+L 8wZFnIyWuBTpd6inSk0jhwWPq/qOKDhiCmrdy2nICENUOLQPMx4zm5vkc1J+N+jf8F/9 NWijWeXBKP6nhRWfAZooF9dECtwpwP1B06LROVi+ZRGQWEPDlF+8FcWtS7IpVg38LWBC sabPszkfHQCdZSa4qO28HKir4zE3m+cUi5LtAFjCZb9xEKl25slyjVF3CoaXZrvttifT RnQA== In-Reply-To: <20230713-vfs-eventfd-signal-v1-2-7fda6c5d212b-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> List-ID: Content-Transfer-Encoding: 7bit To: Christian Brauner Cc: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Vitaly Kuznetsov , Paolo Bonzini , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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 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 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. 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(). 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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id EC883C0015E for ; Thu, 13 Jul 2023 14:33:10 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 24BF310E0E3; Thu, 13 Jul 2023 14:33:10 +0000 (UTC) Received: from mail-pg1-x54a.google.com (mail-pg1-x54a.google.com [IPv6:2607:f8b0:4864:20::54a]) by gabe.freedesktop.org (Postfix) with ESMTPS id BC6B410E6EB for ; Thu, 13 Jul 2023 14:33:07 +0000 (UTC) Received: by mail-pg1-x54a.google.com with SMTP id 41be03b00d2f7-55b2ab496ecso400663a12.2 for ; Thu, 13 Jul 2023 07:33:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1689258787; x=1691850787; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=fr2vYa/Ch6xYV487nmzxcm8iM/FlpnkWVHQ5jynQTbg=; b=jacaK9M2p8ae07DYtf3WTwW4sJ88AeR0PMoTM/K8Zom80G9CNsRED8/7OkNACpL4FI vSrK+poAQrpW9b9XXAEX3N+/eJqGG0fl2uSilp/V2zhMmynFGmkNp19rQMAPCUIgro+L 8wZFnIyWuBTpd6inSk0jhwWPq/qOKDhiCmrdy2nICENUOLQPMx4zm5vkc1J+N+jf8F/9 NWijWeXBKP6nhRWfAZooF9dECtwpwP1B06LROVi+ZRGQWEPDlF+8FcWtS7IpVg38LWBC sabPszkfHQCdZSa4qO28HKir4zE3m+cUi5LtAFjCZb9xEKl25slyjVF3CoaXZrvttifT RnQA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689258787; x=1691850787; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=fr2vYa/Ch6xYV487nmzxcm8iM/FlpnkWVHQ5jynQTbg=; b=T263ELBL+VMM2b6gyqmHBhFX+Uu/YloW+HIR+GmVtA1wcOok4XIluCRDcnnbTkPV3b ljUIKIzP200f4I/mwnFgUCWBBP+moQqoUJMe59AtZh4O959lpV7zvRQJeTKid0+RmS8K nVjMk7f6p933AJxTi/fMReKlv5FFuCFzd1+rC8F5nfexwL/Jr+q5B9Hdhjm6GWWX2duQ OfE/kz2e5RAumpAGxLUkLkCi7y9FDsDaGWPCA4DSjkreQQQd26pp7pbaFmQrC2RlXlAF Z4cCHu8Stmpn4L6nxjybZ1yjx+93ycqP8Qzy4gU2BSkH8yO4CaDc8sgrR6eveKgSnL8G xAYA== X-Gm-Message-State: ABy/qLamclffbOoV0+jRyLLnM9EtBPK/AQlufj5vbQwOXMz7I7nYyjkX YnrkcvRqspUms+nqVB479UR5gNJrGcs= X-Google-Smtp-Source: APBJJlFAAhWItEMB5PDxlVd4/XD0kDAeGPn5ZZPTnMU3ojU8FDM9Oji9AhkfLIh7rpZuc1o0H3AhYJQLwhY= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a17:902:b282:b0:1ba:1704:89d1 with SMTP id u2-20020a170902b28200b001ba170489d1mr5846plr.10.1689258786828; Thu, 13 Jul 2023 07:33:06 -0700 (PDT) Date: Thu, 13 Jul 2023 07:33:05 -0700 In-Reply-To: <20230713-vfs-eventfd-signal-v1-2-7fda6c5d212b@kernel.org> Mime-Version: 1.0 References: <20230713-vfs-eventfd-signal-v1-0-7fda6c5d212b@kernel.org> <20230713-vfs-eventfd-signal-v1-2-7fda6c5d212b@kernel.org> Message-ID: From: Sean Christopherson To: Christian Brauner Content-Type: text/plain; charset="us-ascii" Subject: Re: [Intel-gfx] [PATCH 2/2] eventfd: simplify eventfd_signal_mask() X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-aio@kvack.org, linux-usb@vger.kernel.org, Matthew Rosato , Paul Durrant , Tom Rix , Jason Wang , dri-devel@lists.freedesktop.org, Michal Hocko , linux-mm@kvack.org, Kirti Wankhede , Paolo Bonzini , Jens Axboe , Vineeth Vijayan , Diana Craciun , Alexander Gordeev , David Airlie , Xuan Zhuo , Shakeel Butt , Vasily Gorbik , Leon Romanovsky , Harald Freudenberger , Fei Li , x86@kernel.org, Roman Gushchin , Halil Pasic , Jason Gunthorpe , Ingo Molnar , intel-gfx@lists.freedesktop.org, Christian Borntraeger , linux-fpga@vger.kernel.org, Wu Hao , Jason Herne , Eric Farman , Dave Hansen , Andrew Donnellan , Arnd Bergmann , linux-s390@vger.kernel.org, Heiko Carstens , linuxppc-dev@lists.ozlabs.org, Frederic Barrat , Moritz Fischer , kvm@vger.kernel.org, Rodrigo Vivi , cgroups@vger.kernel.org, Thomas Gleixner , virtualization@lists.linux-foundation.org, intel-gvt-dev@lists.freedesktop.org, io-uring@vger.kernel.org, netdev@vger.kernel.org, Tony Krowiak , Pavel Begunkov , Eric Auger , Greg Kroah-Hartman , Oded Gabbay , Muchun Song , Peter Oberparleiter , linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org, Benjamin LaHaise , "Michael S. Tsirkin" , Sven Schnelle , Daniel Vetter , Johannes Weiner , linux-fsdevel@vger.kernel.org, Borislav Petkov , Vitaly Kuznetsov , David Woodhouse , Xu Yilun Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" 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 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. 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(). 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 31329C001DD for ; Thu, 13 Jul 2023 14:33:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230475AbjGMOdM (ORCPT ); Thu, 13 Jul 2023 10:33:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46466 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230346AbjGMOdL (ORCPT ); Thu, 13 Jul 2023 10:33:11 -0400 Received: from mail-pg1-x549.google.com (mail-pg1-x549.google.com [IPv6:2607:f8b0:4864:20::549]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C376926B8 for ; Thu, 13 Jul 2023 07:33:07 -0700 (PDT) Received: by mail-pg1-x549.google.com with SMTP id 41be03b00d2f7-53b9eb7bda0so410481a12.0 for ; Thu, 13 Jul 2023 07:33:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1689258787; x=1691850787; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=fr2vYa/Ch6xYV487nmzxcm8iM/FlpnkWVHQ5jynQTbg=; b=jacaK9M2p8ae07DYtf3WTwW4sJ88AeR0PMoTM/K8Zom80G9CNsRED8/7OkNACpL4FI vSrK+poAQrpW9b9XXAEX3N+/eJqGG0fl2uSilp/V2zhMmynFGmkNp19rQMAPCUIgro+L 8wZFnIyWuBTpd6inSk0jhwWPq/qOKDhiCmrdy2nICENUOLQPMx4zm5vkc1J+N+jf8F/9 NWijWeXBKP6nhRWfAZooF9dECtwpwP1B06LROVi+ZRGQWEPDlF+8FcWtS7IpVg38LWBC sabPszkfHQCdZSa4qO28HKir4zE3m+cUi5LtAFjCZb9xEKl25slyjVF3CoaXZrvttifT RnQA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689258787; x=1691850787; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=fr2vYa/Ch6xYV487nmzxcm8iM/FlpnkWVHQ5jynQTbg=; b=KDFb5CFjFNTnYJmpeGRBQwRDSsDDT8wKTgZDx3R2fLEzXJ+lv9p+WiDBUxr52RYDTc 95WXCDcdeXOicOaVBjwW52b9Z0yjw+WbLoLkV/mgUuBNgzO62VLzLggbhhPU66QswZm1 hUCg9LW+IDv7LNa7gMux0gz+6Tauyz33TqakzxuPoW4YtnWyaWrqOHEmD2lLRAEgjbbQ EislXtDtM2dP5LiBdmcdsLKCVcl0nSok/rzQmLm1MOlWeWXDHSLA98KbuJdiCuMTTLDi QgA5Y1kVIY61jjbPeB0PsJiLkpNVwzk7lzrEA3wX2iqPpkimmGFaQvmKMlaKFq4GKcYY zTng== X-Gm-Message-State: ABy/qLYrgHdefy8/VHaTUvM7Yk64EGRmsU8hd/dbfdM4e5xQ9qEKlCIZ MbVMByCr91gQmBauhFvY8mujiFZzpTc= X-Google-Smtp-Source: APBJJlFAAhWItEMB5PDxlVd4/XD0kDAeGPn5ZZPTnMU3ojU8FDM9Oji9AhkfLIh7rpZuc1o0H3AhYJQLwhY= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a17:902:b282:b0:1ba:1704:89d1 with SMTP id u2-20020a170902b28200b001ba170489d1mr5846plr.10.1689258786828; Thu, 13 Jul 2023 07:33:06 -0700 (PDT) Date: Thu, 13 Jul 2023 07:33:05 -0700 In-Reply-To: <20230713-vfs-eventfd-signal-v1-2-7fda6c5d212b@kernel.org> Mime-Version: 1.0 References: <20230713-vfs-eventfd-signal-v1-0-7fda6c5d212b@kernel.org> <20230713-vfs-eventfd-signal-v1-2-7fda6c5d212b@kernel.org> Message-ID: Subject: Re: [PATCH 2/2] eventfd: simplify eventfd_signal_mask() From: Sean Christopherson To: Christian Brauner 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 Content-Type: text/plain; charset="us-ascii" Precedence: bulk List-ID: X-Mailing-List: io-uring@vger.kernel.org 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 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. 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(). 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 lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 7580FC001B0 for ; Thu, 13 Jul 2023 23:23:37 +0000 (UTC) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20221208 header.b=jacaK9M2; dkim-atps=neutral Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4R29f00pLTz3cm7 for ; Fri, 14 Jul 2023 09:23:36 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20221208 header.b=jacaK9M2; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=flex--seanjc.bounces.google.com (client-ip=2607:f8b0:4864:20::54a; helo=mail-pg1-x54a.google.com; envelope-from=3iguwzaykdcutfbokdhpphmf.dpnmjovyqqd-efwmjtut.pambct.psh@flex--seanjc.bounces.google.com; receiver=lists.ozlabs.org) Received: from mail-pg1-x54a.google.com (mail-pg1-x54a.google.com [IPv6:2607:f8b0:4864:20::54a]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4R1xt218vjz30P3 for ; Fri, 14 Jul 2023 00:33:12 +1000 (AEST) Received: by mail-pg1-x54a.google.com with SMTP id 41be03b00d2f7-55c072efe78so396749a12.3 for ; Thu, 13 Jul 2023 07:33:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1689258787; x=1691850787; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=fr2vYa/Ch6xYV487nmzxcm8iM/FlpnkWVHQ5jynQTbg=; b=jacaK9M2p8ae07DYtf3WTwW4sJ88AeR0PMoTM/K8Zom80G9CNsRED8/7OkNACpL4FI vSrK+poAQrpW9b9XXAEX3N+/eJqGG0fl2uSilp/V2zhMmynFGmkNp19rQMAPCUIgro+L 8wZFnIyWuBTpd6inSk0jhwWPq/qOKDhiCmrdy2nICENUOLQPMx4zm5vkc1J+N+jf8F/9 NWijWeXBKP6nhRWfAZooF9dECtwpwP1B06LROVi+ZRGQWEPDlF+8FcWtS7IpVg38LWBC sabPszkfHQCdZSa4qO28HKir4zE3m+cUi5LtAFjCZb9xEKl25slyjVF3CoaXZrvttifT RnQA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689258787; x=1691850787; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=fr2vYa/Ch6xYV487nmzxcm8iM/FlpnkWVHQ5jynQTbg=; b=QRu8Xm4rD4FyKOEU5DHi9xfAn3H0Fi13EgSBya0EDW5dowzNlaNzXxAd1OpkVmLa6h fkBXqplOUTvEzkR5Ab1Ikoc05DfKT48hcoaToKyO2Ke40iEEJtsxiGO9weCOJVFvr6TE sW74Su4pHe2oq+RxhrGxGCBGQaHW6ngZJ2clMzmqro5vcAEHcL4dns9WPnD3wlc8aYX9 D+3wwmbyNb9lT/IafwRjUxBqTtS+VF7WlETpQ9Q7J+OLI0da/2WEPq8PImDYTKZY8Hrx BN/ZOU9Obwrfi3Z8ChHgZNHtceewHPAxt8pI2KBOJd+ltDth7sCvEs6bCBknzln8ZBfX YEzg== X-Gm-Message-State: ABy/qLb9tO9Fciqs6TG0akSKuV5TdpinctODZBqgdaUzRZMX6oZnv4sr 6cWrSdWtc8L7ZsYwYkTHhkmWyvIPfqA= X-Google-Smtp-Source: APBJJlFAAhWItEMB5PDxlVd4/XD0kDAeGPn5ZZPTnMU3ojU8FDM9Oji9AhkfLIh7rpZuc1o0H3AhYJQLwhY= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a17:902:b282:b0:1ba:1704:89d1 with SMTP id u2-20020a170902b28200b001ba170489d1mr5846plr.10.1689258786828; Thu, 13 Jul 2023 07:33:06 -0700 (PDT) Date: Thu, 13 Jul 2023 07:33:05 -0700 In-Reply-To: <20230713-vfs-eventfd-signal-v1-2-7fda6c5d212b@kernel.org> Mime-Version: 1.0 References: <20230713-vfs-eventfd-signal-v1-0-7fda6c5d212b@kernel.org> <20230713-vfs-eventfd-signal-v1-2-7fda6c5d212b@kernel.org> Message-ID: Subject: Re: [PATCH 2/2] eventfd: simplify eventfd_signal_mask() From: Sean Christopherson To: Christian Brauner Content-Type: text/plain; charset="us-ascii" X-Mailman-Approved-At: Fri, 14 Jul 2023 09:17:19 +1000 X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-aio@kvack.org, linux-usb@vger.kernel.org, Matthew Rosato , Paul Durrant , Tom Rix , Jason Wang , Joonas Lahtinen , dri-devel@lists.freedesktop.org, Michal Hocko , linux-mm@kvack.org, Kirti Wankhede , Paolo Bonzini , Jens Axboe , Vineeth Vijayan , Diana Craciun , netdev@vger.kernel.org, Alexander Gordeev , David Airlie , Xuan Zhuo , Shakeel Butt , Vasily Gorbik , Leon Romanovsky , Harald Freudenberger , Fei Li , x86@kernel.org, Roman Gushchin , Halil Pasic , Jason Gunthorpe , Ingo Molnar , intel-gf x@lists.freedesktop.org, Christian Borntraeger , linux-fpga@vger.kernel.org, Zhi Wang , Wu Hao , Jason Herne , Eric Farman , Dave Hansen , Andrew Donnellan , Arnd Bergmann , linux-s390@vger.kernel.org, Heiko Carstens , linuxppc-dev@lists.ozlabs.org, Zhenyu Wang , Frederic Barrat , Alex Williamson , Moritz Fischer , Jani Nikula , kvm@vger.kernel.org, Rodrigo Vivi , cgroups@vger.kernel.org, Thomas Gleixner , virtualization@lists.linux-foundation.org, intel-gvt-dev@lists.freedesktop.org, io-uring@vger.kernel.org, Tony Krowiak , Tvrtko Ursulin , Pavel Begunkov , Eric Auger , Greg Kroah-Hartman , Oded Gabbay , Muchun Song , Peter Oberparleiter , linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org, Benjamin LaHaise , "Michael S. Tsirkin" , Sven Schnelle , Daniel Vetter , Johannes Weiner , linux-fsdevel@vger.kernel.org, Borislav Petkov , Vitaly Kuznetsov , David Woodhouse , Xu Yilun Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" 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 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. 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(). 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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id CF7C1C001E0 for ; Thu, 13 Jul 2023 14:33:12 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2ED4C10E6EB; Thu, 13 Jul 2023 14:33:10 +0000 (UTC) Received: from mail-pl1-x649.google.com (mail-pl1-x649.google.com [IPv6:2607:f8b0:4864:20::649]) by gabe.freedesktop.org (Postfix) with ESMTPS id B85AC10E0E3 for ; Thu, 13 Jul 2023 14:33:07 +0000 (UTC) Received: by mail-pl1-x649.google.com with SMTP id d9443c01a7336-1b9de7951easo3440615ad.0 for ; Thu, 13 Jul 2023 07:33:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1689258787; x=1691850787; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=fr2vYa/Ch6xYV487nmzxcm8iM/FlpnkWVHQ5jynQTbg=; b=jacaK9M2p8ae07DYtf3WTwW4sJ88AeR0PMoTM/K8Zom80G9CNsRED8/7OkNACpL4FI vSrK+poAQrpW9b9XXAEX3N+/eJqGG0fl2uSilp/V2zhMmynFGmkNp19rQMAPCUIgro+L 8wZFnIyWuBTpd6inSk0jhwWPq/qOKDhiCmrdy2nICENUOLQPMx4zm5vkc1J+N+jf8F/9 NWijWeXBKP6nhRWfAZooF9dECtwpwP1B06LROVi+ZRGQWEPDlF+8FcWtS7IpVg38LWBC sabPszkfHQCdZSa4qO28HKir4zE3m+cUi5LtAFjCZb9xEKl25slyjVF3CoaXZrvttifT RnQA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689258787; x=1691850787; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=fr2vYa/Ch6xYV487nmzxcm8iM/FlpnkWVHQ5jynQTbg=; b=IUejQt4Fu8PMYQxqL7QDwn9ukDs7RxtsHUoyyNyC9JUISAeDT4B0OdMyKMIWybZwG+ O9leBePVlFb+VXdox18avS5Yd1c+WUV417XdItAqivgYJkne+FjI0Thz983x3JBrIWk9 w7CViRiioOtQOwTwr8Qp9WS1m5sC3XnyeA/jqs1/FLcBMDo0pt1Y+vacepw6WI1UttvP ++3s1E87nxAuARinDfEuv6cwNZIEm9d4jK55yaKuSKtPs544bsNH+dKHoH/TjJLft+du 7jnS8aKpwqKHfjcQQiSPIPGM7KCLoiyqB+vh2YAdE4QyJSRCLOgIriyNY/ziUCGmZBZM Ikpg== X-Gm-Message-State: ABy/qLb9GDp1wtpH6wNU53iINkPexRzY6hNybq0fZSUwk0IU+CC6FVAB I6e1lj+MdXHgkgx5+eV9dXa/KrYgkrw= X-Google-Smtp-Source: APBJJlFAAhWItEMB5PDxlVd4/XD0kDAeGPn5ZZPTnMU3ojU8FDM9Oji9AhkfLIh7rpZuc1o0H3AhYJQLwhY= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a17:902:b282:b0:1ba:1704:89d1 with SMTP id u2-20020a170902b28200b001ba170489d1mr5846plr.10.1689258786828; Thu, 13 Jul 2023 07:33:06 -0700 (PDT) Date: Thu, 13 Jul 2023 07:33:05 -0700 In-Reply-To: <20230713-vfs-eventfd-signal-v1-2-7fda6c5d212b@kernel.org> Mime-Version: 1.0 References: <20230713-vfs-eventfd-signal-v1-0-7fda6c5d212b@kernel.org> <20230713-vfs-eventfd-signal-v1-2-7fda6c5d212b@kernel.org> Message-ID: Subject: Re: [PATCH 2/2] eventfd: simplify eventfd_signal_mask() From: Sean Christopherson To: Christian Brauner Content-Type: text/plain; charset="us-ascii" X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-aio@kvack.org, linux-usb@vger.kernel.org, Matthew Rosato , Paul Durrant , Tom Rix , Jason Wang , dri-devel@lists.freedesktop.org, Michal Hocko , linux-mm@kvack.org, Kirti Wankhede , Paolo Bonzini , Jens Axboe , Vineeth Vijayan , Diana Craciun , Alexander Gordeev , Xuan Zhuo , Shakeel Butt , Vasily Gorbik , Leon Romanovsky , Harald Freudenberger , Fei Li , x86@kernel.org, Roman Gushchin , Halil Pasic , Jason Gunthorpe , Ingo Molnar , intel-gfx@lists.freedesktop.org, Christian Borntraeger , linux-fpga@vger.kernel.org, Zhi Wang , Wu Hao , Jason Herne , Eric Farman , Dave Hansen , Andrew Donnellan , Arnd Bergmann , linux-s390@vger.kernel.org, Heiko Carstens , linuxppc-dev@lists.ozlabs.org, Frederic Barrat , Alex Williamson , Moritz Fischer , kvm@vger.kernel.org, Rodrigo Vivi , cgroups@vger.kernel.org, Thomas Gleixner , virtualization@lists.linux-foundation.org, intel-gvt-dev@lists.freedesktop.org, io-uring@vger.kernel.org, netdev@vger.kernel.org, Tony Krowiak , Tvrtko Ursulin , Pavel Begunkov , Eric Auger , Greg Kroah-Hartman , Oded Gabbay , Muchun Song , Peter Oberparleiter , linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org, Benjamin LaHaise , "Michael S. Tsirkin" , Sven Schnelle , Johannes Weiner , linux-fsdevel@vger.kernel.org, Borislav Petkov , Vitaly Kuznetsov , David Woodhouse , Xu Yilun Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" 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 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. 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().