| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20151211090229.GE9903@c203.arch.suse.de>
Date: Fri, 11 Dec 2015 10:02:29 +0100
From: Johannes Thumshirn <jthumshirn@...e.de>
To: "K. Y. Srinivasan" <kys@...rosoft.com>
Cc: gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
devel@...uxdriverproject.org, ohering@...e.com,
jbottomley@...allels.com, hch@...radead.org,
linux-scsi@...r.kernel.org, apw@...onical.com, vkuznets@...hat.com,
jasowang@...hat.com, martin.petersen@...cle.com
Subject: Re: [PATCH 3/4] scsi: storvsc: Refactor the code in
storvsc_channel_init()
On Thu, Dec 10, 2015 at 04:14:19PM -0800, K. Y. Srinivasan wrote:
> The function storvsc_channel_init() repeatedly interacts with the host to
> extract various channel properties. Refactor this code to eliminate code
> repetition.
>
> Signed-off-by: K. Y. Srinivasan <kys@...rosoft.com>
> Reviewed-by: Long Li <longli@...rosoft.com>
> Tested-by: Alex Ng <alexng@...rosoft.com>
> ---
> drivers/scsi/storvsc_drv.c | 155 ++++++++++++++++----------------------------
> 1 files changed, 57 insertions(+), 98 deletions(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index b94d471..6f18e94 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -721,29 +721,16 @@ static void cache_wwn(struct storvsc_device *stor_device,
> }
> }
>
> -static int storvsc_channel_init(struct hv_device *device, bool is_fc)
> +static int storvsc_execute_vstor_op(struct hv_device *device,
> + struct storvsc_cmd_request *request,
> + bool status_check)
> {
> - struct storvsc_device *stor_device;
> - struct storvsc_cmd_request *request;
> struct vstor_packet *vstor_packet;
> - int ret, t, i;
> - int max_chns;
> - bool process_sub_channels = false;
> -
> - stor_device = get_out_stor_device(device);
> - if (!stor_device)
> - return -ENODEV;
> + int ret, t;
>
> - request = &stor_device->init_request;
> vstor_packet = &request->vstor_packet;
>
> - /*
> - * Now, initiate the vsc/vsp initialization protocol on the open
> - * channel
> - */
> - memset(request, 0, sizeof(struct storvsc_cmd_request));
> init_completion(&request->wait_event);
> - vstor_packet->operation = VSTOR_OPERATION_BEGIN_INITIALIZATION;
> vstor_packet->flags = REQUEST_COMPLETION_FLAG;
>
> ret = vmbus_sendpacket(device->channel, vstor_packet,
> @@ -753,27 +740,62 @@ static int storvsc_channel_init(struct hv_device *device, bool is_fc)
> VM_PKT_DATA_INBAND,
> VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
> if (ret != 0)
> - goto cleanup;
> + goto done;
>
> t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
> if (t == 0) {
> ret = -ETIMEDOUT;
> - goto cleanup;
> + goto done;
> }
>
> + if (!status_check)
> + goto done;
> +
> if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
> vstor_packet->status != 0) {
> ret = -EINVAL;
> - goto cleanup;
> + goto done;
> }
>
> +done:
> + return ret;
> +}
> +
> +static int storvsc_channel_init(struct hv_device *device, bool is_fc)
> +{
> + struct storvsc_device *stor_device;
> + struct storvsc_cmd_request *request;
> + struct vstor_packet *vstor_packet;
> + int ret, i;
> + int max_chns;
> + bool process_sub_channels = false;
> +
> + stor_device = get_out_stor_device(device);
> + if (!stor_device)
> + return -ENODEV;
> +
> + request = &stor_device->init_request;
> + vstor_packet = &request->vstor_packet;
> +
> + /*
> + * Now, initiate the vsc/vsp initialization protocol on the open
> + * channel
> + */
> + memset(request, 0, sizeof(struct storvsc_cmd_request));
> + vstor_packet->operation = VSTOR_OPERATION_BEGIN_INITIALIZATION;
> + ret = storvsc_execute_vstor_op(device, request, true);
> + if (ret)
> + goto cleanup;
> +
> + /*
> + * Query host supported protocol version.
> + */
>
> for (i = 0; i < ARRAY_SIZE(vmstor_protocols); i++) {
> /* reuse the packet for version range supported */
> memset(vstor_packet, 0, sizeof(struct vstor_packet));
> vstor_packet->operation =
> VSTOR_OPERATION_QUERY_PROTOCOL_VERSION;
> - vstor_packet->flags = REQUEST_COMPLETION_FLAG;
>
> vstor_packet->version.major_minor =
> vmstor_protocols[i].protocol_version;
> @@ -783,20 +805,9 @@ static int storvsc_channel_init(struct hv_device *device, bool is_fc)
> */
> vstor_packet->version.revision = 0;
>
> - ret = vmbus_sendpacket(device->channel, vstor_packet,
> - (sizeof(struct vstor_packet) -
> - vmscsi_size_delta),
> - (unsigned long)request,
> - VM_PKT_DATA_INBAND,
> - VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
> - if (ret != 0)
> - goto cleanup;
> -
> - t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
> - if (t == 0) {
> - ret = -ETIMEDOUT;
> + ret = storvsc_execute_vstor_op(device, request, false);
> + if (ret)
> goto cleanup;
> - }
>
> if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO) {
> ret = -EINVAL;
> @@ -822,32 +833,14 @@ static int storvsc_channel_init(struct hv_device *device, bool is_fc)
> goto cleanup;
> }
>
> -
> + /*
> + * Query channel properties.
> + */
> memset(vstor_packet, 0, sizeof(struct vstor_packet));
> vstor_packet->operation = VSTOR_OPERATION_QUERY_PROPERTIES;
> - vstor_packet->flags = REQUEST_COMPLETION_FLAG;
> -
> - ret = vmbus_sendpacket(device->channel, vstor_packet,
> - (sizeof(struct vstor_packet) -
> - vmscsi_size_delta),
> - (unsigned long)request,
> - VM_PKT_DATA_INBAND,
> - VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
> -
> - if (ret != 0)
> - goto cleanup;
> -
> - t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
> - if (t == 0) {
> - ret = -ETIMEDOUT;
> - goto cleanup;
> - }
> -
> - if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
> - vstor_packet->status != 0) {
> - ret = -EINVAL;
> + ret = storvsc_execute_vstor_op(device, request, true);
> + if (ret)
> goto cleanup;
> - }
>
> /*
> * Check to see if multi-channel support is there.
> @@ -866,30 +859,15 @@ static int storvsc_channel_init(struct hv_device *device, bool is_fc)
> if (!is_fc)
> goto done;
>
> + /*
> + * For FC devices retrieve FC HBA data.
> + */
> memset(vstor_packet, 0, sizeof(struct vstor_packet));
> vstor_packet->operation = VSTOR_OPERATION_FCHBA_DATA;
> - vstor_packet->flags = REQUEST_COMPLETION_FLAG;
>
> - ret = vmbus_sendpacket(device->channel, vstor_packet,
> - (sizeof(struct vstor_packet) -
> - vmscsi_size_delta),
> - (unsigned long)request,
> - VM_PKT_DATA_INBAND,
> - VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
> -
> - if (ret != 0)
> - goto cleanup;
> -
> - t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
> - if (t == 0) {
> - ret = -ETIMEDOUT;
> - goto cleanup;
> - }
> -
> - if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
> - vstor_packet->status != 0)
> + ret = storvsc_execute_vstor_op(device, request, true);
> + if (ret)
> goto cleanup;
> -
> /*
> * Cache the currently active port and node ww names.
> */
> @@ -899,29 +877,10 @@ done:
>
> memset(vstor_packet, 0, sizeof(struct vstor_packet));
> vstor_packet->operation = VSTOR_OPERATION_END_INITIALIZATION;
> - vstor_packet->flags = REQUEST_COMPLETION_FLAG;
>
> - ret = vmbus_sendpacket(device->channel, vstor_packet,
> - (sizeof(struct vstor_packet) -
> - vmscsi_size_delta),
> - (unsigned long)request,
> - VM_PKT_DATA_INBAND,
> - VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
> -
> - if (ret != 0)
> - goto cleanup;
> -
> - t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
> - if (t == 0) {
> - ret = -ETIMEDOUT;
> - goto cleanup;
> - }
> -
> - if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
> - vstor_packet->status != 0) {
> - ret = -EINVAL;
> + ret = storvsc_execute_vstor_op(device, request, true);
> + if (ret)
> goto cleanup;
> - }
>
> if (process_sub_channels)
> handle_multichannel_storage(device, max_chns);
> --
> 1.7.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Reviewed-by: Johannes Thumshirn <jthumshirn@...e.de>
--
Johannes Thumshirn Storage
jthumshirn@...e.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists