Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [smartpqi updates PATCH V2 01/11] smartpqi: update device removal
 management
To:     Don Brace <don.brace@microchip.com>, hch@infradead.org,
        martin.petersen@oracle.com, jejb@linux.vnet.ibm.com,
        linux-scsi@vger.kernel.org
Cc:     Kevin.Barnett@microchip.com, scott.teel@microchip.com,
        Justin.Lindley@microchip.com, scott.benesh@microchip.com,
        gerry.morong@microchip.com, mahesh.rajashekhara@microchip.com,
        mike.mcgowen@microchip.com, murthy.bhat@microchip.com,
        balsundar.p@microchip.com, joseph.szczypek@hpe.com,
        jeff@canonical.com, POSWALD@suse.com, mwilck@suse.com,
        pmenzel@molgen.mpg.de, linux-kernel@vger.kernel.org
References: <20210928235442.201875-1-don.brace@microchip.com>
 <20210928235442.201875-2-don.brace@microchip.com>
From:   john.p.donnelly@oracle.com
Message-ID: <b3d91ad4-b3ea-d189-1606-d565166a4fe8@oracle.com>
Date:   Thu, 30 Sep 2021 13:21:26 -0500
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0)
 Gecko/20100101 Thunderbird/78.14.0
In-Reply-To: <20210928235442.201875-2-don.brace@microchip.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?N2pITDNST3NlNWU3NU5DZFF1RG1qbTBGMVNWVjlVVXNEQVQvcVptMUVPQlZU?=
 =?utf-8?B?V3Q2bWQva2dva2MyWXlsMjR4TkE5TUVRTjc4TlJWMEZIREVhRGtGY0s3WmI3?=
 =?utf-8?B?RzRKYWd6eFBLaHgyanljVEhJUStlTjNzMGdUcHA0YTVsWnpRRmJhNUFRNjlN?=
 =?utf-8?B?Wi96ZUNhOFBBSkMrdk9ETHdlM3R4d1ZyWWZDL1NpQ0h4bDkyaFpiV1hJbHVr?=
 =?utf-8?B?R2xDclpHL0JCWUJqOW9PY2UrV2E2cEZHQUo1MFBtbkRzdlEvTytsSVJWbFQy?=
 =?utf-8?B?Y1Bna3poN1NkVFIrWmpJVHJqZmVvTTRpb3VVeGlXV3RaQmJoUTZyTk56aHNV?=
 =?utf-8?B?Rk1wbWRVdHY5dkwzZUVBdzlXVHV1ZCs3OWNQajRYQWVqcTJvRE91YkZzQ3VB?=
 =?utf-8?B?Qm5TMlZNT3V6NlRnbUEzeFRtRmR1by9HUVdEenR1YkgrYnJiS2hOR1RhenF0?=
 =?utf-8?B?eUNuSmpLZkpGVVU4aytndFZaek5iQ0VKbW1nbVpjckpDdHBlZW03MW5VcllX?=
 =?utf-8?B?NVI0Qlk2dm8yOFBYWmMxWFF6NVpSL1RHV3VTWmRNWjNKc2xLbGJGRENtMXlu?=
 =?utf-8?B?RXpieUZGcVhIZ2g2Tmd6Q3hpK2JaQVl4Z0VGbm5KbkNDT2dzcmp4b0Nqekl3?=
 =?utf-8?B?SnJnNTdxdnJGTzV1VzQ5b3d3V0xxT1E3TUZzU0RlVFBwNHN0Skw3ZWlGazM0?=
 =?utf-8?B?U2pCR0pDeXFPQzdvZ0R6QUxRcTJ6c0NNL3AxNURaZENpcXhVZ0NMd29NaDRa?=
 =?utf-8?B?ZzhoaTRsR25HTWhTbUd6UDhXa1huSGl1TVd6VlJMTThmVnU5amFwd3NNc0p4?=
 =?utf-8?B?WXQ5K1JCdXZtTkUrWC9jUUNSWnoxVU9FWVlyL3VlTE01Tjh6OGg3dFdoWERm?=
 =?utf-8?B?c0NETGRpaEJOdnJPdk9IdWg4NUF0WUxLZnBIVXZDc3RweS93QWhPS2xvWk9W?=
 =?utf-8?B?N01ScGRremo2SWdwWkltNUptbWNPbUFPdWh4MVVvTER0VVZXWTN3Y3k4cGZt?=
 =?utf-8?B?ZzVHczYzenc0c1JmbzRhdTlRNmN0ekZLaTJKRVEzSEZXN0kzeEpMNU0rMFZ4?=
 =?utf-8?B?bENrN1F3aDluWXpmUDNraDVmQTEyTFp1RkkreGVxSDZPZVA0UkNaNkNwQ09Q?=
 =?utf-8?B?THBqcGVFOENiYVJvUUtna3ZOaUtGb2JYc0dPWjc3TlNzN3RMNWVxQWszTFp2?=
 =?utf-8?B?cUNPWVk1cGM4d0hVVFA4VHNkY1ovd3Qrd0hidTVwd3hxcURxTTdqcHNIQWQx?=
 =?utf-8?B?cVZGM2lhSEZVRVZIeEdqWGpMYU9Ha2hNWUUzeXJWVXV6R2JraDRNaEdJZjNU?=
 =?utf-8?B?TFNVZkZJbC8wQmMxS2tVU0R6TmFpdTVxOEdGSEhoMElJaVlqRnlDOUVPYjNO?=
 =?utf-8?B?MzJSYzdTMTBFRitqRkNZbDhHVXFZL1dtTUVhQkVYQXprcXI4Q2VlSG1WTHdM?=
 =?utf-8?B?c0hrcWJZdTZqZlNFb3hoeW1vS1lCL2RrQVkrMU0zT05yZmFyR1B5Nk9hZ1NT?=
 =?utf-8?B?a3IwcWpaaXpESGRwTU5mVUR6ZGlndTZNOEwzQmdZTE5RQ1JYU2E3WXQ0a0NS?=
 =?utf-8?B?aHVEREV0Mm1vdnJhT3hMSGZkeDZuVFMzbVdXQS84Q3lwTk9OUS9uSFYrS1FB?=
 =?utf-8?B?RXVaU291Y3h3YWlPR1M4aFZrS0V3Y1YyR3l0ZlZ0bjhCcDB6aGo2TjE0UXNQ?=
 =?utf-8?B?Zlh5eVdMSzlVTlQxeGU0VkliMDJyeVQ4RmRFNGlxZkw0bHNMTkVNV3Q1cXJQ?=
 =?utf-8?Q?qCAzQouv1SR7C0VAPG9txpy+5Ap9TsmWuxJpJBQ?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c08532f6-f43f-4db2-dc23-08d9843f21da
X-MS-Exchange-CrossTenant-AuthSource: CO1PR10MB4722.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Sep 2021 18:21:33.9525
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: wNAvXYLxbMQPIHt9RqagPwFRgbY8uI1qW0VGIcKVltjzyhPSYHtvceit8gNh2Mjlq4eGsT+cp/wWhqWAE0cSOjxU6gbkrFWlVlehxaHJMrc=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR10MB4612
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10123 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 suspectscore=0 spamscore=0
 adultscore=0 bulkscore=0 mlxscore=0 malwarescore=0 mlxlogscore=999
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2109230001
 definitions=main-2109300112
X-Proofpoint-GUID: 3QBy16_KZ--xce0P6KycJh5-BjBKvvRG
X-Proofpoint-ORIG-GUID: 3QBy16_KZ--xce0P6KycJh5-BjBKvvRG
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 9/28/21 6:54 PM, Don Brace wrote:
> Update device removal path to handle issues for:
>    rmmod - Correct stack trace when removing devices.
>    rmmod - Synchronize SCSI cache.
>    Update handling for removing devices using sysfs.
> 
> This patch also aligns the device removal code with
> our out-of-box driver.
> 
> Reviewed-by: Scott Benesh <scott.benesh@microchip.com>
> Reviewed-by: Scott Teel <scott.teel@microchip.com>
> Reviewed-by: Mike McGowen <mike.mcgowen@microchip.com>
> Signed-off-by: Don Brace <don.brace@microchip.com>

Acked-by: John Donnelly <john.p.donnelly@oracle.com>


> ---
>   drivers/scsi/smartpqi/smartpqi_init.c | 64 ++++++++++++---------------
>   1 file changed, 28 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
> index ecb2af3f43ca..97027574eb1f 100644
> --- a/drivers/scsi/smartpqi/smartpqi_init.c
> +++ b/drivers/scsi/smartpqi/smartpqi_init.c
> @@ -1693,8 +1693,6 @@ static inline void pqi_remove_device(struct pqi_ctrl_info *ctrl_info, struct pqi
>   {
>   	int rc;
>   
> -	pqi_device_remove_start(device);
> -
>   	rc = pqi_device_wait_for_pending_io(ctrl_info, device,
>   		PQI_REMOVE_DEVICE_PENDING_IO_TIMEOUT_MSECS);
>   	if (rc)
> @@ -1708,6 +1706,8 @@ static inline void pqi_remove_device(struct pqi_ctrl_info *ctrl_info, struct pqi
>   		scsi_remove_device(device->sdev);
>   	else
>   		pqi_remove_sas_device(device);
> +
> +	pqi_device_remove_start(device);
>   }
>   
>   /* Assumes the SCSI device list lock is held. */
> @@ -1986,7 +1986,7 @@ static void pqi_update_device_list(struct pqi_ctrl_info *ctrl_info,
>   	list_for_each_entry_safe(device, next, &ctrl_info->scsi_device_list,
>   		scsi_device_list_entry) {
>   		if (device->device_gone) {
> -			list_del_init(&device->scsi_device_list_entry);
> +			list_del(&device->scsi_device_list_entry);
>   			list_add_tail(&device->delete_list_entry, &delete_list);
>   		}
>   	}
> @@ -2025,15 +2025,13 @@ static void pqi_update_device_list(struct pqi_ctrl_info *ctrl_info,
>   		if (device->volume_offline) {
>   			pqi_dev_info(ctrl_info, "offline", device);
>   			pqi_show_volume_status(ctrl_info, device);
> -		}
> -		list_del(&device->delete_list_entry);
> -		if (pqi_is_device_added(device)) {
> -			pqi_remove_device(ctrl_info, device);
>   		} else {
> -			if (!device->volume_offline)
> -				pqi_dev_info(ctrl_info, "removed", device);
> -			pqi_free_device(device);
> +			pqi_dev_info(ctrl_info, "removed", device);
>   		}
> +		if (pqi_is_device_added(device))
> +			pqi_remove_device(ctrl_info, device);
> +		list_del(&device->delete_list_entry);
> +		pqi_free_device(device);
>   	}
>   
>   	/*
> @@ -2328,6 +2326,25 @@ static int pqi_update_scsi_devices(struct pqi_ctrl_info *ctrl_info)
>   	return rc;
>   }
>   
> +static void pqi_remove_all_scsi_devices(struct pqi_ctrl_info *ctrl_info)
> +{
> +	unsigned long flags;
> +	struct pqi_scsi_dev *device;
> +	struct pqi_scsi_dev *next;
> +
> +	spin_lock_irqsave(&ctrl_info->scsi_device_list_lock, flags);
> +
> +	list_for_each_entry_safe(device, next, &ctrl_info->scsi_device_list,
> +		scsi_device_list_entry) {
> +		if (pqi_is_device_added(device))
> +			pqi_remove_device(ctrl_info, device);
> +		list_del(&device->scsi_device_list_entry);
> +		pqi_free_device(device);
> +	}
> +
> +	spin_unlock_irqrestore(&ctrl_info->scsi_device_list_lock, flags);
> +}
> +
>   static int pqi_scan_scsi_devices(struct pqi_ctrl_info *ctrl_info)
>   {
>   	int rc;
> @@ -6120,31 +6137,6 @@ static int pqi_slave_configure(struct scsi_device *sdev)
>   	return 0;
>   }
>   
> -static void pqi_slave_destroy(struct scsi_device *sdev)
> -{
> -	unsigned long flags;
> -	struct pqi_scsi_dev *device;
> -	struct pqi_ctrl_info *ctrl_info;
> -
> -	ctrl_info = shost_to_hba(sdev->host);
> -
> -	spin_lock_irqsave(&ctrl_info->scsi_device_list_lock, flags);
> -
> -	device = sdev->hostdata;
> -	if (device) {
> -		sdev->hostdata = NULL;
> -		if (!list_empty(&device->scsi_device_list_entry))
> -			list_del(&device->scsi_device_list_entry);
> -	}
> -
> -	spin_unlock_irqrestore(&ctrl_info->scsi_device_list_lock, flags);
> -
> -	if (device) {
> -		pqi_dev_info(ctrl_info, "removed", device);
> -		pqi_free_device(device);
> -	}
> -}
> -
>   static int pqi_getpciinfo_ioctl(struct pqi_ctrl_info *ctrl_info, void __user *arg)
>   {
>   	struct pci_dev *pci_dev;
> @@ -6938,7 +6930,6 @@ static struct scsi_host_template pqi_driver_template = {
>   	.ioctl = pqi_ioctl,
>   	.slave_alloc = pqi_slave_alloc,
>   	.slave_configure = pqi_slave_configure,
> -	.slave_destroy = pqi_slave_destroy,
>   	.map_queues = pqi_map_queues,
>   	.sdev_attrs = pqi_sdev_attrs,
>   	.shost_attrs = pqi_shost_attrs,
> @@ -8169,6 +8160,7 @@ static void pqi_remove_ctrl(struct pqi_ctrl_info *ctrl_info)
>   {
>   	pqi_cancel_rescan_worker(ctrl_info);
>   	pqi_cancel_update_time_worker(ctrl_info);
> +	pqi_remove_all_scsi_devices(ctrl_info);
>   	pqi_unregister_scsi(ctrl_info);
>   	if (ctrl_info->pqi_mode_enabled)
>   		pqi_revert_to_sis_mode(ctrl_info);
>