> -----Original Message----- > From: devel [mailto:driverdev-devel-bounces@xxxxxxxxxxxxxxxxxxxxxx] On Behalf > Of K. Y. Srinivasan > Sent: Friday, October 30, 2015 9:13 > To: gregkh@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > devel@xxxxxxxxxxxxxxxxxxxxxx; olaf@xxxxxxxxx; apw@xxxxxxxxxxxxx; > vkuznets@xxxxxxxxxx; jasowang@xxxxxxxxxx > Subject: [PATCH V2 02/10] Drivers: hv: utils: run polling callback always in > interrupt context > > From: Olaf Hering <olaf@xxxxxxxxx> > > All channel interrupts are bound to specific VCPUs in the guest > at the point channel is created. While currently, we invoke the > polling function on the correct CPU (the CPU to which the channel > is bound to) in some cases we may run the polling function in > a non-interrupt context. This potentially can cause an issue as the > polling function can be interrupted by the channel callback function. > Fix the issue by running the polling function on the appropriate CPU > at interrupt level. Additional details of the issue being addressed by > this patch are given below: > > Currently hv_fcopy_onchannelcallback is called from interrupts and also > via the ->write function of hv_utils. Since the used global variables to > maintain state are not thread safe the state can get out of sync. > This affects the variable state as well as the channel inbound buffer. > > As suggested by KY adjust hv_poll_channel to always run the given > callback on the cpu which the channel is bound to. This avoids the need > for locking because all the util services are single threaded and only > one transaction is active at any given point in time. > > Additionally, remove the context variable, they will always be the same as > recv_channel. > > Signed-off-by: Olaf Hering <olaf@xxxxxxxxx> > Signed-off-by: K. Y. Srinivasan <kys@xxxxxxxxxxxxx> > --- > V2: Added the check to catch unsolicited daemon writes - Vitaly > > drivers/hv/hv_fcopy.c | 34 +++++++++++++--------------------- > drivers/hv/hv_kvp.c | 28 ++++++++++------------------ > drivers/hv/hv_snapshot.c | 29 +++++++++++------------------ > drivers/hv/hyperv_vmbus.h | 6 +----- > 4 files changed, 35 insertions(+), 62 deletions(-) (Sorry for not joining the discussion when the patch was firstly made) It looks the patch has not been Greg's tree yet. I have 2 questions about the patch: 1. hv_poll_channel() is invoked in fcopy_handle_handshake(), but not in vss_handle_handshake() and kvp_handle_handshake(). Why -- I guess we missed the vss/kvp cases somehow? 2. With the patch, hv_fcopy_onchannelcallback() can be invoked in the tasklet (i.e., vmbus_on_event(). NB: local irq is enabled), and in the hard irq handler(the IPI handler, e.g., fcopy_poll_wrapper() -> fcopy_poll_wrapper()). Can the former be interrupted by the latter? e.g., when the callback is running in the tasklet on vCPU0, fcopy_timeout_func() or fcopy_on_msg() could send the IPI to vCPU0 from another vCPU. Thanks, -- Dexuan _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
