| Message ID | 20231122-vfs-eventfd-signal-v2-1-bd549b14ce0c@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | eventfd: simplify signal helpers | expand |
On Wed 22-11-23 13:48:22, Christian Brauner wrote: > The single caller of inject_virtual_interrupt() ignores the return value > anyway. This allows us to simplify eventfd_signal() in follow-up > patches. > > Signed-off-by: Christian Brauner <brauner@kernel.org> Looks good. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > drivers/gpu/drm/i915/gvt/interrupt.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gvt/interrupt.c b/drivers/gpu/drm/i915/gvt/interrupt.c > index de3f5903d1a7..9665876b4b13 100644 > --- a/drivers/gpu/drm/i915/gvt/interrupt.c > +++ b/drivers/gpu/drm/i915/gvt/interrupt.c > @@ -422,7 +422,7 @@ static void init_irq_map(struct intel_gvt_irq *irq) > #define MSI_CAP_DATA(offset) (offset + 8) > #define MSI_CAP_EN 0x1 > > -static int inject_virtual_interrupt(struct intel_vgpu *vgpu) > +static void inject_virtual_interrupt(struct intel_vgpu *vgpu) > { > unsigned long offset = vgpu->gvt->device_info.msi_cap_offset; > u16 control, data; > @@ -434,10 +434,10 @@ static int inject_virtual_interrupt(struct intel_vgpu *vgpu) > > /* Do not generate MSI if MSIEN is disabled */ > if (!(control & MSI_CAP_EN)) > - return 0; > + return; > > if (WARN(control & GENMASK(15, 1), "only support one MSI format\n")) > - return -EINVAL; > + return; > > trace_inject_msi(vgpu->id, addr, data); > > @@ -451,10 +451,10 @@ static int inject_virtual_interrupt(struct intel_vgpu *vgpu) > * returned and don't inject interrupt into guest. > */ > if (!test_bit(INTEL_VGPU_STATUS_ATTACHED, vgpu->status)) > - return -ESRCH; > - if (vgpu->msi_trigger && eventfd_signal(vgpu->msi_trigger, 1) != 1) > - return -EFAULT; > - return 0; > + return; > + if (!vgpu->msi_trigger) > + return; > + eventfd_signal(vgpu->msi_trigger, 1); > } > > static void propagate_event(struct intel_gvt_irq *irq, > > -- > 2.42.0 >
On 2023.11.22 13:48:22 +0100, Christian Brauner wrote: > The single caller of inject_virtual_interrupt() ignores the return value > anyway. This allows us to simplify eventfd_signal() in follow-up > patches. > > Signed-off-by: Christian Brauner <brauner@kernel.org> > --- > drivers/gpu/drm/i915/gvt/interrupt.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gvt/interrupt.c b/drivers/gpu/drm/i915/gvt/interrupt.c > index de3f5903d1a7..9665876b4b13 100644 > --- a/drivers/gpu/drm/i915/gvt/interrupt.c > +++ b/drivers/gpu/drm/i915/gvt/interrupt.c > @@ -422,7 +422,7 @@ static void init_irq_map(struct intel_gvt_irq *irq) > #define MSI_CAP_DATA(offset) (offset + 8) > #define MSI_CAP_EN 0x1 > > -static int inject_virtual_interrupt(struct intel_vgpu *vgpu) > +static void inject_virtual_interrupt(struct intel_vgpu *vgpu) > { > unsigned long offset = vgpu->gvt->device_info.msi_cap_offset; > u16 control, data; > @@ -434,10 +434,10 @@ static int inject_virtual_interrupt(struct intel_vgpu *vgpu) > > /* Do not generate MSI if MSIEN is disabled */ > if (!(control & MSI_CAP_EN)) > - return 0; > + return; > > if (WARN(control & GENMASK(15, 1), "only support one MSI format\n")) > - return -EINVAL; > + return; > > trace_inject_msi(vgpu->id, addr, data); > > @@ -451,10 +451,10 @@ static int inject_virtual_interrupt(struct intel_vgpu *vgpu) > * returned and don't inject interrupt into guest. > */ > if (!test_bit(INTEL_VGPU_STATUS_ATTACHED, vgpu->status)) > - return -ESRCH; > - if (vgpu->msi_trigger && eventfd_signal(vgpu->msi_trigger, 1) != 1) > - return -EFAULT; > - return 0; > + return; > + if (!vgpu->msi_trigger) > + return; > + eventfd_signal(vgpu->msi_trigger, 1); > } I think it's a little simpler to write as if (vgpu->msi_trigger) eventfd_signal(vgpu->msi_trigger, 1); Looks fine with me. Reviewed-by: Zhenyu Wang <zhenyuw@linux.intel.com> Thanks! > > static void propagate_event(struct intel_gvt_irq *irq, > > -- > 2.42.0 >
> > + if (!vgpu->msi_trigger) > > + return; > > + eventfd_signal(vgpu->msi_trigger, 1); > > } > > I think it's a little simpler to write as > if (vgpu->msi_trigger) > eventfd_signal(vgpu->msi_trigger, 1); Good point. I folded that suggestion into the patch.
diff --git a/drivers/gpu/drm/i915/gvt/interrupt.c b/drivers/gpu/drm/i915/gvt/interrupt.c index de3f5903d1a7..9665876b4b13 100644 --- a/drivers/gpu/drm/i915/gvt/interrupt.c +++ b/drivers/gpu/drm/i915/gvt/interrupt.c @@ -422,7 +422,7 @@ static void init_irq_map(struct intel_gvt_irq *irq) #define MSI_CAP_DATA(offset) (offset + 8) #define MSI_CAP_EN 0x1 -static int inject_virtual_interrupt(struct intel_vgpu *vgpu) +static void inject_virtual_interrupt(struct intel_vgpu *vgpu) { unsigned long offset = vgpu->gvt->device_info.msi_cap_offset; u16 control, data; @@ -434,10 +434,10 @@ static int inject_virtual_interrupt(struct intel_vgpu *vgpu) /* Do not generate MSI if MSIEN is disabled */ if (!(control & MSI_CAP_EN)) - return 0; + return; if (WARN(control & GENMASK(15, 1), "only support one MSI format\n")) - return -EINVAL; + return; trace_inject_msi(vgpu->id, addr, data); @@ -451,10 +451,10 @@ static int inject_virtual_interrupt(struct intel_vgpu *vgpu) * returned and don't inject interrupt into guest. */ if (!test_bit(INTEL_VGPU_STATUS_ATTACHED, vgpu->status)) - return -ESRCH; - if (vgpu->msi_trigger && eventfd_signal(vgpu->msi_trigger, 1) != 1) - return -EFAULT; - return 0; + return; + if (!vgpu->msi_trigger) + return; + eventfd_signal(vgpu->msi_trigger, 1); } static void propagate_event(struct intel_gvt_irq *irq,
The single caller of inject_virtual_interrupt() ignores the return value anyway. This allows us to simplify eventfd_signal() in follow-up patches. Signed-off-by: Christian Brauner <brauner@kernel.org> --- drivers/gpu/drm/i915/gvt/interrupt.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)