Re: [PATCH 10/13] target_core_user: add backend plug/unplug callouts

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09.02.21 13:38, Mike Christie wrote:
> This patch adds plug/unplug callouts for tcmu, so we can avoid the
> number of times we switch to userspace. Using this driver with tcm
> loop is a common config, and dependng on the nr_hw_queues
> (nr_hw_queues=1 performs much better) and fio jobs (lower num jobs
> around 4) this patch can increase IOPs by only around 5-10% because
> we hit other issues like the big per tcmu device mutex.> 
> Signed-off-by: Mike Christie <michael.christie@xxxxxxxxxx>
> ---
>   drivers/target/target_core_user.c | 27 +++++++++++++++++++++++++--
>   1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
> index a5991df23581..a030ca6f0f4c 100644
> --- a/drivers/target/target_core_user.c
> +++ b/drivers/target/target_core_user.c
> @@ -111,6 +111,7 @@ struct tcmu_dev {
>   	struct kref kref;
>   
>   	struct se_device se_dev;
> +	struct se_dev_plug se_plug;
>   
>   	char *name;
>   	struct se_hba *hba;
> @@ -119,6 +120,7 @@ struct tcmu_dev {
>   #define TCMU_DEV_BIT_BROKEN 1
>   #define TCMU_DEV_BIT_BLOCKED 2
>   #define TCMU_DEV_BIT_TMR_NOTIFY 3
> +#define TCM_DEV_BIT_PLUGGED 4
>   	unsigned long flags;
>   
>   	struct uio_info uio_info;
> @@ -959,6 +961,25 @@ static uint32_t ring_insert_padding(struct tcmu_dev *udev, size_t cmd_size)
>   	return cmd_head;
>   }
>   
> +static void tcmu_unplug_device(struct se_dev_plug *se_plug)
> +{
> +	struct se_device *se_dev = se_plug->se_dev;
> +	struct tcmu_dev *udev = TCMU_DEV(se_dev);
> +
> +	uio_event_notify(&udev->uio_info);

Don't we have a race here?

Let's assume that
 - just here the thread is interrupted
 - userspace starts,empties the ring and sleeps again
 - another cpu queues a new CDB in the ring
In that - of course very rare condition - userspace will not wake up for the freshly queued CDB.

I think, first clearing the bit, then doing the uio_event_notify would work (without need to take the big tcmu mutex).

> +	clear_bit(TCM_DEV_BIT_PLUGGED, &udev->flags);
> +}
> +
> +static struct se_dev_plug *tcmu_plug_device(struct se_device *se_dev)
> +{
> +	struct tcmu_dev *udev = TCMU_DEV(se_dev);
> +
> +	if (!test_and_set_bit(TCM_DEV_BIT_PLUGGED, &udev->flags))
> +		return &udev->se_plug;
> +
> +	return NULL;
> +}
> +
>   /**
>    * queue_cmd_ring - queue cmd to ring or internally
>    * @tcmu_cmd: cmd to queue
> @@ -1086,8 +1107,8 @@ static int queue_cmd_ring(struct tcmu_cmd *tcmu_cmd, sense_reason_t *scsi_err)
>   
>   	list_add_tail(&tcmu_cmd->queue_entry, &udev->inflight_queue);
>   
> -	/* TODO: only if FLUSH and FUA? */
> -	uio_event_notify(&udev->uio_info);
> +	if (!test_bit(TCM_DEV_BIT_PLUGGED, &udev->flags))
> +		uio_event_notify(&udev->uio_info);
>   
>   	return 0;
>   
> @@ -2840,6 +2861,8 @@ static struct target_backend_ops tcmu_ops = {
>   	.configure_device	= tcmu_configure_device,
>   	.destroy_device		= tcmu_destroy_device,
>   	.free_device		= tcmu_free_device,
> +	.unplug_device		= tcmu_unplug_device,
> +	.plug_device		= tcmu_plug_device,
>   	.parse_cdb		= tcmu_parse_cdb,
>   	.tmr_notify		= tcmu_tmr_notify,
>   	.set_configfs_dev_params = tcmu_set_configfs_dev_params,
> 



[Index of Archives]     [Linux SCSI]     [Kernel Newbies]     [Linux SCSI Target Infrastructure]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Device Mapper]

  Powered by Linux