All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Christie <michael.christie@oracle.com>
To: target-devel@vger.kernel.org, linux-scsi@vger.kernel.org,
	stefanha@redhat.com, jasowang@redhat.com, mst@redhat.com,
	sgarzare@redhat.com, virtualization@lists.linux-foundation.org
Subject: Re: [PATCH 3/5] vhost-scsi: Remove vhost_scsi_mutex from port link/unlink
Date: Sun, 19 Mar 2023 10:48:24 -0500	[thread overview]
Message-ID: <b6606537-5200-041b-3157-2e81ef6ac3be@oracle.com> (raw)
In-Reply-To: <20230223001949.2884-4-michael.christie@oracle.com>

On 2/22/23 6:19 PM, Mike Christie wrote:
> We don't need the vhost_scsi_mutex in vhost_scsi_port_link and
> vhost_scsi_port_unlink because LIO has a refcount on the se_tpg for us,
> so it can't be removed while these functions are called. This removes the
> vhost_scsi_mutex from those functions to avoid cases where we are adding
> or removing LUNs to vhost-deviceA but are stuck waiting on the
> vhost_scsi_mutex because we are running vhost_scsi_clear_endpoint on
> vhost-deviceB and it's stuck in vhost_scsi_flush waiting for a flakey
> physical device.

I'm going to self Nack these patches. This patch has a bug where we can
leak vhost_scsi_tmf structs. It's ok to drop the vhost_scsi_mutex use but
in vhost_scsi_port_unlink it needs to be replaced with the device->mutex
or we need to flush the tmf/VHOST_SCSI_VQ_CTL queue to make sure outstanding
tmfs are freed.


WARNING: multiple messages have this Message-ID (diff)
From: Mike Christie <michael.christie@oracle.com>
To: target-devel@vger.kernel.org, linux-scsi@vger.kernel.org,
	stefanha@redhat.com, jasowang@redhat.com, mst@redhat.com,
	sgarzare@redhat.com, virtualization@lists.linux-foundation.org
Subject: Re: [PATCH 3/5] vhost-scsi: Remove vhost_scsi_mutex from port link/unlink
Date: Sun, 19 Mar 2023 10:48:24 -0500	[thread overview]
Message-ID: <b6606537-5200-041b-3157-2e81ef6ac3be@oracle.com> (raw)
In-Reply-To: <20230223001949.2884-4-michael.christie@oracle.com>

On 2/22/23 6:19 PM, Mike Christie wrote:
> We don't need the vhost_scsi_mutex in vhost_scsi_port_link and
> vhost_scsi_port_unlink because LIO has a refcount on the se_tpg for us,
> so it can't be removed while these functions are called. This removes the
> vhost_scsi_mutex from those functions to avoid cases where we are adding
> or removing LUNs to vhost-deviceA but are stuck waiting on the
> vhost_scsi_mutex because we are running vhost_scsi_clear_endpoint on
> vhost-deviceB and it's stuck in vhost_scsi_flush waiting for a flakey
> physical device.

I'm going to self Nack these patches. This patch has a bug where we can
leak vhost_scsi_tmf structs. It's ok to drop the vhost_scsi_mutex use but
in vhost_scsi_port_unlink it needs to be replaced with the device->mutex
or we need to flush the tmf/VHOST_SCSI_VQ_CTL queue to make sure outstanding
tmfs are freed.

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

  reply	other threads:[~2023-03-19 15:48 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-23  0:19 [PATCH 0/5] vhost-scsi: Fix management operation hangs Mike Christie
2023-02-23  0:19 ` Mike Christie
2023-02-23  0:19 ` [PATCH 1/5] vhost-scsi: Hold tv_tpg_mutex when decrementing tv_tpg_vhost_count Mike Christie
2023-02-23  0:19   ` Mike Christie
2023-02-23  0:19 ` [PATCH 2/5] vhost-scsi: Drop vhost_scsi_flush during endpoint clearing Mike Christie
2023-02-23  0:19   ` Mike Christie
2023-02-23  0:19 ` [PATCH 3/5] vhost-scsi: Remove vhost_scsi_mutex from port link/unlink Mike Christie
2023-02-23  0:19   ` Mike Christie
2023-03-19 15:48   ` Mike Christie [this message]
2023-03-19 15:48     ` Mike Christie
2023-02-23  0:19 ` [PATCH 4/5] vhost-scsi: Delay releasing our refcount on the tpg Mike Christie
2023-02-23  0:19   ` Mike Christie
2023-02-23  0:19 ` [PATCH 5/5] vhost-scsi: Reduce vhost_scsi_mutex use Mike Christie
2023-02-23  0:19   ` Mike Christie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b6606537-5200-041b-3157-2e81ef6ac3be@oracle.com \
    --to=michael.christie@oracle.com \
    --cc=jasowang@redhat.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=sgarzare@redhat.com \
    --cc=stefanha@redhat.com \
    --cc=target-devel@vger.kernel.org \
    --cc=virtualization@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.