diff mbox series

[1/3] caif_virtio: remove virtqueue_disable_cb() in probe

Message ID 20220620051115.3142-2-jasowang@redhat.com (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series Fixing races in probe/remove | expand

Checks

Context Check Description
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 2 maintainers not CCed: pabeni@redhat.com edumazet@google.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 9 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Jason Wang June 20, 2022, 5:11 a.m. UTC
This disabling is a just a hint with best effort, there's no guarantee
that device doesn't send notification. The driver should survive with
that, so let's remove it.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/caif/caif_virtio.c | 3 ---
 1 file changed, 3 deletions(-)

Comments

Michael S. Tsirkin June 20, 2022, 9:02 a.m. UTC | #1
On Mon, Jun 20, 2022 at 01:11:13PM +0800, Jason Wang wrote:
> This disabling is a just a hint with best effort, there's no guarantee
> that device doesn't send notification. The driver should survive with
> that, so let's remove it.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>

I guess, but frankly this change feels gratituous, and just might
uncover latent bugs. Which would be fine if we were out to
find and fix them, but given this was compile-tested only,
I'm not sure that's the case.


> ---
>  drivers/net/caif/caif_virtio.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/net/caif/caif_virtio.c b/drivers/net/caif/caif_virtio.c
> index 5458f57177a0..c677ded81133 100644
> --- a/drivers/net/caif/caif_virtio.c
> +++ b/drivers/net/caif/caif_virtio.c
> @@ -705,9 +705,6 @@ static int cfv_probe(struct virtio_device *vdev)
>  	netdev->needed_headroom = cfv->tx_hr;
>  	netdev->needed_tailroom = cfv->tx_tr;
>  
> -	/* Disable buffer release interrupts unless we have stopped TX queues */
> -	virtqueue_disable_cb(cfv->vq_tx);
> -
>  	netdev->mtu = cfv->mtu - cfv->tx_tr;
>  	vdev->priv = cfv;
>  
> -- 
> 2.25.1
Jason Wang June 21, 2022, 3:10 a.m. UTC | #2
On Mon, Jun 20, 2022 at 5:02 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, Jun 20, 2022 at 01:11:13PM +0800, Jason Wang wrote:
> > This disabling is a just a hint with best effort, there's no guarantee
> > that device doesn't send notification. The driver should survive with
> > that, so let's remove it.
> >
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
>
> I guess, but frankly this change feels gratituous, and just might
> uncover latent bugs. Which would be fine if we were out to
> find and fix them, but given this was compile-tested only,
> I'm not sure that's the case.

Ok, let me drop this from the next version (no way to test).

Thanks

>
>
> > ---
> >  drivers/net/caif/caif_virtio.c | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/drivers/net/caif/caif_virtio.c b/drivers/net/caif/caif_virtio.c
> > index 5458f57177a0..c677ded81133 100644
> > --- a/drivers/net/caif/caif_virtio.c
> > +++ b/drivers/net/caif/caif_virtio.c
> > @@ -705,9 +705,6 @@ static int cfv_probe(struct virtio_device *vdev)
> >       netdev->needed_headroom = cfv->tx_hr;
> >       netdev->needed_tailroom = cfv->tx_tr;
> >
> > -     /* Disable buffer release interrupts unless we have stopped TX queues */
> > -     virtqueue_disable_cb(cfv->vq_tx);
> > -
> >       netdev->mtu = cfv->mtu - cfv->tx_tr;
> >       vdev->priv = cfv;
> >
> > --
> > 2.25.1
>
diff mbox series

Patch

diff --git a/drivers/net/caif/caif_virtio.c b/drivers/net/caif/caif_virtio.c
index 5458f57177a0..c677ded81133 100644
--- a/drivers/net/caif/caif_virtio.c
+++ b/drivers/net/caif/caif_virtio.c
@@ -705,9 +705,6 @@  static int cfv_probe(struct virtio_device *vdev)
 	netdev->needed_headroom = cfv->tx_hr;
 	netdev->needed_tailroom = cfv->tx_tr;
 
-	/* Disable buffer release interrupts unless we have stopped TX queues */
-	virtqueue_disable_cb(cfv->vq_tx);
-
 	netdev->mtu = cfv->mtu - cfv->tx_tr;
 	vdev->priv = cfv;