Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/3] Allow in-kernel consumers to drain events from eventfd
       [not found] <1faa5405-3640-f4ad-5cd9-89a9e5e834e9@redhat.com>
@ 2020-10-27 13:55 ` David Woodhouse
  2020-10-27 13:55   ` [PATCH 1/3] eventfd: Export eventfd_ctx_do_read() David Woodhouse
                     ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: David Woodhouse @ 2020-10-27 13:55 UTC (permalink / raw)
  To: bonzini
  Cc: Alex Williamson, Cornelia Huck, Alexander Viro, Jens Axboe, kvm,
	linux-kernel, linux-fsdevel

Paolo pointed out that the KVM eventfd doesn't drain the events from the
irqfd as it handles them, and just lets them accumulate. This is also
true for the VFIO virqfd used for handling acks for level-triggered IRQs.

Export eventfd_ctx_do_read() and make the wakeup functions call it as they
handle their respective events.

David Woodhouse (3):
      eventfd: Export eventfd_ctx_do_read()
      vfio/virqfd: Drain events from eventfd in virqfd_wakeup()
      kvm/eventfd: Drain events from eventfd in irqfd_wakeup()

 drivers/vfio/virqfd.c   | 3 +++
 fs/eventfd.c            | 5 ++++-
 include/linux/eventfd.h | 6 ++++++
 virt/kvm/eventfd.c      | 3 +++
 4 files changed, 16 insertions(+), 1 deletion(-)



^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH 1/3] eventfd: Export eventfd_ctx_do_read()
  2020-10-27 13:55 ` [PATCH 0/3] Allow in-kernel consumers to drain events from eventfd David Woodhouse
@ 2020-10-27 13:55   ` David Woodhouse
  2020-10-27 13:55   ` [PATCH 2/3] vfio/virqfd: Drain events from eventfd in virqfd_wakeup() David Woodhouse
  2020-10-27 13:55   ` [PATCH 3/3] kvm/eventfd: Drain events from eventfd in irqfd_wakeup() David Woodhouse
  2 siblings, 0 replies; 9+ messages in thread
From: David Woodhouse @ 2020-10-27 13:55 UTC (permalink / raw)
  To: bonzini
  Cc: Alex Williamson, Cornelia Huck, Alexander Viro, Jens Axboe, kvm,
	linux-kernel, linux-fsdevel

From: David Woodhouse <dwmw@amazon.co.uk>

Where events are consumed in the kernel, for example by KVM's
irqfd_wakeup() and VFIO's virqfd_wakeup(), they currently lack a
mechanism to drain the eventfd's counter.

Since the wait queue is already locked while the wakeup functions are
invoked, all they really need to do is call eventfd_ctx_do_read().

Add a check for the lock, and export it for them.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 fs/eventfd.c            | 5 ++++-
 include/linux/eventfd.h | 6 ++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/fs/eventfd.c b/fs/eventfd.c
index df466ef81ddd..e265b6dd4f34 100644
--- a/fs/eventfd.c
+++ b/fs/eventfd.c
@@ -182,11 +182,14 @@ static __poll_t eventfd_poll(struct file *file, poll_table *wait)
 	return events;
 }
 
-static void eventfd_ctx_do_read(struct eventfd_ctx *ctx, __u64 *cnt)
+void eventfd_ctx_do_read(struct eventfd_ctx *ctx, __u64 *cnt)
 {
+	lockdep_assert_held(&ctx->wqh.lock);
+
 	*cnt = (ctx->flags & EFD_SEMAPHORE) ? 1 : ctx->count;
 	ctx->count -= *cnt;
 }
+EXPORT_SYMBOL_GPL(eventfd_ctx_do_read);
 
 /**
  * eventfd_ctx_remove_wait_queue - Read the current counter and removes wait queue.
diff --git a/include/linux/eventfd.h b/include/linux/eventfd.h
index dc4fd8a6644d..fa0a524baed0 100644
--- a/include/linux/eventfd.h
+++ b/include/linux/eventfd.h
@@ -41,6 +41,7 @@ struct eventfd_ctx *eventfd_ctx_fileget(struct file *file);
 __u64 eventfd_signal(struct eventfd_ctx *ctx, __u64 n);
 int eventfd_ctx_remove_wait_queue(struct eventfd_ctx *ctx, wait_queue_entry_t *wait,
 				  __u64 *cnt);
+void eventfd_ctx_do_read(struct eventfd_ctx *ctx, __u64 *cnt);
 
 DECLARE_PER_CPU(int, eventfd_wake_count);
 
@@ -82,6 +83,11 @@ static inline bool eventfd_signal_count(void)
 	return false;
 }
 
+static inline void eventfd_ctx_do_read(struct eventfd_ctx *ctx, __u64 *cnt)
+{
+
+}
+
 #endif
 
 #endif /* _LINUX_EVENTFD_H */
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH 2/3] vfio/virqfd: Drain events from eventfd in virqfd_wakeup()
  2020-10-27 13:55 ` [PATCH 0/3] Allow in-kernel consumers to drain events from eventfd David Woodhouse
  2020-10-27 13:55   ` [PATCH 1/3] eventfd: Export eventfd_ctx_do_read() David Woodhouse
@ 2020-10-27 13:55   ` David Woodhouse
  2020-11-06 23:29     ` Alex Williamson
  2020-10-27 13:55   ` [PATCH 3/3] kvm/eventfd: Drain events from eventfd in irqfd_wakeup() David Woodhouse
  2 siblings, 1 reply; 9+ messages in thread
From: David Woodhouse @ 2020-10-27 13:55 UTC (permalink / raw)
  To: bonzini
  Cc: Alex Williamson, Cornelia Huck, Alexander Viro, Jens Axboe, kvm,
	linux-kernel, linux-fsdevel

From: David Woodhouse <dwmw@amazon.co.uk>

Don't allow the events to accumulate in the eventfd counter, drain them
as they are handled.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 drivers/vfio/virqfd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/vfio/virqfd.c b/drivers/vfio/virqfd.c
index 997cb5d0a657..414e98d82b02 100644
--- a/drivers/vfio/virqfd.c
+++ b/drivers/vfio/virqfd.c
@@ -46,6 +46,9 @@ static int virqfd_wakeup(wait_queue_entry_t *wait, unsigned mode, int sync, void
 	__poll_t flags = key_to_poll(key);
 
 	if (flags & EPOLLIN) {
+		u64 cnt;
+		eventfd_ctx_do_read(virqfd->eventfd, &cnt);
+
 		/* An event has been signaled, call function */
 		if ((!virqfd->handler ||
 		     virqfd->handler(virqfd->opaque, virqfd->data)) &&
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH 3/3] kvm/eventfd: Drain events from eventfd in irqfd_wakeup()
  2020-10-27 13:55 ` [PATCH 0/3] Allow in-kernel consumers to drain events from eventfd David Woodhouse
  2020-10-27 13:55   ` [PATCH 1/3] eventfd: Export eventfd_ctx_do_read() David Woodhouse
  2020-10-27 13:55   ` [PATCH 2/3] vfio/virqfd: Drain events from eventfd in virqfd_wakeup() David Woodhouse
@ 2020-10-27 13:55   ` David Woodhouse
  2020-10-27 18:41     ` kernel test robot
                       ` (2 more replies)
  2 siblings, 3 replies; 9+ messages in thread
From: David Woodhouse @ 2020-10-27 13:55 UTC (permalink / raw)
  To: bonzini
  Cc: Alex Williamson, Cornelia Huck, Alexander Viro, Jens Axboe, kvm,
	linux-kernel, linux-fsdevel

From: David Woodhouse <dwmw@amazon.co.uk>

Don't allow the events to accumulate in the eventfd counter, drain them
as they are handled.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 virt/kvm/eventfd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index d6408bb497dc..98b5cfa1d69f 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -193,6 +193,9 @@ irqfd_wakeup(wait_queue_entry_t *wait, unsigned mode, int sync, void *key)
 	int idx;
 
 	if (flags & EPOLLIN) {
+		u64 cnt;
+		eventfd_ctx_do_read(&irqfd->eventfd, &cnt);
+
 		idx = srcu_read_lock(&kvm->irq_srcu);
 		do {
 			seq = read_seqcount_begin(&irqfd->irq_entry_sc);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] kvm/eventfd: Drain events from eventfd in irqfd_wakeup()
  2020-10-27 13:55   ` [PATCH 3/3] kvm/eventfd: Drain events from eventfd in irqfd_wakeup() David Woodhouse
@ 2020-10-27 18:41     ` kernel test robot
  2020-10-27 21:42     ` kernel test robot
  2020-10-27 23:13     ` kernel test robot
  2 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2020-10-27 18:41 UTC (permalink / raw)
  To: David Woodhouse, bonzini
  Cc: kbuild-all, Alex Williamson, Cornelia Huck, Alexander Viro,
	Jens Axboe, kvm, linux-kernel, linux-fsdevel

[-- Attachment #1: Type: text/plain, Size: 4397 bytes --]

Hi David,

I love your patch! Yet something to improve:

[auto build test ERROR on vfio/next]
[also build test ERROR on vhost/linux-next linus/master kvm/linux-next v5.10-rc1 next-20201027]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/David-Woodhouse/Allow-in-kernel-consumers-to-drain-events-from-eventfd/20201027-215658
base:   https://github.com/awilliam/linux-vfio.git next
config: x86_64-randconfig-s021-20201027 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-56-gc09e8239-dirty
        # https://github.com/0day-ci/linux/commit/dc45dd9af28fede8f8dd29b705b90f78cf87538c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review David-Woodhouse/Allow-in-kernel-consumers-to-drain-events-from-eventfd/20201027-215658
        git checkout dc45dd9af28fede8f8dd29b705b90f78cf87538c
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   arch/x86/kvm/../../../virt/kvm/eventfd.c: In function 'irqfd_wakeup':
>> arch/x86/kvm/../../../virt/kvm/eventfd.c:197:23: error: passing argument 1 of 'eventfd_ctx_do_read' from incompatible pointer type [-Werror=incompatible-pointer-types]
     197 |   eventfd_ctx_do_read(&irqfd->eventfd, &cnt);
         |                       ^~~~~~~~~~~~~~~
         |                       |
         |                       struct eventfd_ctx **
   In file included from arch/x86/kvm/../../../virt/kvm/eventfd.c:21:
   include/linux/eventfd.h:44:46: note: expected 'struct eventfd_ctx *' but argument is of type 'struct eventfd_ctx **'
      44 | void eventfd_ctx_do_read(struct eventfd_ctx *ctx, __u64 *cnt);
         |                          ~~~~~~~~~~~~~~~~~~~~^~~
   cc1: some warnings being treated as errors

vim +/eventfd_ctx_do_read +197 arch/x86/kvm/../../../virt/kvm/eventfd.c

   180	
   181	/*
   182	 * Called with wqh->lock held and interrupts disabled
   183	 */
   184	static int
   185	irqfd_wakeup(wait_queue_entry_t *wait, unsigned mode, int sync, void *key)
   186	{
   187		struct kvm_kernel_irqfd *irqfd =
   188			container_of(wait, struct kvm_kernel_irqfd, wait);
   189		__poll_t flags = key_to_poll(key);
   190		struct kvm_kernel_irq_routing_entry irq;
   191		struct kvm *kvm = irqfd->kvm;
   192		unsigned seq;
   193		int idx;
   194	
   195		if (flags & EPOLLIN) {
   196			u64 cnt;
 > 197			eventfd_ctx_do_read(&irqfd->eventfd, &cnt);
   198	
   199			idx = srcu_read_lock(&kvm->irq_srcu);
   200			do {
   201				seq = read_seqcount_begin(&irqfd->irq_entry_sc);
   202				irq = irqfd->irq_entry;
   203			} while (read_seqcount_retry(&irqfd->irq_entry_sc, seq));
   204			/* An event has been signaled, inject an interrupt */
   205			if (kvm_arch_set_irq_inatomic(&irq, kvm,
   206						      KVM_USERSPACE_IRQ_SOURCE_ID, 1,
   207						      false) == -EWOULDBLOCK)
   208				schedule_work(&irqfd->inject);
   209			srcu_read_unlock(&kvm->irq_srcu, idx);
   210		}
   211	
   212		if (flags & EPOLLHUP) {
   213			/* The eventfd is closing, detach from KVM */
   214			unsigned long iflags;
   215	
   216			spin_lock_irqsave(&kvm->irqfds.lock, iflags);
   217	
   218			/*
   219			 * We must check if someone deactivated the irqfd before
   220			 * we could acquire the irqfds.lock since the item is
   221			 * deactivated from the KVM side before it is unhooked from
   222			 * the wait-queue.  If it is already deactivated, we can
   223			 * simply return knowing the other side will cleanup for us.
   224			 * We cannot race against the irqfd going away since the
   225			 * other side is required to acquire wqh->lock, which we hold
   226			 */
   227			if (irqfd_is_active(irqfd))
   228				irqfd_deactivate(irqfd);
   229	
   230			spin_unlock_irqrestore(&kvm->irqfds.lock, iflags);
   231		}
   232	
   233		return 0;
   234	}
   235	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 42031 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] kvm/eventfd: Drain events from eventfd in irqfd_wakeup()
  2020-10-27 13:55   ` [PATCH 3/3] kvm/eventfd: Drain events from eventfd in irqfd_wakeup() David Woodhouse
  2020-10-27 18:41     ` kernel test robot
@ 2020-10-27 21:42     ` kernel test robot
  2020-10-27 23:13     ` kernel test robot
  2 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2020-10-27 21:42 UTC (permalink / raw)
  To: David Woodhouse, bonzini
  Cc: kbuild-all, clang-built-linux, Alex Williamson, Cornelia Huck,
	Alexander Viro, Jens Axboe, kvm, linux-kernel, linux-fsdevel

[-- Attachment #1: Type: text/plain, Size: 11205 bytes --]

Hi David,

I love your patch! Yet something to improve:

[auto build test ERROR on vfio/next]
[also build test ERROR on vhost/linux-next linus/master kvm/linux-next v5.10-rc1 next-20201027]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/David-Woodhouse/Allow-in-kernel-consumers-to-drain-events-from-eventfd/20201027-215658
base:   https://github.com/awilliam/linux-vfio.git next
config: s390-randconfig-r023-20201027 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project f2c25c70791de95d2466e09b5b58fc37f6ccd7a4)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # https://github.com/0day-ci/linux/commit/dc45dd9af28fede8f8dd29b705b90f78cf87538c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review David-Woodhouse/Allow-in-kernel-consumers-to-drain-events-from-eventfd/20201027-215658
        git checkout dc45dd9af28fede8f8dd29b705b90f78cf87538c
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from arch/s390/include/asm/kvm_para.h:25:
   In file included from arch/s390/include/asm/diag.h:12:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:11:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:72:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
           ___constant_swab32(x) :                 \
                              ^
   include/uapi/linux/swab.h:21:12: note: expanded from macro '___constant_swab32'
           (((__u32)(x) & (__u32)0x00ff0000UL) >>  8) |            \
                     ^
   In file included from arch/s390/kvm/../../../virt/kvm/eventfd.c:12:
   In file included from include/linux/kvm_host.h:32:
   In file included from include/linux/kvm_para.h:5:
   In file included from include/uapi/linux/kvm_para.h:36:
   In file included from arch/s390/include/asm/kvm_para.h:25:
   In file included from arch/s390/include/asm/diag.h:12:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:11:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:72:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
           ___constant_swab32(x) :                 \
                              ^
   include/uapi/linux/swab.h:22:12: note: expanded from macro '___constant_swab32'
           (((__u32)(x) & (__u32)0xff000000UL) >> 24)))
                     ^
   In file included from arch/s390/kvm/../../../virt/kvm/eventfd.c:12:
   In file included from include/linux/kvm_host.h:32:
   In file included from include/linux/kvm_para.h:5:
   In file included from include/uapi/linux/kvm_para.h:36:
   In file included from arch/s390/include/asm/kvm_para.h:25:
   In file included from arch/s390/include/asm/diag.h:12:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:11:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:72:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:120:12: note: expanded from macro '__swab32'
           __fswab32(x))
                     ^
   In file included from arch/s390/kvm/../../../virt/kvm/eventfd.c:12:
   In file included from include/linux/kvm_host.h:32:
   In file included from include/linux/kvm_para.h:5:
   In file included from include/uapi/linux/kvm_para.h:36:
   In file included from arch/s390/include/asm/kvm_para.h:25:
   In file included from arch/s390/include/asm/diag.h:12:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:11:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:72:
   include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsb(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsw(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsl(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesb(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesw(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesl(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
>> arch/s390/kvm/../../../virt/kvm/eventfd.c:197:23: error: incompatible pointer types passing 'struct eventfd_ctx **' to parameter of type 'struct eventfd_ctx *'; remove & [-Werror,-Wincompatible-pointer-types]
                   eventfd_ctx_do_read(&irqfd->eventfd, &cnt);
                                       ^~~~~~~~~~~~~~~
   include/linux/eventfd.h:44:46: note: passing argument to parameter 'ctx' here
   void eventfd_ctx_do_read(struct eventfd_ctx *ctx, __u64 *cnt);
                                                ^
   20 warnings and 1 error generated.

vim +197 arch/s390/kvm/../../../virt/kvm/eventfd.c

   180	
   181	/*
   182	 * Called with wqh->lock held and interrupts disabled
   183	 */
   184	static int
   185	irqfd_wakeup(wait_queue_entry_t *wait, unsigned mode, int sync, void *key)
   186	{
   187		struct kvm_kernel_irqfd *irqfd =
   188			container_of(wait, struct kvm_kernel_irqfd, wait);
   189		__poll_t flags = key_to_poll(key);
   190		struct kvm_kernel_irq_routing_entry irq;
   191		struct kvm *kvm = irqfd->kvm;
   192		unsigned seq;
   193		int idx;
   194	
   195		if (flags & EPOLLIN) {
   196			u64 cnt;
 > 197			eventfd_ctx_do_read(&irqfd->eventfd, &cnt);
   198	
   199			idx = srcu_read_lock(&kvm->irq_srcu);
   200			do {
   201				seq = read_seqcount_begin(&irqfd->irq_entry_sc);
   202				irq = irqfd->irq_entry;
   203			} while (read_seqcount_retry(&irqfd->irq_entry_sc, seq));
   204			/* An event has been signaled, inject an interrupt */
   205			if (kvm_arch_set_irq_inatomic(&irq, kvm,
   206						      KVM_USERSPACE_IRQ_SOURCE_ID, 1,
   207						      false) == -EWOULDBLOCK)
   208				schedule_work(&irqfd->inject);
   209			srcu_read_unlock(&kvm->irq_srcu, idx);
   210		}
   211	
   212		if (flags & EPOLLHUP) {
   213			/* The eventfd is closing, detach from KVM */
   214			unsigned long iflags;
   215	
   216			spin_lock_irqsave(&kvm->irqfds.lock, iflags);
   217	
   218			/*
   219			 * We must check if someone deactivated the irqfd before
   220			 * we could acquire the irqfds.lock since the item is
   221			 * deactivated from the KVM side before it is unhooked from
   222			 * the wait-queue.  If it is already deactivated, we can
   223			 * simply return knowing the other side will cleanup for us.
   224			 * We cannot race against the irqfd going away since the
   225			 * other side is required to acquire wqh->lock, which we hold
   226			 */
   227			if (irqfd_is_active(irqfd))
   228				irqfd_deactivate(irqfd);
   229	
   230			spin_unlock_irqrestore(&kvm->irqfds.lock, iflags);
   231		}
   232	
   233		return 0;
   234	}
   235	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26571 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] kvm/eventfd: Drain events from eventfd in irqfd_wakeup()
  2020-10-27 13:55   ` [PATCH 3/3] kvm/eventfd: Drain events from eventfd in irqfd_wakeup() David Woodhouse
  2020-10-27 18:41     ` kernel test robot
  2020-10-27 21:42     ` kernel test robot
@ 2020-10-27 23:13     ` kernel test robot
  2 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2020-10-27 23:13 UTC (permalink / raw)
  To: David Woodhouse, bonzini
  Cc: kbuild-all, clang-built-linux, Alex Williamson, Cornelia Huck,
	Alexander Viro, Jens Axboe, kvm, linux-kernel, linux-fsdevel

[-- Attachment #1: Type: text/plain, Size: 4377 bytes --]

Hi David,

I love your patch! Yet something to improve:

[auto build test ERROR on vfio/next]
[also build test ERROR on vhost/linux-next linus/master kvm/linux-next v5.10-rc1 next-20201027]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/David-Woodhouse/Allow-in-kernel-consumers-to-drain-events-from-eventfd/20201027-215658
base:   https://github.com/awilliam/linux-vfio.git next
config: x86_64-randconfig-a004-20201026 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project f2c25c70791de95d2466e09b5b58fc37f6ccd7a4)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/dc45dd9af28fede8f8dd29b705b90f78cf87538c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review David-Woodhouse/Allow-in-kernel-consumers-to-drain-events-from-eventfd/20201027-215658
        git checkout dc45dd9af28fede8f8dd29b705b90f78cf87538c
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> arch/x86/kvm/../../../virt/kvm/eventfd.c:197:23: error: incompatible pointer types passing 'struct eventfd_ctx **' to parameter of type 'struct eventfd_ctx *'; remove & [-Werror,-Wincompatible-pointer-types]
                   eventfd_ctx_do_read(&irqfd->eventfd, &cnt);
                                       ^~~~~~~~~~~~~~~
   include/linux/eventfd.h:44:46: note: passing argument to parameter 'ctx' here
   void eventfd_ctx_do_read(struct eventfd_ctx *ctx, __u64 *cnt);
                                                ^
   1 error generated.

vim +197 arch/x86/kvm/../../../virt/kvm/eventfd.c

   180	
   181	/*
   182	 * Called with wqh->lock held and interrupts disabled
   183	 */
   184	static int
   185	irqfd_wakeup(wait_queue_entry_t *wait, unsigned mode, int sync, void *key)
   186	{
   187		struct kvm_kernel_irqfd *irqfd =
   188			container_of(wait, struct kvm_kernel_irqfd, wait);
   189		__poll_t flags = key_to_poll(key);
   190		struct kvm_kernel_irq_routing_entry irq;
   191		struct kvm *kvm = irqfd->kvm;
   192		unsigned seq;
   193		int idx;
   194	
   195		if (flags & EPOLLIN) {
   196			u64 cnt;
 > 197			eventfd_ctx_do_read(&irqfd->eventfd, &cnt);
   198	
   199			idx = srcu_read_lock(&kvm->irq_srcu);
   200			do {
   201				seq = read_seqcount_begin(&irqfd->irq_entry_sc);
   202				irq = irqfd->irq_entry;
   203			} while (read_seqcount_retry(&irqfd->irq_entry_sc, seq));
   204			/* An event has been signaled, inject an interrupt */
   205			if (kvm_arch_set_irq_inatomic(&irq, kvm,
   206						      KVM_USERSPACE_IRQ_SOURCE_ID, 1,
   207						      false) == -EWOULDBLOCK)
   208				schedule_work(&irqfd->inject);
   209			srcu_read_unlock(&kvm->irq_srcu, idx);
   210		}
   211	
   212		if (flags & EPOLLHUP) {
   213			/* The eventfd is closing, detach from KVM */
   214			unsigned long iflags;
   215	
   216			spin_lock_irqsave(&kvm->irqfds.lock, iflags);
   217	
   218			/*
   219			 * We must check if someone deactivated the irqfd before
   220			 * we could acquire the irqfds.lock since the item is
   221			 * deactivated from the KVM side before it is unhooked from
   222			 * the wait-queue.  If it is already deactivated, we can
   223			 * simply return knowing the other side will cleanup for us.
   224			 * We cannot race against the irqfd going away since the
   225			 * other side is required to acquire wqh->lock, which we hold
   226			 */
   227			if (irqfd_is_active(irqfd))
   228				irqfd_deactivate(irqfd);
   229	
   230			spin_unlock_irqrestore(&kvm->irqfds.lock, iflags);
   231		}
   232	
   233		return 0;
   234	}
   235	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 40494 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] vfio/virqfd: Drain events from eventfd in virqfd_wakeup()
  2020-10-27 13:55   ` [PATCH 2/3] vfio/virqfd: Drain events from eventfd in virqfd_wakeup() David Woodhouse
@ 2020-11-06 23:29     ` Alex Williamson
  2020-11-08  9:17       ` Paolo Bonzini
  0 siblings, 1 reply; 9+ messages in thread
From: Alex Williamson @ 2020-11-06 23:29 UTC (permalink / raw)
  To: David Woodhouse, Bonzini, Paolo
  Cc: Cornelia Huck, Alexander Viro, Jens Axboe, kvm, linux-kernel,
	linux-fsdevel

On Tue, 27 Oct 2020 13:55:22 +0000
David Woodhouse <dwmw2@infradead.org> wrote:

> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> Don't allow the events to accumulate in the eventfd counter, drain them
> as they are handled.
> 
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---

Acked-by: Alex Williamson <alex.williamson@redhat.com>

Paolo, I assume you'll add this to your queue.  Thanks,

Alex

>  drivers/vfio/virqfd.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/vfio/virqfd.c b/drivers/vfio/virqfd.c
> index 997cb5d0a657..414e98d82b02 100644
> --- a/drivers/vfio/virqfd.c
> +++ b/drivers/vfio/virqfd.c
> @@ -46,6 +46,9 @@ static int virqfd_wakeup(wait_queue_entry_t *wait, unsigned mode, int sync, void
>  	__poll_t flags = key_to_poll(key);
>  
>  	if (flags & EPOLLIN) {
> +		u64 cnt;
> +		eventfd_ctx_do_read(virqfd->eventfd, &cnt);
> +
>  		/* An event has been signaled, call function */
>  		if ((!virqfd->handler ||
>  		     virqfd->handler(virqfd->opaque, virqfd->data)) &&


^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] vfio/virqfd: Drain events from eventfd in virqfd_wakeup()
  2020-11-06 23:29     ` Alex Williamson
@ 2020-11-08  9:17       ` Paolo Bonzini
  0 siblings, 0 replies; 9+ messages in thread
From: Paolo Bonzini @ 2020-11-08  9:17 UTC (permalink / raw)
  To: Alex Williamson, David Woodhouse
  Cc: Cornelia Huck, Alexander Viro, Jens Axboe, kvm, linux-kernel,
	linux-fsdevel

On 07/11/20 00:29, Alex Williamson wrote:
>> From: David Woodhouse<dwmw@amazon.co.uk>
>>
>> Don't allow the events to accumulate in the eventfd counter, drain them
>> as they are handled.
>>
>> Signed-off-by: David Woodhouse<dwmw@amazon.co.uk>
>> ---
> Acked-by: Alex Williamson<alex.williamson@redhat.com>
> 
> Paolo, I assume you'll add this to your queue.  Thanks,
> 
> Alex
> 

Yes, thanks.

Paolo


^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-11-08  9:17 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1faa5405-3640-f4ad-5cd9-89a9e5e834e9@redhat.com>
2020-10-27 13:55 ` [PATCH 0/3] Allow in-kernel consumers to drain events from eventfd David Woodhouse
2020-10-27 13:55   ` [PATCH 1/3] eventfd: Export eventfd_ctx_do_read() David Woodhouse
2020-10-27 13:55   ` [PATCH 2/3] vfio/virqfd: Drain events from eventfd in virqfd_wakeup() David Woodhouse
2020-11-06 23:29     ` Alex Williamson
2020-11-08  9:17       ` Paolo Bonzini
2020-10-27 13:55   ` [PATCH 3/3] kvm/eventfd: Drain events from eventfd in irqfd_wakeup() David Woodhouse
2020-10-27 18:41     ` kernel test robot
2020-10-27 21:42     ` kernel test robot
2020-10-27 23:13     ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).