From patchwork Fri Sep 15 08:14:45 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 724113 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 475DFEE6442 for ; Fri, 15 Sep 2023 08:26:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233001AbjIOI0J (ORCPT ); Fri, 15 Sep 2023 04:26:09 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53744 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233085AbjIOI0B (ORCPT ); Fri, 15 Sep 2023 04:26:01 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9DE1F449C; Fri, 15 Sep 2023 01:23:42 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id ED033C433C9; Fri, 15 Sep 2023 08:15:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765712; bh=g3BIcIFco/oFr9nhk2nJIvAHcMrNUjY6hk5vgy3kEqE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Zyfc7ra35gePyg/Pws+81nGy/H6Rfi6GIgk9FO8Ty3ycZh/cATRdY50sRISPESel1 1jjnpwJWYAKP5F46nE/cqaDvEb9e8Vd7SRBH14bI1CUD3hKSh8cScDmcCMx7eAEvKo qoRSkCGLujISNhRNmKVN+mUSegAU1DI8tm6lz3mD7x+4oyRZHTe+XW6cnYSF/NX6zx whWkl1Iti3jkYg8LlFXEQwv2l9MyW8/YoB8x3CEPRApxXRwYrTyUo6/IR78LErHara 6tmEt6a7rM9fSq/HmAUakODgcAOd2qb3X8yf/G7mH2v7WINFVv18PPDyJjOO/Sh+E6 t5EtW7iCYSPgA== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 01/23] ata: libata-core: Fix ata_port_request_pm() locking Date: Fri, 15 Sep 2023 17:14:45 +0900 Message-ID: <20230915081507.761711-2-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org The function ata_port_request_pm() checks the port flag ATA_PFLAG_PM_PENDING and calls ata_port_wait_eh() if this flag is set to ensure that power management operations for a port are not secheduled simultaneously. However, this flag check is done without holding the port lock. Fix this by taking the port lock on entry to the function and checking the flag under this lock. The lock is released and re-taken if ata_port_wait_eh() needs to be called. Fixes: 5ef41082912b ("ata: add ata port system PM callbacks") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke Tested-by: Chia-Lin Kao (AceLan) --- drivers/ata/libata-core.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 74314311295f..c4898483d716 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -5040,17 +5040,20 @@ static void ata_port_request_pm(struct ata_port *ap, pm_message_t mesg, struct ata_link *link; unsigned long flags; - /* Previous resume operation might still be in - * progress. Wait for PM_PENDING to clear. + spin_lock_irqsave(ap->lock, flags); + + /* + * A previous PM operation might still be in progress. Wait for + * ATA_PFLAG_PM_PENDING to clear. */ if (ap->pflags & ATA_PFLAG_PM_PENDING) { + spin_unlock_irqrestore(ap->lock, flags); ata_port_wait_eh(ap); + spin_lock_irqsave(ap->lock, flags); WARN_ON(ap->pflags & ATA_PFLAG_PM_PENDING); } - /* request PM ops to EH */ - spin_lock_irqsave(ap->lock, flags); - + /* Request PM operation to EH */ ap->pm_mesg = mesg; ap->pflags |= ATA_PFLAG_PM_PENDING; ata_for_each_link(link, ap, HOST_FIRST) { @@ -5062,10 +5065,8 @@ static void ata_port_request_pm(struct ata_port *ap, pm_message_t mesg, spin_unlock_irqrestore(ap->lock, flags); - if (!async) { + if (!async) ata_port_wait_eh(ap); - WARN_ON(ap->pflags & ATA_PFLAG_PM_PENDING); - } } /* From patchwork Fri Sep 15 08:14:46 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 723482 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 598D8EE643D for ; Fri, 15 Sep 2023 08:21:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232831AbjIOIVk (ORCPT ); Fri, 15 Sep 2023 04:21:40 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33784 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232830AbjIOIVO (ORCPT ); Fri, 15 Sep 2023 04:21:14 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AB2184204; Fri, 15 Sep 2023 01:19:09 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id C2017C433CA; Fri, 15 Sep 2023 08:15:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765714; bh=zOe28gJJSf8OTmDL6uLvodfOenut1Ucwv+kF4u8YqrA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=jZevBuWmlLJy5rYtNxT5rqc+Lo/bSVe9MwKHgH579AjTTtd00RKXY2erboUOLsosv gSX93Be3oA7BKvzvRedIDiGq/5LEP9qXzEEhf0+6q6XFjCKfN3gZ5UZfXlegYUaJVv 8WjwDpAhxdn9iIAuWx0I+4HSksXQJtJjyGr0+mNvX04XFKqzct7apzisWGkE6ONm90 2qmz/D4z2bheg5cV16jDRbbbAPcQxIyTCPzRGFcm9QixCJ5mLzPDaBWHE5TmT3WfvM NR0TKjEi+7PB8MKvqXuQe4S6L9fKMHf78o+PcUQesKn2MOoS71VnWMyzuj9HepIo2R yhB5nZVG54vVA== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 02/23] ata: libata-core: Fix port and device removal Date: Fri, 15 Sep 2023 17:14:46 +0900 Message-ID: <20230915081507.761711-3-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org Whenever an ATA adapter driver is removed (e.g. rmmod), ata_port_detach() is called repeatedly for all the adapter ports to remove (unload) the devices attached to the port and delete the port device itself. Removing of devices is done using libata EH with the ATA_PFLAG_UNLOADING port flag set. This causes libata EH to execute ata_eh_unload() which disables all devices attached to the port. ata_port_detach() finishes by calling scsi_remove_host() to remove the scsi host associated with the port. This function will trigger the removal of all scsi devices attached to the host and in the case of disks, calls to sd_shutdown() which will flush the device write cache and stop the device. However, given that the devices were already disabled by ata_eh_unload(), the synchronize write cache command and start stop unit commands fail. E.g. running "rmmod ahci" with first removing sd_mod results in error messages like: ata13.00: disable device sd 0:0:0:0: [sda] Synchronizing SCSI cache sd 0:0:0:0: [sda] Synchronize Cache(10) failed: Result: hostbyte=DID_BAD_TARGET driverbyte=DRIVER_OK sd 0:0:0:0: [sda] Stopping disk sd 0:0:0:0: [sda] Start/Stop Unit failed: Result: hostbyte=DID_BAD_TARGET driverbyte=DRIVER_OK Fix this by removing all scsi devices of the ata devices connected to the port before scheduling libata EH to disable the ATA devices. Fixes: 720ba12620ee ("[PATCH] libata-hp: update unload-unplug") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke Tested-by: Chia-Lin Kao (AceLan) Reviewed-by: Niklas Cassel --- drivers/ata/libata-core.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index c4898483d716..693cb3cd70cd 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -5952,11 +5952,30 @@ static void ata_port_detach(struct ata_port *ap) struct ata_link *link; struct ata_device *dev; - /* tell EH we're leaving & flush EH */ + /* Wait for any ongoing EH */ + ata_port_wait_eh(ap); + + mutex_lock(&ap->scsi_scan_mutex); spin_lock_irqsave(ap->lock, flags); + + /* Remove scsi devices */ + ata_for_each_link(link, ap, HOST_FIRST) { + ata_for_each_dev(dev, link, ALL) { + if (dev->sdev) { + spin_unlock_irqrestore(ap->lock, flags); + scsi_remove_device(dev->sdev); + spin_lock_irqsave(ap->lock, flags); + dev->sdev = NULL; + } + } + } + + /* Tell EH to disable all devices */ ap->pflags |= ATA_PFLAG_UNLOADING; ata_port_schedule_eh(ap); + spin_unlock_irqrestore(ap->lock, flags); + mutex_unlock(&ap->scsi_scan_mutex); /* wait till EH commits suicide */ ata_port_wait_eh(ap); From patchwork Fri Sep 15 08:14:47 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 724112 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0BF10EE6442 for ; Fri, 15 Sep 2023 08:26:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233093AbjIOI0b (ORCPT ); Fri, 15 Sep 2023 04:26:31 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48768 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233061AbjIOI0Y (ORCPT ); Fri, 15 Sep 2023 04:26:24 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 399263AA0; Fri, 15 Sep 2023 01:24:55 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 94BA7C433CB; Fri, 15 Sep 2023 08:15:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765716; bh=4DKquiB4S/emfjkbWLYiqKJhCjGbwS78HSyGtZ17Wws=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=aZtsC65dmv+dk1tC8OLwys+2kqv24YNjJhaiNnPLNstTwW4FKcMNHTfs4ldlPC02Z QaUyBfDEtgIEtRiuL+hEKpao2FdzGGelFwtgdWLbxAG+srpQkaAJkXmTbH5I01M9Y2 U1c6hNWusVTXYgEx0Su7rrFbjpGbhEim4yevKi9Afn0VH8ayOf93JSEaFE5zbEX7LL kkcb5VZqu2lqUAmkpRB95I/kfYr8/Ilr/VAOvLiPxZWeDkj4yisKZ8GOfJZYj2JEG5 GqGVIekKjGD/R160+8/cXh2du4hOJBVWzFCBNaJ4Ly5WhvKN3GAROaE1fL8Jjlg/rb OKvf3Q4BqQ/Pg== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 03/23] ata: libata-scsi: link ata port and scsi device Date: Fri, 15 Sep 2023 17:14:47 +0900 Message-ID: <20230915081507.761711-4-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org There is no direct device ancestry defined between an ata_device and its scsi device which prevents the power management code from correctly ordering suspend and resume operations. Create such ancestry with the ata device as the parent to ensure that the scsi device (child) is suspended before the ata device and that resume handles the ata device before the scsi device. The parent-child (supplier-consumer) relationship is established between the ata_port (parent) and the scsi device (child) with the function device_add_link(). The parent used is not the ata_device as the PM operations are defined per port and the status of all devices connected through that port is controlled from the port operations. The device link is established with the new function ata_scsi_dev_alloc(). This function is used to define the ->slave_alloc callback of the scsi host template of most drivers. Fixes: a19a93e4c6a9 ("scsi: core: pm: Rely on the device driver core for async power management") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke --- drivers/ata/ahci.h | 1 + drivers/ata/libata-scsi.c | 49 +++++++++++++++++++++++++++++++++++---- drivers/ata/libata.h | 1 + drivers/ata/pata_macio.c | 1 + drivers/ata/sata_mv.c | 1 + drivers/ata/sata_nv.c | 2 ++ drivers/ata/sata_sil24.c | 1 + include/linux/libata.h | 3 +++ 8 files changed, 54 insertions(+), 5 deletions(-) diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h index 4bae95b06ae3..72085756f4ba 100644 --- a/drivers/ata/ahci.h +++ b/drivers/ata/ahci.h @@ -398,6 +398,7 @@ extern const struct attribute_group *ahci_sdev_groups[]; .sdev_groups = ahci_sdev_groups, \ .change_queue_depth = ata_scsi_change_queue_depth, \ .tag_alloc_policy = BLK_TAG_ALLOC_RR, \ + .slave_alloc = ata_scsi_slave_alloc, \ .slave_configure = ata_scsi_slave_config extern struct ata_port_operations ahci_ops; diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index d3f28b82c97b..eef76af1af90 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -1089,6 +1089,46 @@ int ata_scsi_dev_config(struct scsi_device *sdev, struct ata_device *dev) return 0; } +int ata_scsi_dev_alloc(struct scsi_device *sdev, struct ata_port *ap) +{ + struct device_link *link; + + ata_scsi_sdev_config(sdev); + + /* + * Create a link from the ata_port device to the scsi device to ensure + * that PM does suspend/resume in the correct order: the scsi device is + * consumer (child) and the ata port the supplier (parent). + */ + link = device_link_add(&sdev->sdev_gendev, &ap->tdev, + DL_FLAG_STATELESS | + DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE); + if (!link) { + ata_port_err(ap, "Failed to create link to scsi device %s\n", + dev_name(&sdev->sdev_gendev)); + return -ENODEV; + } + + return 0; +} + +/** + * ata_scsi_slave_alloc - Early setup of SCSI device + * @sdev: SCSI device to examine + * + * This is called from scsi_alloc_sdev() when the scsi device + * associated with an ATA device is scanned on a port. + * + * LOCKING: + * Defined by SCSI layer. We don't really care. + */ + +int ata_scsi_slave_alloc(struct scsi_device *sdev) +{ + return ata_scsi_dev_alloc(sdev, ata_shost_to_port(sdev->host)); +} +EXPORT_SYMBOL_GPL(ata_scsi_slave_alloc); + /** * ata_scsi_slave_config - Set SCSI device attributes * @sdev: SCSI device to examine @@ -1105,14 +1145,11 @@ int ata_scsi_slave_config(struct scsi_device *sdev) { struct ata_port *ap = ata_shost_to_port(sdev->host); struct ata_device *dev = __ata_scsi_find_dev(ap, sdev); - int rc = 0; - - ata_scsi_sdev_config(sdev); if (dev) - rc = ata_scsi_dev_config(sdev, dev); + return ata_scsi_dev_config(sdev, dev); - return rc; + return 0; } EXPORT_SYMBOL_GPL(ata_scsi_slave_config); @@ -1136,6 +1173,8 @@ void ata_scsi_slave_destroy(struct scsi_device *sdev) unsigned long flags; struct ata_device *dev; + device_link_remove(&sdev->sdev_gendev, &ap->tdev); + spin_lock_irqsave(ap->lock, flags); dev = __ata_scsi_find_dev(ap, sdev); if (dev && dev->sdev) { diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h index 6e7d352803bd..079981e7156a 100644 --- a/drivers/ata/libata.h +++ b/drivers/ata/libata.h @@ -111,6 +111,7 @@ extern struct ata_device *ata_scsi_find_dev(struct ata_port *ap, extern int ata_scsi_add_hosts(struct ata_host *host, const struct scsi_host_template *sht); extern void ata_scsi_scan_host(struct ata_port *ap, int sync); +extern int ata_scsi_dev_alloc(struct scsi_device *sdev, struct ata_port *ap); extern int ata_scsi_offline_dev(struct ata_device *dev); extern bool ata_scsi_sense_is_valid(u8 sk, u8 asc, u8 ascq); extern void ata_scsi_set_sense(struct ata_device *dev, diff --git a/drivers/ata/pata_macio.c b/drivers/ata/pata_macio.c index 17f6ccee53c7..32968b4cf8e4 100644 --- a/drivers/ata/pata_macio.c +++ b/drivers/ata/pata_macio.c @@ -918,6 +918,7 @@ static const struct scsi_host_template pata_macio_sht = { * use 64K minus 256 */ .max_segment_size = MAX_DBDMA_SEG, + .slave_alloc = ata_scsi_slave_alloc, .slave_configure = pata_macio_slave_config, .sdev_groups = ata_common_sdev_groups, .can_queue = ATA_DEF_QUEUE, diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c index d105db5c7d81..353ac7b2f14a 100644 --- a/drivers/ata/sata_mv.c +++ b/drivers/ata/sata_mv.c @@ -673,6 +673,7 @@ static const struct scsi_host_template mv6_sht = { .sdev_groups = ata_ncq_sdev_groups, .change_queue_depth = ata_scsi_change_queue_depth, .tag_alloc_policy = BLK_TAG_ALLOC_RR, + .slave_alloc = ata_scsi_slave_alloc, .slave_configure = ata_scsi_slave_config }; diff --git a/drivers/ata/sata_nv.c b/drivers/ata/sata_nv.c index 0a0cee755bde..5428dc2ec5e3 100644 --- a/drivers/ata/sata_nv.c +++ b/drivers/ata/sata_nv.c @@ -380,6 +380,7 @@ static const struct scsi_host_template nv_adma_sht = { .can_queue = NV_ADMA_MAX_CPBS, .sg_tablesize = NV_ADMA_SGTBL_TOTAL_LEN, .dma_boundary = NV_ADMA_DMA_BOUNDARY, + .slave_alloc = ata_scsi_slave_alloc, .slave_configure = nv_adma_slave_config, .sdev_groups = ata_ncq_sdev_groups, .change_queue_depth = ata_scsi_change_queue_depth, @@ -391,6 +392,7 @@ static const struct scsi_host_template nv_swncq_sht = { .can_queue = ATA_MAX_QUEUE - 1, .sg_tablesize = LIBATA_MAX_PRD, .dma_boundary = ATA_DMA_BOUNDARY, + .slave_alloc = ata_scsi_slave_alloc, .slave_configure = nv_swncq_slave_config, .sdev_groups = ata_ncq_sdev_groups, .change_queue_depth = ata_scsi_change_queue_depth, diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c index 142e70bfc498..e0b1b3625031 100644 --- a/drivers/ata/sata_sil24.c +++ b/drivers/ata/sata_sil24.c @@ -381,6 +381,7 @@ static const struct scsi_host_template sil24_sht = { .tag_alloc_policy = BLK_TAG_ALLOC_FIFO, .sdev_groups = ata_ncq_sdev_groups, .change_queue_depth = ata_scsi_change_queue_depth, + .slave_alloc = ata_scsi_slave_alloc, .slave_configure = ata_scsi_slave_config }; diff --git a/include/linux/libata.h b/include/linux/libata.h index 52d58b13e5ee..c8cfea386c16 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -1144,6 +1144,7 @@ extern int ata_std_bios_param(struct scsi_device *sdev, struct block_device *bdev, sector_t capacity, int geom[]); extern void ata_scsi_unlock_native_capacity(struct scsi_device *sdev); +extern int ata_scsi_slave_alloc(struct scsi_device *sdev); extern int ata_scsi_slave_config(struct scsi_device *sdev); extern void ata_scsi_slave_destroy(struct scsi_device *sdev); extern int ata_scsi_change_queue_depth(struct scsi_device *sdev, @@ -1401,12 +1402,14 @@ extern const struct attribute_group *ata_common_sdev_groups[]; __ATA_BASE_SHT(drv_name), \ .can_queue = ATA_DEF_QUEUE, \ .tag_alloc_policy = BLK_TAG_ALLOC_RR, \ + .slave_alloc = ata_scsi_slave_alloc, \ .slave_configure = ata_scsi_slave_config #define ATA_SUBBASE_SHT_QD(drv_name, drv_qd) \ __ATA_BASE_SHT(drv_name), \ .can_queue = drv_qd, \ .tag_alloc_policy = BLK_TAG_ALLOC_RR, \ + .slave_alloc = ata_scsi_slave_alloc, \ .slave_configure = ata_scsi_slave_config #define ATA_BASE_SHT(drv_name) \ From patchwork Fri Sep 15 08:14:48 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 724119 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 44475EE6447 for ; Fri, 15 Sep 2023 08:21:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232873AbjIOIVl (ORCPT ); Fri, 15 Sep 2023 04:21:41 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33818 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232877AbjIOIVQ (ORCPT ); Fri, 15 Sep 2023 04:21:16 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 02D7746BB; Fri, 15 Sep 2023 01:19:11 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 66240C433CC; Fri, 15 Sep 2023 08:15:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765717; bh=Z/kWCntVsCigYvBBX9kmP0IxajRJ+a0OfKhMDpgp7jc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=FMxdDamQUk5XLkS4N69Ln2eRQ9BRWHp6Mj091keTq8BmsQt0MCyRw7DFaQepbtg92 JNZVw7txieeUEXdmnivCcJy8eTzVlgQtblevOghZ7euhqILqIRuJ2bKaDvd80AHkcX Ar+Jf85JkhN1bSyJnFJxLNydlpu9HwKXFeXNBGnVqq6Giqw8bJuVVkKmv/pgfWLaju xqpW1DTN9mfYYNHqAvBnv2GfCZ5Eezle/nBlUrtZusKsDvR8fDqcaSuX2yH+y5BCwO F0ztM4itj6REKJ2wSZtb4FpcCXx1XCkYa7Fkb26UgMKJdVSwJfG4+IRLHx3/DzpENF hkn4CoTu+AVVg== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 04/23] scsi: sd: Differentiate system and runtime start/stop management Date: Fri, 15 Sep 2023 17:14:48 +0900 Message-ID: <20230915081507.761711-5-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org The underlying device and driver of a scsi disk may have different system and runtime power mode control requirements. This is because runtime power management affects only the scsi disk, while sustem level power management affects all devices, including the controller for the scsi disk. For instance, issuing a START STOP UNIT command when a scsi disk is runtime suspended and resumed is fine: the command is translated to a STANDBY IMMEDIATE command to spin down the ATA disk and to a VERIFY command to wake it up. The scsi disk runtime operations have no effect on the ata port device used to connect the ATA disk. However, for system suspend/resume operations, the ATA port used to connect the device will also be suspended and resumed, with the resum operation requiring re-validating the device link and the device itseld. In this case, issuing a VERIFY command to spinup the disk must be done before starting to revalidate the device, when the ata port is being resumed. In such case, we must not allow the scsi disk driver to issue START STOP UNIT commands. Allow a low level driver to refine the scsi disk start/stop management by differentiating system and runtime cases with two new scsi device flags: manage_system_start_stop and manage_runtime_start_stop. These new flags replace the current manage_start_stop flag. Drivers setting the manage_start_stop are modifed to set both new flags, thus preserving the existing start/stop management behavior. Fixes: 0a8589055936 ("ata,scsi: do not issue START STOP UNIT on resume") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal --- drivers/ata/libata-scsi.c | 3 +- drivers/firewire/sbp2.c | 9 +++-- drivers/scsi/sd.c | 78 ++++++++++++++++++++++++++++---------- include/scsi/scsi_device.h | 3 +- 4 files changed, 69 insertions(+), 24 deletions(-) diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index eef76af1af90..02a064a9cfdd 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -1056,7 +1056,8 @@ int ata_scsi_dev_config(struct scsi_device *sdev, struct ata_device *dev) * will be woken up by ata_port_pm_resume() with a port reset * and device revalidation. */ - sdev->manage_start_stop = 1; + sdev->manage_system_start_stop = 1; + sdev->manage_runtime_start_stop = 1; sdev->no_start_on_resume = 1; } diff --git a/drivers/firewire/sbp2.c b/drivers/firewire/sbp2.c index 26db5b8dfc1e..f759e26241d3 100644 --- a/drivers/firewire/sbp2.c +++ b/drivers/firewire/sbp2.c @@ -81,7 +81,8 @@ MODULE_PARM_DESC(exclusive_login, "Exclusive login to sbp2 device " * * - power condition * Set the power condition field in the START STOP UNIT commands sent by - * sd_mod on suspend, resume, and shutdown (if manage_start_stop is on). + * sd_mod on suspend, resume, and shutdown (if manage_system_start_stop or + * manage_runtime_start_stop is on). * Some disks need this to spin down or to resume properly. * * - override internal blacklist @@ -1517,8 +1518,10 @@ static int sbp2_scsi_slave_configure(struct scsi_device *sdev) sdev->use_10_for_rw = 1; - if (sbp2_param_exclusive_login) - sdev->manage_start_stop = 1; + if (sbp2_param_exclusive_login) { + sdev->manage_system_start_stop = 1; + sdev->manage_runtime_start_stop = 1; + } if (sdev->type == TYPE_ROM) sdev->use_10_for_ms = 1; diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index c92a317ba547..1d106c8ad5af 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -201,18 +201,50 @@ cache_type_store(struct device *dev, struct device_attribute *attr, } static ssize_t -manage_start_stop_show(struct device *dev, struct device_attribute *attr, - char *buf) +manage_system_start_stop_show(struct device *dev, + struct device_attribute *attr, char *buf) { struct scsi_disk *sdkp = to_scsi_disk(dev); struct scsi_device *sdp = sdkp->device; - return sprintf(buf, "%u\n", sdp->manage_start_stop); + return sprintf(buf, "%u\n", sdp->manage_system_start_stop); } static ssize_t -manage_start_stop_store(struct device *dev, struct device_attribute *attr, - const char *buf, size_t count) +manage_system_start_stop_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct scsi_disk *sdkp = to_scsi_disk(dev); + struct scsi_device *sdp = sdkp->device; + bool v; + + if (!capable(CAP_SYS_ADMIN)) + return -EACCES; + + if (kstrtobool(buf, &v)) + return -EINVAL; + + sdp->manage_system_start_stop = v; + + return count; +} +static DEVICE_ATTR_RW(manage_system_start_stop); + +static ssize_t +manage_runtime_start_stop_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct scsi_disk *sdkp = to_scsi_disk(dev); + struct scsi_device *sdp = sdkp->device; + + return sprintf(buf, "%u\n", sdp->manage_runtime_start_stop); +} + +static ssize_t +manage_runtime_start_stop_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) { struct scsi_disk *sdkp = to_scsi_disk(dev); struct scsi_device *sdp = sdkp->device; @@ -224,11 +256,11 @@ manage_start_stop_store(struct device *dev, struct device_attribute *attr, if (kstrtobool(buf, &v)) return -EINVAL; - sdp->manage_start_stop = v; + sdp->manage_runtime_start_stop = v; return count; } -static DEVICE_ATTR_RW(manage_start_stop); +static DEVICE_ATTR_RW(manage_runtime_start_stop); static ssize_t allow_restart_show(struct device *dev, struct device_attribute *attr, char *buf) @@ -559,7 +591,8 @@ static struct attribute *sd_disk_attrs[] = { &dev_attr_cache_type.attr, &dev_attr_FUA.attr, &dev_attr_allow_restart.attr, - &dev_attr_manage_start_stop.attr, + &dev_attr_manage_system_start_stop.attr, + &dev_attr_manage_runtime_start_stop.attr, &dev_attr_protection_type.attr, &dev_attr_protection_mode.attr, &dev_attr_app_tag_own.attr, @@ -3771,13 +3804,20 @@ static void sd_shutdown(struct device *dev) sd_sync_cache(sdkp, NULL); } - if (system_state != SYSTEM_RESTART && sdkp->device->manage_start_stop) { + if (system_state != SYSTEM_RESTART && + sdkp->device->manage_system_start_stop) { sd_printk(KERN_NOTICE, sdkp, "Stopping disk\n"); sd_start_stop_device(sdkp, 0); } } -static int sd_suspend_common(struct device *dev, bool ignore_stop_errors) +static inline bool sd_do_start_stop(struct scsi_device *sdev, bool runtime) +{ + return (sdev->manage_system_start_stop && !runtime) || + (sdev->manage_runtime_start_stop && runtime); +} + +static int sd_suspend_common(struct device *dev, bool runtime) { struct scsi_disk *sdkp = dev_get_drvdata(dev); struct scsi_sense_hdr sshdr; @@ -3809,12 +3849,12 @@ static int sd_suspend_common(struct device *dev, bool ignore_stop_errors) } } - if (sdkp->device->manage_start_stop) { + if (sd_do_start_stop(sdkp->device, runtime)) { if (!sdkp->device->silence_suspend) sd_printk(KERN_NOTICE, sdkp, "Stopping disk\n"); /* an error is not worth aborting a system sleep */ ret = sd_start_stop_device(sdkp, 0); - if (ignore_stop_errors) + if (!runtime) ret = 0; } @@ -3826,23 +3866,23 @@ static int sd_suspend_system(struct device *dev) if (pm_runtime_suspended(dev)) return 0; - return sd_suspend_common(dev, true); + return sd_suspend_common(dev, false); } static int sd_suspend_runtime(struct device *dev) { - return sd_suspend_common(dev, false); + return sd_suspend_common(dev, true); } -static int sd_resume(struct device *dev) +static int sd_resume(struct device *dev, bool runtime) { struct scsi_disk *sdkp = dev_get_drvdata(dev); - int ret = 0; + int ret; if (!sdkp) /* E.g.: runtime resume at the start of sd_probe() */ return 0; - if (!sdkp->device->manage_start_stop) + if (!sd_do_start_stop(sdkp->device, runtime)) return 0; if (!sdkp->device->no_start_on_resume) { @@ -3860,7 +3900,7 @@ static int sd_resume_system(struct device *dev) if (pm_runtime_suspended(dev)) return 0; - return sd_resume(dev); + return sd_resume(dev, false); } static int sd_resume_runtime(struct device *dev) @@ -3887,7 +3927,7 @@ static int sd_resume_runtime(struct device *dev) "Failed to clear sense data\n"); } - return sd_resume(dev); + return sd_resume(dev, true); } static const struct dev_pm_ops sd_pm_ops = { diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h index b9230b6add04..b7df1e6da969 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -193,7 +193,8 @@ struct scsi_device { unsigned use_192_bytes_for_3f:1; /* ask for 192 bytes from page 0x3f */ unsigned no_start_on_add:1; /* do not issue start on add */ unsigned allow_restart:1; /* issue START_UNIT in error handler */ - unsigned manage_start_stop:1; /* Let HLD (sd) manage start/stop */ + unsigned manage_system_start_stop:1; /* Let HLD (sd) manage system start/stop */ + unsigned manage_runtime_start_stop:1; /* Let HLD (sd) manage runtime start/stop */ unsigned no_start_on_resume:1; /* Do not issue START_STOP_UNIT on resume */ unsigned start_stop_pwr_cond:1; /* Set power cond. in START_STOP_UNIT */ unsigned no_uld_attach:1; /* disable connecting to upper level drivers */ From patchwork Fri Sep 15 08:14:49 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 723480 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E0491EE6449 for ; Fri, 15 Sep 2023 08:23:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233008AbjIOIXt (ORCPT ); Fri, 15 Sep 2023 04:23:49 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49214 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232960AbjIOIXq (ORCPT ); Fri, 15 Sep 2023 04:23:46 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 920A744B9; Fri, 15 Sep 2023 01:22:37 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3B1BBC433C8; Fri, 15 Sep 2023 08:15:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765719; bh=o4F2CpKbIAREqljcifS32rYquoPEGSVJ6rwVuv+nKvQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=uwERH0BIa5Hi70WS03UOZJ4Xuz92qXFLivISXr0GR67y1hV+Gr+U2NFJ1dH8W6RPF JaL3Hswm4NsxCqzbC77lXYba5T2Ozs0Rc+fkjYOBGZNccZm7gJNqkaKmIupunIYidN 2Qf8GvWJ5Kunk9lFzPawX87zS6BP0NuIYsSJkv/e7gP7xpC/HH6L7RIwtXJgapQ8Dg DKGrgrSboGcBbjSbBdZ1Wy3KXnnMGslI0aS0WXxxmofeuANJugHGpUV8/UZgHbkv6d xtgbkOlqsgRQLsGdqnJKocp5LKvL7aV0iLN4fW/jSEGPkQ2VG6e4Dbcf6B2ls1Xmuq u6sQd512sBtxA== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 05/23] ata: libata-scsi: Disable scsi device manage_system_start_stop Date: Fri, 15 Sep 2023 17:14:49 +0900 Message-ID: <20230915081507.761711-6-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org The introduction of a device link to create a consumer/supplier relationship between the scsi device of an ATA device and the ATA port of that ATA device fixes the ordering of system suspend and resume operations. For suspend, the scsi device is suspended first and the ata port after it. This is fine as this allows the synchronize cache and START STOP UNIT commands issued by the scsi disk driver to be executed before the ata port is disabled. For resume operations, the ata port is resumed first, followed by the scsi device. This allows having the request queue of the scsi device to be unfrozen after the ata port resume is scheduled in EH, thus avoiding to see new requests prematurely issued to the ATA device. Since libata sets manage_system_start_stop to 1, the scsi disk resume operation also results in issuing a START STOP UNIT command to the device being resumed so that the device exits standby power mode. However, restoring the ATA device to the active power mode must be synchronized with libata EH processing of the port resume operation to avoid either 1) seeing the start stop unit command being received too early when the port is not yet resumed and ready to accept commands, or after the port resume process issues commands such as IDENTIFY to revalidate the device. In this last case, the risk is that the device revalidation fails with timeout errors as the drive is still spun down. Commit 0a8589055936 ("ata,scsi: do not issue START STOP UNIT on resume") disabled issuing the START STOP UNIT command to avoid issues with it. But this is incorrect as transitioning a device to the active power mode from the standby power mode set on suspend requires a media access command. The IDENTIFY, READ LOG and SET FEATURES commands executed in libata EH context triggered by the ata port resume operation may thus fail. Fix these synchronization issues is by handling a device power mode transitions for system suspend and resume directly in libata EH context, without relying on the scsi disk driver management triggered with the manage_system_start_stop flag. To do this, the following libata helper functions are introduced: 1) ata_dev_power_set_standby(): This function issues a STANDBY IMMEDIATE command to transitiom a device to the standby power mode. For HDDs, this spins down the disks. This function applies only to ATA and ZAC devices and does nothing otherwise. This function also does nothing for devices that have the ATA_FLAG_NO_POWEROFF_SPINDOWN or ATA_FLAG_NO_HIBERNATE_SPINDOWN flag set. For suspend, call ata_dev_power_set_standby() in ata_eh_handle_port_suspend() before the port is disabled and frozen. ata_eh_unload() is also modified to transition all enabled devices to the standby power mode when the system is shutdown or devices removed. 2) ata_dev_power_set_active() and This function applies to ATA or ZAC devices and issues a VERIFY command for 1 sector at LBA 0 to transition the device to the active power mode. For HDDs, since this function will complete only once the disk spin up. Its execution uses the same timeouts as for reset, to give the drive enough time to complete spinup without triggering a command timeout. For resume, call ata_dev_power_set_active() in ata_eh_revalidate_and_attach() after the port has been enabled and before any other command is issued to the device. With these changes, the manage_system_start_stop and no_start_on_resume scsi device flags do not need to be set in ata_scsi_dev_config(). The flag manage_runtime_start_stop is still set to allow the sd driver to spinup/spindown a disk through the sd runtime operations. Fixes: 0a8589055936 ("ata,scsi: do not issue START STOP UNIT on resume") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke --- drivers/ata/libata-core.c | 90 +++++++++++++++++++++++++++++++++++++++ drivers/ata/libata-eh.c | 46 +++++++++++++++++++- drivers/ata/libata-scsi.c | 16 +++---- drivers/ata/libata.h | 2 + include/linux/libata.h | 6 ++- 5 files changed, 148 insertions(+), 12 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 693cb3cd70cd..0cf0caf77907 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -1972,6 +1972,96 @@ int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class, return rc; } +/** + * ata_dev_power_set_standby - Set a device power mode to standby + * @dev: target device + * + * Issue a STANDBY IMMEDIATE command to set a device power mode to standby. + * For an HDD device, this spins down the disks. + * + * LOCKING: + * Kernel thread context (may sleep). + */ +void ata_dev_power_set_standby(struct ata_device *dev) +{ + unsigned long ap_flags = dev->link->ap->flags; + struct ata_taskfile tf; + unsigned int err_mask; + + /* Issue STANDBY IMMEDIATE command only if supported by the device */ + if (dev->class != ATA_DEV_ATA && dev->class != ATA_DEV_ZAC) + return; + + /* + * Some odd clown BIOSes issue spindown on power off (ACPI S4 or S5) + * causing some drives to spin up and down again. For these, do nothing + * if we are being called on shutdown. + */ + if ((ap_flags & ATA_FLAG_NO_POWEROFF_SPINDOWN) && + system_state == SYSTEM_POWER_OFF) + return; + + if ((ap_flags & ATA_FLAG_NO_HIBERNATE_SPINDOWN) && + system_entering_hibernation()) + return; + + ata_tf_init(dev, &tf); + tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR; + tf.protocol = ATA_PROT_NODATA; + tf.command = ATA_CMD_STANDBYNOW1; + + ata_dev_notice(dev, "Entering standby power mode\n"); + + err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, 0); + if (err_mask) + ata_dev_err(dev, "STANDBY IMMEDIATE failed (err_mask=0x%x)\n", + err_mask); +} + +/** + * ata_dev_power_set_active - Set a device power mode to active + * @dev: target device + * + * Issue a VERIFY command to enter to ensure that the device is in the + * active power mode. For a spun-down HDD (standby or idle power mode), + * the VERIFY command will complete after the disk spins up. + * + * LOCKING: + * Kernel thread context (may sleep). + */ +void ata_dev_power_set_active(struct ata_device *dev) +{ + struct ata_taskfile tf; + unsigned int err_mask; + + /* + * Issue READ VERIFY SECTORS command for 1 sector at lba=0 only + * if supported by the device. + */ + if (dev->class != ATA_DEV_ATA && dev->class != ATA_DEV_ZAC) + return; + + ata_tf_init(dev, &tf); + tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR; + tf.protocol = ATA_PROT_NODATA; + tf.command = ATA_CMD_VERIFY; + tf.nsect = 1; + if (dev->flags & ATA_DFLAG_LBA) { + tf.flags |= ATA_TFLAG_LBA; + tf.device |= ATA_LBA; + } else { + /* CHS */ + tf.lbal = 0x1; /* sect */ + } + + ata_dev_notice(dev, "Entering active power mode\n"); + + err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, 0); + if (err_mask) + ata_dev_err(dev, "VERIFY failed (err_mask=0x%x)\n", + err_mask); +} + /** * ata_read_log_page - read a specific log page * @dev: target device diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index 159ba6ba19eb..43b0a1509548 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -147,6 +147,8 @@ ata_eh_cmd_timeout_table[ATA_EH_CMD_TIMEOUT_TABLE_SIZE] = { .timeouts = ata_eh_other_timeouts, }, { .commands = CMDS(ATA_CMD_FLUSH, ATA_CMD_FLUSH_EXT), .timeouts = ata_eh_flush_timeouts }, + { .commands = CMDS(ATA_CMD_VERIFY), + .timeouts = ata_eh_reset_timeouts }, }; #undef CMDS @@ -498,7 +500,19 @@ static void ata_eh_unload(struct ata_port *ap) struct ata_device *dev; unsigned long flags; - /* Restore SControl IPM and SPD for the next driver and + /* + * Unless we are restarting, transition all enabled devices to + * standby power mode. + */ + if (system_state != SYSTEM_RESTART) { + ata_for_each_link(link, ap, PMP_FIRST) { + ata_for_each_dev(dev, link, ENABLED) + ata_dev_power_set_standby(dev); + } + } + + /* + * Restore SControl IPM and SPD for the next driver and * disable attached devices. */ ata_for_each_link(link, ap, PMP_FIRST) { @@ -684,6 +698,10 @@ void ata_scsi_port_error_handler(struct Scsi_Host *host, struct ata_port *ap) ehc->saved_xfer_mode[devno] = dev->xfer_mode; if (ata_ncq_enabled(dev)) ehc->saved_ncq_enabled |= 1 << devno; + + /* If we are resuming, wake up the device */ + if (ap->pflags & ATA_PFLAG_RESUMING) + ehc->i.dev_action[devno] |= ATA_EH_SET_ACTIVE; } } @@ -743,6 +761,8 @@ void ata_scsi_port_error_handler(struct Scsi_Host *host, struct ata_port *ap) /* clean up */ spin_lock_irqsave(ap->lock, flags); + ap->pflags &= ~ATA_PFLAG_RESUMING; + if (ap->pflags & ATA_PFLAG_LOADING) ap->pflags &= ~ATA_PFLAG_LOADING; else if ((ap->pflags & ATA_PFLAG_SCSI_HOTPLUG) && @@ -1218,6 +1238,13 @@ void ata_eh_detach_dev(struct ata_device *dev) struct ata_eh_context *ehc = &link->eh_context; unsigned long flags; + /* + * If the device is still enabled, transition it to standby power mode + * (i.e. spin down HDDs). + */ + if (ata_dev_enabled(dev)) + ata_dev_power_set_standby(dev); + ata_dev_disable(dev); spin_lock_irqsave(ap->lock, flags); @@ -3026,6 +3053,15 @@ static int ata_eh_revalidate_and_attach(struct ata_link *link, if (ehc->i.flags & ATA_EHI_DID_RESET) readid_flags |= ATA_READID_POSTRESET; + /* + * When resuming, before executing any command, make sure to + * transition the device to the active power mode. + */ + if ((action & ATA_EH_SET_ACTIVE) && ata_dev_enabled(dev)) { + ata_dev_power_set_active(dev); + ata_eh_done(link, dev, ATA_EH_SET_ACTIVE); + } + if ((action & ATA_EH_REVALIDATE) && ata_dev_enabled(dev)) { WARN_ON(dev->class == ATA_DEV_PMP); @@ -3999,6 +4035,7 @@ static void ata_eh_handle_port_suspend(struct ata_port *ap) unsigned long flags; int rc = 0; struct ata_device *dev; + struct ata_link *link; /* are we suspending? */ spin_lock_irqsave(ap->lock, flags); @@ -4011,6 +4048,12 @@ static void ata_eh_handle_port_suspend(struct ata_port *ap) WARN_ON(ap->pflags & ATA_PFLAG_SUSPENDED); + /* Set all devices attached to the port in standby mode */ + ata_for_each_link(link, ap, HOST_FIRST) { + ata_for_each_dev(dev, link, ENABLED) + ata_dev_power_set_standby(dev); + } + /* * If we have a ZPODD attached, check its zero * power ready status before the port is frozen. @@ -4093,6 +4136,7 @@ static void ata_eh_handle_port_resume(struct ata_port *ap) /* update the flags */ spin_lock_irqsave(ap->lock, flags); ap->pflags &= ~(ATA_PFLAG_PM_PENDING | ATA_PFLAG_SUSPENDED); + ap->pflags |= ATA_PFLAG_RESUMING; spin_unlock_irqrestore(ap->lock, flags); } #endif /* CONFIG_PM */ diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 02a064a9cfdd..ac2d332b4963 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -1050,15 +1050,13 @@ int ata_scsi_dev_config(struct scsi_device *sdev, struct ata_device *dev) } } else { sdev->sector_size = ata_id_logical_sector_size(dev->id); + /* - * Stop the drive on suspend but do not issue START STOP UNIT - * on resume as this is not necessary and may fail: the device - * will be woken up by ata_port_pm_resume() with a port reset - * and device revalidation. + * Ask the sd driver to issue START STOP UNIT on runtime suspend + * and resume only. For system level suspend/resume, devices + * power state is handled directly by libata EH. */ - sdev->manage_system_start_stop = 1; sdev->manage_runtime_start_stop = 1; - sdev->no_start_on_resume = 1; } /* @@ -1235,7 +1233,7 @@ static unsigned int ata_scsi_start_stop_xlat(struct ata_queued_cmd *qc) } if (cdb[4] & 0x1) { - tf->nsect = 1; /* 1 sector, lba=0 */ + tf->nsect = 1; /* 1 sector, lba=0 */ if (qc->dev->flags & ATA_DFLAG_LBA) { tf->flags |= ATA_TFLAG_LBA; @@ -1251,7 +1249,7 @@ static unsigned int ata_scsi_start_stop_xlat(struct ata_queued_cmd *qc) tf->lbah = 0x0; /* cyl high */ } - tf->command = ATA_CMD_VERIFY; /* READ VERIFY */ + tf->command = ATA_CMD_VERIFY; /* READ VERIFY */ } else { /* Some odd clown BIOSen issue spindown on power off (ACPI S4 * or S5) causing some drives to spin up and down again. @@ -1261,7 +1259,7 @@ static unsigned int ata_scsi_start_stop_xlat(struct ata_queued_cmd *qc) goto skip; if ((qc->ap->flags & ATA_FLAG_NO_HIBERNATE_SPINDOWN) && - system_entering_hibernation()) + system_entering_hibernation()) goto skip; /* Issue ATA STANDBY IMMEDIATE command */ diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h index 079981e7156a..a5ee06f0234a 100644 --- a/drivers/ata/libata.h +++ b/drivers/ata/libata.h @@ -60,6 +60,8 @@ extern int ata_dev_reread_id(struct ata_device *dev, unsigned int readid_flags); extern int ata_dev_revalidate(struct ata_device *dev, unsigned int new_class, unsigned int readid_flags); extern int ata_dev_configure(struct ata_device *dev); +extern void ata_dev_power_set_standby(struct ata_device *dev); +extern void ata_dev_power_set_active(struct ata_device *dev); extern int sata_down_spd_limit(struct ata_link *link, u32 spd_limit); extern int ata_down_xfermask_limit(struct ata_device *dev, unsigned int sel); extern unsigned int ata_dev_set_feature(struct ata_device *dev, diff --git a/include/linux/libata.h b/include/linux/libata.h index c8cfea386c16..6593c79b7290 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -192,6 +192,7 @@ enum { ATA_PFLAG_UNLOADING = (1 << 9), /* driver is being unloaded */ ATA_PFLAG_UNLOADED = (1 << 10), /* driver is unloaded */ + ATA_PFLAG_RESUMING = (1 << 16), /* port is being resumed */ ATA_PFLAG_SUSPENDED = (1 << 17), /* port is suspended (power) */ ATA_PFLAG_PM_PENDING = (1 << 18), /* PM operation pending */ ATA_PFLAG_INIT_GTM_VALID = (1 << 19), /* initial gtm data valid */ @@ -314,9 +315,10 @@ enum { ATA_EH_ENABLE_LINK = (1 << 3), ATA_EH_PARK = (1 << 5), /* unload heads and stop I/O */ ATA_EH_GET_SUCCESS_SENSE = (1 << 6), /* Get sense data for successful cmd */ + ATA_EH_SET_ACTIVE = (1 << 7), /* Set a device to active power mode */ ATA_EH_PERDEV_MASK = ATA_EH_REVALIDATE | ATA_EH_PARK | - ATA_EH_GET_SUCCESS_SENSE, + ATA_EH_GET_SUCCESS_SENSE | ATA_EH_SET_ACTIVE, ATA_EH_ALL_ACTIONS = ATA_EH_REVALIDATE | ATA_EH_RESET | ATA_EH_ENABLE_LINK, @@ -353,7 +355,7 @@ enum { /* This should match the actual table size of * ata_eh_cmd_timeout_table in libata-eh.c. */ - ATA_EH_CMD_TIMEOUT_TABLE_SIZE = 7, + ATA_EH_CMD_TIMEOUT_TABLE_SIZE = 8, /* Horkage types. May be set by libata or controller on drives (some horkage may be drive/controller pair dependent */ From patchwork Fri Sep 15 08:14:50 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 723477 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4F82DEE643D for ; Fri, 15 Sep 2023 08:25:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233049AbjIOIZ5 (ORCPT ); Fri, 15 Sep 2023 04:25:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59610 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233092AbjIOIZx (ORCPT ); Fri, 15 Sep 2023 04:25:53 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 01FBE4215; Fri, 15 Sep 2023 01:23:40 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0C598C433CD; Fri, 15 Sep 2023 08:15:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765721; bh=xXVwedrZT/LFdUWhRCXCKcCASpb8VmRoyYMVCjOGXCo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=MIg2UnOx2Ra8lcIx5yASvpJwUkpHWbUZ5yJMbkwyocAMtPlzO6YIn8nF71EBWkTW8 37a2i5lOwk7DS0JvLbNlCCFxVf7UY+BkO3eff36+XHiuOjPxHeMqDnarJAmh+kmmtf 6Lqg6bNgXj+MVVnhEvpTEzX9Muy2Az2mEaIWPOTmOyOYqbmb0yDAeiiAkaVeB5RO4V 7q9oXahE9h6rJZoCsDr0UBmV+9gGAkNJRCOCF8I8ECK3DJYWXH9OaN95k9XTFVyTlE RyUZyBTfjf3Bjf2ex2OsB8HyLAYyvVEVwnPz3ZT6M020OmjAwGlhBJucHFEzOwM61e QfbeqAPegK9cw== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 06/23] scsi: Do not attempt to rescan suspended devices Date: Fri, 15 Sep 2023 17:14:50 +0900 Message-ID: <20230915081507.761711-7-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org scsi_rescan_device() takes a scsi device lock before executing a device handler and device driver rescan methods. Waiting for the completion of any command issued to the device by these methods will thus be done with the device lock held. As a result, there is a risk of deadlocking within the power management code if scsi_rescan_device() is called to handle a device resume with the associated scsi device not yet resumed. Avoid such situation by checking that the target scsi device is in the running state, that is, fully capable of executing commands, before proceeding with the rescan and bailout returning -EWOULDBLOCK otherwise. With this error return, the caller can retry rescaning the device after a delay. The state check is done with the device lock held and is thus safe against incoming suspend power management operations. Fixes: 6aa0365a3c85 ("ata: libata-scsi: Avoid deadlock on rescan after device resume") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal Reviewed-by: Niklas Cassel --- drivers/scsi/scsi_scan.c | 18 +++++++++++++++++- include/scsi/scsi_host.h | 2 +- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index 52014b2d39e1..3db4d31a03a1 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -1619,12 +1619,24 @@ int scsi_add_device(struct Scsi_Host *host, uint channel, } EXPORT_SYMBOL(scsi_add_device); -void scsi_rescan_device(struct scsi_device *sdev) +int scsi_rescan_device(struct scsi_device *sdev) { struct device *dev = &sdev->sdev_gendev; + int ret = 0; device_lock(dev); + /* + * Bail out if the device is not running. Otherwise, the rescan may + * block waiting for commands to be executed, with us holding the + * device lock. This can result in a potential deadlock in the power + * management core code when system resume is on-going. + */ + if (sdev->sdev_state != SDEV_RUNNING) { + ret = -EWOULDBLOCK; + goto unlock; + } + scsi_attach_vpd(sdev); scsi_cdl_check(sdev); @@ -1638,7 +1650,11 @@ void scsi_rescan_device(struct scsi_device *sdev) drv->rescan(dev); module_put(dev->driver->owner); } + +unlock: device_unlock(dev); + + return ret; } EXPORT_SYMBOL(scsi_rescan_device); diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h index 49f768d0ff37..4c2dc8150c6d 100644 --- a/include/scsi/scsi_host.h +++ b/include/scsi/scsi_host.h @@ -764,7 +764,7 @@ scsi_template_proc_dir(const struct scsi_host_template *sht); #define scsi_template_proc_dir(sht) NULL #endif extern void scsi_scan_host(struct Scsi_Host *); -extern void scsi_rescan_device(struct scsi_device *); +extern int scsi_rescan_device(struct scsi_device *sdev); extern void scsi_remove_host(struct Scsi_Host *); extern struct Scsi_Host *scsi_host_get(struct Scsi_Host *); extern int scsi_host_busy(struct Scsi_Host *shost); From patchwork Fri Sep 15 08:14:51 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 723479 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 049DBEE6442 for ; Fri, 15 Sep 2023 08:24:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232952AbjIOIYL (ORCPT ); Fri, 15 Sep 2023 04:24:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45204 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232976AbjIOIYE (ORCPT ); Fri, 15 Sep 2023 04:24:04 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 528054483; Fri, 15 Sep 2023 01:22:37 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id D6DD6C433A9; Fri, 15 Sep 2023 08:15:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765723; bh=xju99tV1KfXuFrMaY6RNdeK2sIN09noX+KrBN3Jy9fA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=sKU/0EbciOdmM457Uo3P8YKRsdNIm6ICjkva50afhi1lZRCirhooobkoupV/qeoV8 nclEx5DjxxfPOulltiuaP30+p/JGK5PWwto0UIbJep5lHlgBnhBgpYsLlWUom1l07w qyDe0X+vNp6Iu0elUxbOJY35q7PxpqE3SK1SifXcxYismUz2GKuROvar2SStXYKVDD w3lGfUyDKUohewqHuqFS8dGdFc+6wvCqSalXZE+iPLVP/7IJ12NcSXd9HYRBe+X0nq QUUbfiLsj9qH4R6aLcVqQFaDP2986WrKeqB4IkTkc12FSblNuwcHBN7WzFnoAKYQH4 Tufq0N4DNI8rw== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 07/23] ata: libata-scsi: Fix delayed scsi_rescan_device() execution Date: Fri, 15 Sep 2023 17:14:51 +0900 Message-ID: <20230915081507.761711-8-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org Commit 6aa0365a3c85 ("ata: libata-scsi: Avoid deadlock on rescan after device resume") modified ata_scsi_dev_rescan() to check the scsi device "is_suspended" power field to ensure that the scsi device associated with an ATA device is fully resumed when scsi_rescan_device() is executed. However, this fix is problematic as: 1) It relies on a PM internal field that should not be used without PM device locking protection. 2) The check for is_suspended and the call to scsi_rescan_device() are not atomic and a suspend PM event may be triggered between them, casuing scsi_rescan_device() to be called on a suspended device and in that function blocking while holding the scsi device lock. This would deadlock a following resume operation. These problems can trigger PM deadlocks on resume, especially with resume operations triggered quickly after or during suspend operations. E.g., a simple bash script like: for (( i=0; i<10; i++ )); do echo "+2 > /sys/class/rtc/rtc0/wakealarm echo mem > /sys/power/state done that triggers a resume 2 seconds after starting suspending a system can quickly lead to a PM deadlock preventing the system from correctly resuming. Fix this by replacing the check on is_suspended with a check on the return value given by scsi_rescan_device() as that function will fail if called against a suspended device. Also make sure rescan tasks already scheduled are first cancelled before suspending an ata port. Fixes: 6aa0365a3c85 ("ata: libata-scsi: Avoid deadlock on rescan after device resume") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke Reviewed-by: Niklas Cassel --- drivers/ata/libata-core.c | 16 ++++++++++++++++ drivers/ata/libata-scsi.c | 33 +++++++++++++++------------------ 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 0cf0caf77907..0479493e54bd 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -5172,11 +5172,27 @@ static const unsigned int ata_port_suspend_ehi = ATA_EHI_QUIET static void ata_port_suspend(struct ata_port *ap, pm_message_t mesg) { + /* + * We are about to suspend the port, so we do not care about + * scsi_rescan_device() calls scheduled by previous resume operations. + * The next resume will schedule the rescan again. So cancel any rescan + * that is not done yet. + */ + cancel_delayed_work_sync(&ap->scsi_rescan_task); + ata_port_request_pm(ap, mesg, 0, ata_port_suspend_ehi, false); } static void ata_port_suspend_async(struct ata_port *ap, pm_message_t mesg) { + /* + * We are about to suspend the port, so we do not care about + * scsi_rescan_device() calls scheduled by previous resume operations. + * The next resume will schedule the rescan again. So cancel any rescan + * that is not done yet. + */ + cancel_delayed_work_sync(&ap->scsi_rescan_task); + ata_port_request_pm(ap, mesg, 0, ata_port_suspend_ehi, true); } diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index ac2d332b4963..6297f8c16a13 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -4760,7 +4760,7 @@ void ata_scsi_dev_rescan(struct work_struct *work) struct ata_link *link; struct ata_device *dev; unsigned long flags; - bool delay_rescan = false; + int ret = 0; mutex_lock(&ap->scsi_scan_mutex); spin_lock_irqsave(ap->lock, flags); @@ -4769,37 +4769,34 @@ void ata_scsi_dev_rescan(struct work_struct *work) ata_for_each_dev(dev, link, ENABLED) { struct scsi_device *sdev = dev->sdev; + /* + * If the port was suspended before this was scheduled, + * bail out. + */ + if (ap->pflags & ATA_PFLAG_SUSPENDED) + goto unlock; + if (!sdev) continue; if (scsi_device_get(sdev)) continue; - /* - * If the rescan work was scheduled because of a resume - * event, the port is already fully resumed, but the - * SCSI device may not yet be fully resumed. In such - * case, executing scsi_rescan_device() may cause a - * deadlock with the PM code on device_lock(). Prevent - * this by giving up and retrying rescan after a short - * delay. - */ - delay_rescan = sdev->sdev_gendev.power.is_suspended; - if (delay_rescan) { - scsi_device_put(sdev); - break; - } - spin_unlock_irqrestore(ap->lock, flags); - scsi_rescan_device(sdev); + ret = scsi_rescan_device(sdev); scsi_device_put(sdev); spin_lock_irqsave(ap->lock, flags); + + if (ret) + goto unlock; } } +unlock: spin_unlock_irqrestore(ap->lock, flags); mutex_unlock(&ap->scsi_scan_mutex); - if (delay_rescan) + /* Reschedule with a delay if scsi_rescan_device() returned an error */ + if (ret) schedule_delayed_work(&ap->scsi_rescan_task, msecs_to_jiffies(5)); } From patchwork Fri Sep 15 08:14:52 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 724116 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6C628EE643D for ; Fri, 15 Sep 2023 08:24:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232791AbjIOIY7 (ORCPT ); Fri, 15 Sep 2023 04:24:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42424 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232831AbjIOIY7 (ORCPT ); Fri, 15 Sep 2023 04:24:59 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 94BD7448F; Fri, 15 Sep 2023 01:23:42 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id AAA3FC43391; Fri, 15 Sep 2023 08:15:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765725; bh=DHhXXRVo/EQlbkp27+ld/0KI9b7Ov/dYdIFyyEm/wDM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Hxf9GtBPgmlKAwDGHVKDcRpS0VlreaZqMVfR7oSz95Yo0yNhpZrG577nxhK9y2Gp7 ZVJvJ5ZAK5g6LbOggJ0JJFT3iQLMLSii9wVi9qNOIUCC6bDjHQ01gjgD+mcyHd7rda ZOXaSkCYNWGSyklqItc5k7757ginlwiXuu99fG8GRTXPDn2zddYmo6PPT8jau7vQGW BQ2MUWKhMLmSgwTVV7L1Q53SbU0Jj6oM2F10KdGbUmBSYLBwJ+HBy+skLERJl7XwDd aIXkxgq8XMRcye2/ecp5bMPj6dO5joNVaxsICg4z8n2XdZnwa5THZKUfTwTaTIRBA0 w4WMvt7bjuxJw== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 08/23] ata: libata-core: Do not register PM operations for SAS ports Date: Fri, 15 Sep 2023 17:14:52 +0900 Message-ID: <20230915081507.761711-9-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org libsas does its own domain based power management of ports. For such ports, libata should not use a device type defining power management operations as executing these operations for suspend/resume in addition to libsas calls to ata_sas_port_suspend() and ata_sas_port_resume() is not necessary (and likely dangerous to do, even though problems are not seen currently). Introduce the new ata_port_sas_type device_type for ports managed by libsas. This new device type is used in ata_tport_add() and is defined without power management operations. Fixes: 2fcbdcb4c802 ("[SCSI] libata: export ata_port suspend/resume infrastructure for sas") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke Tested-by: Chia-Lin Kao (AceLan) --- drivers/ata/libata-core.c | 2 +- drivers/ata/libata-transport.c | 9 ++++++++- drivers/ata/libata.h | 2 ++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 0479493e54bd..18b2a0da9e54 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -5339,7 +5339,7 @@ EXPORT_SYMBOL_GPL(ata_host_resume); #endif const struct device_type ata_port_type = { - .name = "ata_port", + .name = ATA_PORT_TYPE_NAME, #ifdef CONFIG_PM .pm = &ata_port_pm_ops, #endif diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c index e4fb9d1b9b39..3e49a877500e 100644 --- a/drivers/ata/libata-transport.c +++ b/drivers/ata/libata-transport.c @@ -266,6 +266,10 @@ void ata_tport_delete(struct ata_port *ap) put_device(dev); } +static const struct device_type ata_port_sas_type = { + .name = ATA_PORT_TYPE_NAME, +}; + /** ata_tport_add - initialize a transport ATA port structure * * @parent: parent device @@ -283,7 +287,10 @@ int ata_tport_add(struct device *parent, struct device *dev = &ap->tdev; device_initialize(dev); - dev->type = &ata_port_type; + if (ap->flags & ATA_FLAG_SAS_HOST) + dev->type = &ata_port_sas_type; + else + dev->type = &ata_port_type; dev->parent = parent; ata_host_get(ap->host); diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h index a5ee06f0234a..c57e094c3af9 100644 --- a/drivers/ata/libata.h +++ b/drivers/ata/libata.h @@ -30,6 +30,8 @@ enum { ATA_DNXFER_QUIET = (1 << 31), }; +#define ATA_PORT_TYPE_NAME "ata_port" + extern atomic_t ata_print_id; extern int atapi_passthru16; extern int libata_fua; From patchwork Fri Sep 15 08:14:53 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 724108 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C838FEE643D for ; Fri, 15 Sep 2023 08:28:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233188AbjIOI2L (ORCPT ); Fri, 15 Sep 2023 04:28:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36452 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233319AbjIOI1z (ORCPT ); Fri, 15 Sep 2023 04:27:55 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 66F9F49FA; Fri, 15 Sep 2023 01:23:47 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 7BC39C433D9; Fri, 15 Sep 2023 08:15:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765727; bh=Fv8EC/3JKlOdzDCCl3PyvLRMaFFYh8IvyAKJkinGyIY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=M0svciAH3NG/OsWu0CDL1RW3i5dATkD56FZHmXnNhQ111FXFU28hj3+zLlwmYUDAF Ew3Wh7v5yhwVS8nYZVOkj4AAmBFpY/B90whe5PLZKNS8a1vk8kwqBL3vAXQ7kquaVO KjZwpaKLEjWFhihO53mJvgDS2v7ayy+XIyn7WFQRW62bNon6F2JPR+cgrlNPRE3Z8c U4UvssavcXU7Zn3KDfsFXk8/DIH2BfPLWJswD2HlpY4lm67lGQk/DmUV1ObBFJpR7P fkqUeTOyGdEKgW0FOz9q8ROAQOzgVpXQQOcD0NmpuBX9/ClkfCNEiNzM5ZSbGanDSx fGwI3gOPPcDPA== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 09/23] scsi: sd: Do not issue commands to suspended disks on shutdown Date: Fri, 15 Sep 2023 17:14:53 +0900 Message-ID: <20230915081507.761711-10-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org If an error occurs when resuming a host adapter before the devices attached to the adapter are resumed, the adapter low level driver may remove the scsi host, resulting in a call to sd_remove() for the disks of the host. This in turn results in a call to sd_shutdown() which will issue a synchronize cache command and a start stop unit command to spindown the disk. sd_shutdown() issues the commands only if the device is not already suspended but does not check the power state for system-wide suspend/resume. That is, the commands may be issued with the device in a suspended state, which causes PM resume to hang, forcing a reset of the machine to recover. Fix this by not calling sd_shutdown() in sd_remove() if the device is not running. Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal --- drivers/scsi/sd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 1d106c8ad5af..d86306d42445 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -3727,7 +3727,8 @@ static int sd_remove(struct device *dev) device_del(&sdkp->disk_dev); del_gendisk(sdkp->disk); - sd_shutdown(dev); + if (sdkp->device->sdev_state == SDEV_RUNNING) + sd_shutdown(dev); put_disk(sdkp->disk); return 0; From patchwork Fri Sep 15 08:14:54 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 724115 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 34708EE6442 for ; Fri, 15 Sep 2023 08:25:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232890AbjIOIZZ (ORCPT ); Fri, 15 Sep 2023 04:25:25 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52890 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232831AbjIOIZY (ORCPT ); Fri, 15 Sep 2023 04:25:24 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BADCF2D7E; Fri, 15 Sep 2023 01:23:40 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4D307C43395; Fri, 15 Sep 2023 08:15:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765728; bh=MuMx9zk1r5kdqaMhnwTxmXQAcG+11H6y+ejoe5oC/+w=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ufffBeBaQ9Sf/T5u7MpcCf5RXWC5r0lIlHjchvjG1aMjCNML6fZX5sdYdyHwiZXzc te95fDloiJzmZpqqMdu4n9Zf05FDiix+8okH2CQVcmWBR2fVDTfxVsOVNpWzLm3QjY DUDPjHoHchmcNx9xEU5GFjniCIGFKkiW00EHwyTjyKgwlq7aMS0yUDhdgYIc4dvuVD cZOETbdND3fADSkSpq3gOo0F0j0z/CrTUh51h0eUqf95p6ngqLMblDuVhGY3vL6ewb 0OTWn7ggCIpgo1JrF/YERi7A0cFQ9VrV/IoPB2Xqrmo+ZYPl/MY+L63DObJDfAHzaZ 6AyLmuK0tCzxA== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 10/23] ata: libata-core: Fix compilation warning in ata_dev_config_ncq() Date: Fri, 15 Sep 2023 17:14:54 +0900 Message-ID: <20230915081507.761711-11-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org The 24 bytes length allocated to the ncq_desc string in ata_dev_config_lba() for ata_dev_config_ncq() to use is too short, causing the following gcc compilation warnings when compiling with W=1: drivers/ata/libata-core.c: In function ‘ata_dev_configure’: drivers/ata/libata-core.c:2378:56: warning: ‘%d’ directive output may be truncated writing between 1 and 2 bytes into a region of size between 1 and 11 [-Wformat-truncation=] 2378 | snprintf(desc, desc_sz, "NCQ (depth %d/%d)%s", hdepth, | ^~ In function ‘ata_dev_config_ncq’, inlined from ‘ata_dev_config_lba’ at drivers/ata/libata-core.c:2649:8, inlined from ‘ata_dev_configure’ at drivers/ata/libata-core.c:2952:9: drivers/ata/libata-core.c:2378:41: note: directive argument in the range [1, 32] 2378 | snprintf(desc, desc_sz, "NCQ (depth %d/%d)%s", hdepth, | ^~~~~~~~~~~~~~~~~~~~~ drivers/ata/libata-core.c:2378:17: note: ‘snprintf’ output between 16 and 31 bytes into a destination of size 24 2378 | snprintf(desc, desc_sz, "NCQ (depth %d/%d)%s", hdepth, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 2379 | ddepth, aa_desc); | ~~~~~~~~~~~~~~~~ Avoid these warnings and the potential truncation by changing the size of the ncq_desc string to 32 characters. Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke --- drivers/ata/libata-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 18b2a0da9e54..2405ac8b53f0 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -2619,7 +2619,7 @@ static int ata_dev_config_lba(struct ata_device *dev) { const u16 *id = dev->id; const char *lba_desc; - char ncq_desc[24]; + char ncq_desc[32]; int ret; dev->flags |= ATA_DFLAG_LBA; From patchwork Fri Sep 15 08:14:55 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 723474 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0A542EE6442 for ; Fri, 15 Sep 2023 08:26:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232822AbjIOI0m (ORCPT ); Fri, 15 Sep 2023 04:26:42 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48786 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233116AbjIOI0g (ORCPT ); Fri, 15 Sep 2023 04:26:36 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E4C102D48; Fri, 15 Sep 2023 01:24:00 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1FA0CC4339A; Fri, 15 Sep 2023 08:15:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765730; bh=sRF3X29V6MuwiwvhAJLfp3Fi9J1PtT6yIsT/WvJryUE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Hw74qLH4w3z7GMUerY+J1DGxVEbhHzfXtrT94haKtYcrjHQGzZeQioYe/N6Qr8ueL AMvGBEKXzhx1WY4DVHhw97kg7sCWSQ0FlNas5PeqW7bWCPGXUvdJuG7TrDgHqq7Bme qCp2/iglqvkNWerN4b0/VvSzCAMO0IyXHpzZfOc2IKog1erzm4SCEDvInAZyKTd/U4 C3Bv52q2z5v0F7MVpuifBq3CM7Xi2cZUqxX4rDhzwOlN3RujWxyrdyCk8eyvJ3NVHA /gekTEXlaLxJ7qzPR+L4asDzojYBFC5Jo2A9Fct9uc84qJgfi6dSMDxxysIIXvHLkU mdINE+3pyXtVA== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 11/23] ata: libata-eh: Fix compilation warning in ata_eh_link_report() Date: Fri, 15 Sep 2023 17:14:55 +0900 Message-ID: <20230915081507.761711-12-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org The 6 bytes length of the tries_buf string in ata_eh_link_report() is too short and results in a gcc compilation warning with W-!: drivers/ata/libata-eh.c: In function ‘ata_eh_link_report’: drivers/ata/libata-eh.c:2371:59: warning: ‘%d’ directive output may be truncated writing between 1 and 11 bytes into a region of size 4 [-Wformat-truncation=] 2371 | snprintf(tries_buf, sizeof(tries_buf), " t%d", | ^~ drivers/ata/libata-eh.c:2371:56: note: directive argument in the range [-2147483648, 4] 2371 | snprintf(tries_buf, sizeof(tries_buf), " t%d", | ^~~~~~ drivers/ata/libata-eh.c:2371:17: note: ‘snprintf’ output between 4 and 14 bytes into a destination of size 6 2371 | snprintf(tries_buf, sizeof(tries_buf), " t%d", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 2372 | ap->eh_tries); | ~~~~~~~~~~~~~ Avoid this warning by increasing the string size to 16B. Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke --- drivers/ata/libata-eh.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index 43b0a1509548..03c45630a35b 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -2332,7 +2332,7 @@ static void ata_eh_link_report(struct ata_link *link) struct ata_eh_context *ehc = &link->eh_context; struct ata_queued_cmd *qc; const char *frozen, *desc; - char tries_buf[6] = ""; + char tries_buf[16] = ""; int tag, nr_failed = 0; if (ehc->i.flags & ATA_EHI_QUIET) From patchwork Fri Sep 15 08:14:56 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 723478 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id EE3E9EE643D for ; Fri, 15 Sep 2023 08:25:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232987AbjIOIZL (ORCPT ); Fri, 15 Sep 2023 04:25:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38910 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232873AbjIOIZK (ORCPT ); Fri, 15 Sep 2023 04:25:10 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 547AE3AB7; Fri, 15 Sep 2023 01:23:47 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E53D4C43397; Fri, 15 Sep 2023 08:15:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765732; bh=Rjmy6H7vMZEpzGgQrIn37F8mFUqbq5OVbs7tVk9/yf0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=aIz2bhCF1kUW1bCuezh7TEtxUYrPxYPNbkfGGh1YyYhqaF8wj6IHDVrm3VKyd6RY0 xahSbOIuUSdbqOVP9Rs/H2aVk4ypPvPYiXiKGOHtxTim6UPjNpmlGF+sqIIUT1WWI5 +kE/yPfakMOE+iI8o0G/9ot+lY94Nv36hC/A4m8LoyYxue8Z2UgNs04lq8sGWgiUZk r52o+NV2V0EoL3NMOhrf+bd4WeegvvRL1JmTBv3sLdVYW5JAQWP0qk+TEBj4tJqI4J VpeO1no8XSIN5chQhnzDJm9yUko7ccH9i7k5YmcMFp+lgFXfrko1GthRSrmtztxBOk QrGl/Hd4eJL7w== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 12/23] scsi: Remove scsi device no_start_on_resume flag Date: Fri, 15 Sep 2023 17:14:56 +0900 Message-ID: <20230915081507.761711-13-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org The scsi device flag no_start_on_resume is not set by any scsi low level driver. Remove it. This reverts the changes introduced by commit 0a8589055936 ("ata,scsi: do not issue START STOP UNIT on resume"). Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke Tested-by: Chia-Lin Kao (AceLan) --- drivers/scsi/sd.c | 7 ++----- include/scsi/scsi_device.h | 1 - 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index d86306d42445..49e9b4ce2e33 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -3886,11 +3886,8 @@ static int sd_resume(struct device *dev, bool runtime) if (!sd_do_start_stop(sdkp->device, runtime)) return 0; - if (!sdkp->device->no_start_on_resume) { - sd_printk(KERN_NOTICE, sdkp, "Starting disk\n"); - ret = sd_start_stop_device(sdkp, 1); - } - + sd_printk(KERN_NOTICE, sdkp, "Starting disk\n"); + ret = sd_start_stop_device(sdkp, 1); if (!ret) opal_unlock_from_suspend(sdkp->opal_dev); return ret; diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h index b7df1e6da969..8db0c88cf48e 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -195,7 +195,6 @@ struct scsi_device { unsigned allow_restart:1; /* issue START_UNIT in error handler */ unsigned manage_system_start_stop:1; /* Let HLD (sd) manage system start/stop */ unsigned manage_runtime_start_stop:1; /* Let HLD (sd) manage runtime start/stop */ - unsigned no_start_on_resume:1; /* Do not issue START_STOP_UNIT on resume */ unsigned start_stop_pwr_cond:1; /* Set power cond. in START_STOP_UNIT */ unsigned no_uld_attach:1; /* disable connecting to upper level drivers */ unsigned select_no_atn:1; From patchwork Fri Sep 15 08:14:57 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 724110 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0F2D8EE6447 for ; Fri, 15 Sep 2023 08:26:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233199AbjIOI1A (ORCPT ); Fri, 15 Sep 2023 04:27:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49284 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233117AbjIOI0t (ORCPT ); Fri, 15 Sep 2023 04:26:49 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 284672D7E; Fri, 15 Sep 2023 01:25:21 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B6553C433AB; Fri, 15 Sep 2023 08:15:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765734; bh=NmbYysMpdoTzQVRd2rI1yE6V1sWwMMvr3LB8iDhwBnc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=B+rrcoHfcDmjiwyhqTRyFMBqcZ0lVKVtBxY9xWod5D4d/LkPF2zHvYKBejuAUGmNQ jwQKxFcHRey62pXJdrsKTlAu1sgD/Prpl7kM1m91jpfk3xhlWECDybjpvq6eGILYQ1 Ju6xXucd6OQRTakFhDpCVztSBU8OHoEEM60HmEIGqya9Pdq6fzZnelBhjjRydUNqzK NWEn9bUcEdL4n73330XIbRx9YAuxTnUYjMW63DN3FUV7FnDcsz8r6K8bhQC23f4jXS dAE4oDH3ZAbcHd4K+LqbfnzJNrh5eEwOByhYtok7OiY/vu4Yf4q7jp5m7Q0JRDALja m5vFkICODlyew== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 13/23] ata: libata-scsi: Cleanup ata_scsi_start_stop_xlat() Date: Fri, 15 Sep 2023 17:14:57 +0900 Message-ID: <20230915081507.761711-14-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org Now that libata does its own internal device power mode management through libata EH, the scsi disk driver will not issue START STOP UNIT commands anymore. We can receive this command only from user passthrough operations. So there is no need to consider the system state and ATA port flags for suspend to translate the command. Since setting up the taskfile for the verify and standby immediate commands is the same as done in ata_dev_power_set_active() and ata_dev_power_set_standby(), factor out this code into the helper function ata_dev_power_init_tf() to simplify ata_scsi_start_stop_xlat() as well as ata_dev_power_set_active() and ata_dev_power_set_standby(). Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke Tested-by: Chia-Lin Kao (AceLan) --- drivers/ata/libata-core.c | 55 +++++++++++++++++++++++---------------- drivers/ata/libata-scsi.c | 53 +++++++------------------------------ drivers/ata/libata.h | 2 ++ 3 files changed, 44 insertions(+), 66 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 2405ac8b53f0..8d1949302a8a 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -1972,6 +1972,35 @@ int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class, return rc; } +bool ata_dev_power_init_tf(struct ata_device *dev, struct ata_taskfile *tf, + bool set_active) +{ + /* Only applies to ATA and ZAC devices */ + if (dev->class != ATA_DEV_ATA && dev->class != ATA_DEV_ZAC) + return false; + + ata_tf_init(dev, tf); + tf->flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR; + tf->protocol = ATA_PROT_NODATA; + + if (set_active) { + /* VERIFY for 1 sector at lba=0 */ + tf->command = ATA_CMD_VERIFY; + tf->nsect = 1; + if (dev->flags & ATA_DFLAG_LBA) { + tf->flags |= ATA_TFLAG_LBA; + tf->device |= ATA_LBA; + } else { + /* CHS */ + tf->lbal = 0x1; /* sect */ + } + } else { + tf->command = ATA_CMD_STANDBYNOW1; + } + + return true; +} + /** * ata_dev_power_set_standby - Set a device power mode to standby * @dev: target device @@ -1988,10 +2017,6 @@ void ata_dev_power_set_standby(struct ata_device *dev) struct ata_taskfile tf; unsigned int err_mask; - /* Issue STANDBY IMMEDIATE command only if supported by the device */ - if (dev->class != ATA_DEV_ATA && dev->class != ATA_DEV_ZAC) - return; - /* * Some odd clown BIOSes issue spindown on power off (ACPI S4 or S5) * causing some drives to spin up and down again. For these, do nothing @@ -2005,10 +2030,9 @@ void ata_dev_power_set_standby(struct ata_device *dev) system_entering_hibernation()) return; - ata_tf_init(dev, &tf); - tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR; - tf.protocol = ATA_PROT_NODATA; - tf.command = ATA_CMD_STANDBYNOW1; + /* Issue STANDBY IMMEDIATE command only if supported by the device */ + if (!ata_dev_power_init_tf(dev, &tf, false)) + return; ata_dev_notice(dev, "Entering standby power mode\n"); @@ -2038,22 +2062,9 @@ void ata_dev_power_set_active(struct ata_device *dev) * Issue READ VERIFY SECTORS command for 1 sector at lba=0 only * if supported by the device. */ - if (dev->class != ATA_DEV_ATA && dev->class != ATA_DEV_ZAC) + if (!ata_dev_power_init_tf(dev, &tf, true)) return; - ata_tf_init(dev, &tf); - tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR; - tf.protocol = ATA_PROT_NODATA; - tf.command = ATA_CMD_VERIFY; - tf.nsect = 1; - if (dev->flags & ATA_DFLAG_LBA) { - tf.flags |= ATA_TFLAG_LBA; - tf.device |= ATA_LBA; - } else { - /* CHS */ - tf.lbal = 0x1; /* sect */ - } - ata_dev_notice(dev, "Entering active power mode\n"); err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, 0); diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 6297f8c16a13..767e78fb9003 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -1206,7 +1206,6 @@ EXPORT_SYMBOL_GPL(ata_scsi_slave_destroy); static unsigned int ata_scsi_start_stop_xlat(struct ata_queued_cmd *qc) { struct scsi_cmnd *scmd = qc->scsicmd; - struct ata_taskfile *tf = &qc->tf; const u8 *cdb = scmd->cmnd; u16 fp; u8 bp = 0xff; @@ -1216,54 +1215,24 @@ static unsigned int ata_scsi_start_stop_xlat(struct ata_queued_cmd *qc) goto invalid_fld; } - tf->flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR; - tf->protocol = ATA_PROT_NODATA; - if (cdb[1] & 0x1) { - ; /* ignore IMMED bit, violates sat-r05 */ - } + /* LOEJ bit set not supported */ if (cdb[4] & 0x2) { fp = 4; bp = 1; - goto invalid_fld; /* LOEJ bit set not supported */ + goto invalid_fld; } + + /* Power conditions not supported */ if (((cdb[4] >> 4) & 0xf) != 0) { fp = 4; bp = 3; - goto invalid_fld; /* power conditions not supported */ + goto invalid_fld; } - if (cdb[4] & 0x1) { - tf->nsect = 1; /* 1 sector, lba=0 */ - - if (qc->dev->flags & ATA_DFLAG_LBA) { - tf->flags |= ATA_TFLAG_LBA; - - tf->lbah = 0x0; - tf->lbam = 0x0; - tf->lbal = 0x0; - tf->device |= ATA_LBA; - } else { - /* CHS */ - tf->lbal = 0x1; /* sect */ - tf->lbam = 0x0; /* cyl low */ - tf->lbah = 0x0; /* cyl high */ - } - - tf->command = ATA_CMD_VERIFY; /* READ VERIFY */ - } else { - /* Some odd clown BIOSen issue spindown on power off (ACPI S4 - * or S5) causing some drives to spin up and down again. - */ - if ((qc->ap->flags & ATA_FLAG_NO_POWEROFF_SPINDOWN) && - system_state == SYSTEM_POWER_OFF) - goto skip; - - if ((qc->ap->flags & ATA_FLAG_NO_HIBERNATE_SPINDOWN) && - system_entering_hibernation()) - goto skip; - - /* Issue ATA STANDBY IMMEDIATE command */ - tf->command = ATA_CMD_STANDBYNOW1; + /* Ignore IMMED bit (cdb[1] & 0x1), violates sat-r05 */ + if (!ata_dev_power_init_tf(qc->dev, &qc->tf, cdb[4] & 0x1)) { + ata_scsi_set_sense(qc->dev, scmd, ABORTED_COMMAND, 0, 0); + return 1; } /* @@ -1278,12 +1247,8 @@ static unsigned int ata_scsi_start_stop_xlat(struct ata_queued_cmd *qc) invalid_fld: ata_scsi_set_invalid_field(qc->dev, scmd, fp, bp); return 1; - skip: - scmd->result = SAM_STAT_GOOD; - return 1; } - /** * ata_scsi_flush_xlat - Translate SCSI SYNCHRONIZE CACHE command * @qc: Storage for translated ATA taskfile diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h index c57e094c3af9..af0e718f2b72 100644 --- a/drivers/ata/libata.h +++ b/drivers/ata/libata.h @@ -62,6 +62,8 @@ extern int ata_dev_reread_id(struct ata_device *dev, unsigned int readid_flags); extern int ata_dev_revalidate(struct ata_device *dev, unsigned int new_class, unsigned int readid_flags); extern int ata_dev_configure(struct ata_device *dev); +extern bool ata_dev_power_init_tf(struct ata_device *dev, + struct ata_taskfile *tf, bool set_active); extern void ata_dev_power_set_standby(struct ata_device *dev); extern void ata_dev_power_set_active(struct ata_device *dev); extern int sata_down_spd_limit(struct ata_link *link, u32 spd_limit); From patchwork Fri Sep 15 08:14:58 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 724114 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 45EE6EE6442 for ; Fri, 15 Sep 2023 08:25:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232537AbjIOIZ6 (ORCPT ); Fri, 15 Sep 2023 04:25:58 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59644 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232870AbjIOIZx (ORCPT ); Fri, 15 Sep 2023 04:25:53 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BC6B52D4A; Fri, 15 Sep 2023 01:23:40 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 88CF7C433AD; Fri, 15 Sep 2023 08:15:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765736; bh=KJoyWvVWhsIz4VVso+iFvB6Gta5M3O5M/mkqnLKFAMo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=fOHIwpDXHQeN0cKfb8Tg7KzXDw7VargqVzd76etLNp6rMvtOt4i2g3/YMGoAEumdY H9CGFmkxHKto93ya71zo1m/wxHsnmwBio2Bi+w+UeCKMozQqGLfVgcn2ML/BMjnQLC KWz1gWzzyDbyIxlYHPu68gwgO8tnU0bUTyoZsMqB+0BAYYs7qCaK3kdVW40nFzX0rB LU2ZdfhEGvvA9dwg96a2eD6LVbSGqWMWWOQPFVmlAUh/K8IodMXob//uCJfs49+CIG QqKGlHiR9UDK+qo/aJlEYJnwRysr0j4UKC6H7x0mzJ1qV32cocsDHmeenUKINj7Y70 U2yUuFUM77vIw== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 14/23] ata: libata-core: Synchronize ata_port_detach() with hotplug Date: Fri, 15 Sep 2023 17:14:58 +0900 Message-ID: <20230915081507.761711-15-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org The call to async_synchronize_cookie() to synchronize a port removal and hotplug probe is done in ata_host_detach() right before calling ata_port_detach(). Move this call at the beginning of ata_port_detach() to ensure that this operation is always synchronized with probe. Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke Tested-by: Chia-Lin Kao (AceLan) --- drivers/ata/libata-core.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 8d1949302a8a..9f05ad187c9e 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -6069,6 +6069,9 @@ static void ata_port_detach(struct ata_port *ap) struct ata_link *link; struct ata_device *dev; + /* Ensure ata_port probe has completed */ + async_synchronize_cookie(ap->cookie + 1); + /* Wait for any ongoing EH */ ata_port_wait_eh(ap); @@ -6133,11 +6136,8 @@ void ata_host_detach(struct ata_host *host) { int i; - for (i = 0; i < host->n_ports; i++) { - /* Ensure ata_port probe has completed */ - async_synchronize_cookie(host->ports[i]->cookie + 1); + for (i = 0; i < host->n_ports; i++) ata_port_detach(host->ports[i]); - } /* the host is dead now, dissociate ACPI */ ata_acpi_dissociate(host); From patchwork Fri Sep 15 08:14:59 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 724118 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5E766EE6446 for ; Fri, 15 Sep 2023 08:23:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232972AbjIOIXp (ORCPT ); Fri, 15 Sep 2023 04:23:45 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49204 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232922AbjIOIXo (ORCPT ); Fri, 15 Sep 2023 04:23:44 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 65CB13C25; Fri, 15 Sep 2023 01:22:39 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 5A171C433B6; Fri, 15 Sep 2023 08:15:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765737; bh=sIlHLfDcuVxZg7ZHLLtnzVuQhXg3ngq0RMU0PqQTYA4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=kz4I8HwXD1YPt7hHIJaEJM5YDSsxMkEeCZWp960rezHbwVOWYK01Ybp9HIWjLPZVJ wipO07vZFmpzZYbytBQkUE63xTV2+CTN3YypRjHvWOU0SfOnwNfBryFQgTctq25p4l HQcm5fXBP4n7yfgTgXHpOVwF19H9lG6UyxLOxHAQ661JIGieoXjCtEtA/6SfGqFgGz wS8kQOoEFLN1lbAIslJx6M0oaJ9PJxuylYcePQWDuJT2VrVdjHWc0Z0KqeYTrchKSM fVRyp4/+SffyAqXaKx9J6qmtn37J214Lrnq+bBTHwGEq3UK0Ri3OLgzFlUEbZlkQNU ZfjDRhX15a1yw== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 15/23] ata: libata-core: Detach a port devices on shutdown Date: Fri, 15 Sep 2023 17:14:59 +0900 Message-ID: <20230915081507.761711-16-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org Modify ata_pci_shutdown_one() to schedule EH to unload a port devices before freezing and thawing the port. This ensures that drives are cleanly disabled and transitioned to standby power mode when a PCI adapter is removed or the system is powered off. Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke Tested-by: Chia-Lin Kao (AceLan) --- drivers/ata/libata-core.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 9f05ad187c9e..7d110e5454bf 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -6168,10 +6168,24 @@ EXPORT_SYMBOL_GPL(ata_pci_remove_one); void ata_pci_shutdown_one(struct pci_dev *pdev) { struct ata_host *host = pci_get_drvdata(pdev); + struct ata_port *ap; + unsigned long flags; int i; + /* Tell EH to disable all devices */ for (i = 0; i < host->n_ports; i++) { - struct ata_port *ap = host->ports[i]; + ap = host->ports[i]; + spin_lock_irqsave(ap->lock, flags); + ap->pflags |= ATA_PFLAG_UNLOADING; + ata_port_schedule_eh(ap); + spin_unlock_irqrestore(ap->lock, flags); + } + + for (i = 0; i < host->n_ports; i++) { + ap = host->ports[i]; + + /* Wait for EH to complete before freezing the port */ + ata_port_wait_eh(ap); ap->pflags |= ATA_PFLAG_FROZEN; From patchwork Fri Sep 15 08:15:00 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 723471 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 74F36EE6442 for ; Fri, 15 Sep 2023 08:27:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233224AbjIOI1z (ORCPT ); Fri, 15 Sep 2023 04:27:55 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36450 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233325AbjIOI1h (ORCPT ); Fri, 15 Sep 2023 04:27:37 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 618BF4480; Fri, 15 Sep 2023 01:25:06 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2B225C433B7; Fri, 15 Sep 2023 08:15:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765739; bh=m2dwfkS3bdz+CQQ1TrydEsz2D94hyNQNquSF5RkvyaQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=KugjvbQO72Ep9p5SFsPC8CR/BVRsTY8nburOecmcth/t4qheMe5706/kGuYmprMaj LvC7fpqUTsvKSq5Nm88F1C3nGVr+Tw6QqKCts0NRRB6s4u8m2YE3pzy/hQ0EKlgZWR jczbhs67BuU+BIF5vF7IGYKzZyd9VA568QX1bCpBNjKkbFeHM5DurS4hPuT+PY7/8g Di6rXioa9jF1klwg1Vpat7f5faG4OUyrGSE52uqqFxJxLg+KdbnVWxZTCQTt8d3GZs qYt+KxC8bepABmnEwywMsxnbJnVP24AmNHUcLHmibAGyrVZthdV5ZwSe3AuYJSvbIh ko/Cbkf6VpZ5g== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 16/23] ata: libata-core: Remove ata_port_suspend_async() Date: Fri, 15 Sep 2023 17:15:00 +0900 Message-ID: <20230915081507.761711-17-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org ata_port_suspend_async() is only called by ata_sas_port_suspend(). Modify ata_port_suspend() with an additional bool argument indicating an asynchronous or synchronous suspend to allow removing that helper function. With this change, the variable ata_port_resume_ehi can also be removed and its value (ATA_EHI_XXX flags passed directly to ata_port_request_pm(). Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke Tested-by: Chia-Lin Kao (AceLan) --- drivers/ata/libata-core.c | 46 +++++++++++++++------------------------ 1 file changed, 17 insertions(+), 29 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 7d110e5454bf..cdfe71ae6a6c 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -5170,18 +5170,8 @@ static void ata_port_request_pm(struct ata_port *ap, pm_message_t mesg, ata_port_wait_eh(ap); } -/* - * On some hardware, device fails to respond after spun down for suspend. As - * the device won't be used before being resumed, we don't need to touch the - * device. Ask EH to skip the usual stuff and proceed directly to suspend. - * - * http://thread.gmane.org/gmane.linux.ide/46764 - */ -static const unsigned int ata_port_suspend_ehi = ATA_EHI_QUIET - | ATA_EHI_NO_AUTOPSY - | ATA_EHI_NO_RECOVERY; - -static void ata_port_suspend(struct ata_port *ap, pm_message_t mesg) +static void ata_port_suspend(struct ata_port *ap, pm_message_t mesg, + bool async) { /* * We are about to suspend the port, so we do not care about @@ -5191,20 +5181,18 @@ static void ata_port_suspend(struct ata_port *ap, pm_message_t mesg) */ cancel_delayed_work_sync(&ap->scsi_rescan_task); - ata_port_request_pm(ap, mesg, 0, ata_port_suspend_ehi, false); -} - -static void ata_port_suspend_async(struct ata_port *ap, pm_message_t mesg) -{ /* - * We are about to suspend the port, so we do not care about - * scsi_rescan_device() calls scheduled by previous resume operations. - * The next resume will schedule the rescan again. So cancel any rescan - * that is not done yet. + * On some hardware, device fails to respond after spun down for + * suspend. As the device wil not t be used until being resumed, we + * do not need to touch the device. Ask EH to skip the usual stuff + * and proceed directly to suspend. + * + * http://thread.gmane.org/gmane.linux.ide/46764 */ - cancel_delayed_work_sync(&ap->scsi_rescan_task); - - ata_port_request_pm(ap, mesg, 0, ata_port_suspend_ehi, true); + ata_port_request_pm(ap, mesg, 0, + ATA_EHI_QUIET | ATA_EHI_NO_AUTOPSY | + ATA_EHI_NO_RECOVERY, + async); } static int ata_port_pm_suspend(struct device *dev) @@ -5214,7 +5202,7 @@ static int ata_port_pm_suspend(struct device *dev) if (pm_runtime_suspended(dev)) return 0; - ata_port_suspend(ap, PMSG_SUSPEND); + ata_port_suspend(ap, PMSG_SUSPEND, false); return 0; } @@ -5225,13 +5213,13 @@ static int ata_port_pm_freeze(struct device *dev) if (pm_runtime_suspended(dev)) return 0; - ata_port_suspend(ap, PMSG_FREEZE); + ata_port_suspend(ap, PMSG_FREEZE, false); return 0; } static int ata_port_pm_poweroff(struct device *dev) { - ata_port_suspend(to_ata_port(dev), PMSG_HIBERNATE); + ata_port_suspend(to_ata_port(dev), PMSG_HIBERNATE, false); return 0; } @@ -5283,7 +5271,7 @@ static int ata_port_runtime_idle(struct device *dev) static int ata_port_runtime_suspend(struct device *dev) { - ata_port_suspend(to_ata_port(dev), PMSG_AUTO_SUSPEND); + ata_port_suspend(to_ata_port(dev), PMSG_AUTO_SUSPEND, false); return 0; } @@ -5313,7 +5301,7 @@ static const struct dev_pm_ops ata_port_pm_ops = { */ void ata_sas_port_suspend(struct ata_port *ap) { - ata_port_suspend_async(ap, PMSG_SUSPEND); + ata_port_suspend(ap, PMSG_SUSPEND, true); } EXPORT_SYMBOL_GPL(ata_sas_port_suspend); From patchwork Fri Sep 15 08:15:01 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 723472 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id ACD3EEE6442 for ; Fri, 15 Sep 2023 08:27:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233381AbjIOI1T (ORCPT ); Fri, 15 Sep 2023 04:27:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51116 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233255AbjIOI1O (ORCPT ); Fri, 15 Sep 2023 04:27:14 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6BFC03ABA; Fri, 15 Sep 2023 01:25:49 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id F0499C433B9; Fri, 15 Sep 2023 08:15:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765741; bh=LE4iH3AHIqlDXMEXWQkzC0CG96IX1D3ZEf3pbwhWkpw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=VasldwvkKi3pzaf13CnMRSjzqh/mrg8qHQ55PeUf+fz/oyuI4sLjBTUBFR8vRP6q9 4h+BD3N7gNC66f5jvZynF3Bsj+MVwTbsScglEdSUVfh/oxat6qrgU+otmLi4JJZL4J 5i1uRQZ0LGM08USpmalylzkIgKnKRGsjUQY0QofNqzEud1/sT+EOCYO5fxObOeOqV2 OAFAmrCzzEqkCqwfdF9ht+qs2/N/2518/Y3UtJ5XFUZgXrwdsusKhi3oj4tcmwjWR0 z3c6zAhsVHM+YFbIJKvpur/CRZyRXgHvSs0KGJlsYqVM74+MzTdlztqMB+Lt201LEl d2KSv7YYszRSA== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 17/23] ata: libata-core: Remove ata_port_resume_async() Date: Fri, 15 Sep 2023 17:15:01 +0900 Message-ID: <20230915081507.761711-18-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org Remove ata_port_resume_async() and replace it with a modified ata_port_resume() taking an additional bool argument indicating if ata EH resume operation should be executed synchronously or asynchronously. With this change, the variable ata_port_resume_ehi is not longer necessary and its value (ATA_EHI_XXX flags) passed directly to ata_port_request_pm(). Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke Tested-by: Chia-Lin Kao (AceLan) --- drivers/ata/libata-core.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index cdfe71ae6a6c..a4389dd807e5 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -5223,22 +5223,17 @@ static int ata_port_pm_poweroff(struct device *dev) return 0; } -static const unsigned int ata_port_resume_ehi = ATA_EHI_NO_AUTOPSY - | ATA_EHI_QUIET; - -static void ata_port_resume(struct ata_port *ap, pm_message_t mesg) +static void ata_port_resume(struct ata_port *ap, pm_message_t mesg, + bool async) { - ata_port_request_pm(ap, mesg, ATA_EH_RESET, ata_port_resume_ehi, false); -} - -static void ata_port_resume_async(struct ata_port *ap, pm_message_t mesg) -{ - ata_port_request_pm(ap, mesg, ATA_EH_RESET, ata_port_resume_ehi, true); + ata_port_request_pm(ap, mesg, ATA_EH_RESET, + ATA_EHI_NO_AUTOPSY | ATA_EHI_QUIET, + async); } static int ata_port_pm_resume(struct device *dev) { - ata_port_resume_async(to_ata_port(dev), PMSG_RESUME); + ata_port_resume(to_ata_port(dev), PMSG_RESUME, true); pm_runtime_disable(dev); pm_runtime_set_active(dev); pm_runtime_enable(dev); @@ -5277,7 +5272,7 @@ static int ata_port_runtime_suspend(struct device *dev) static int ata_port_runtime_resume(struct device *dev) { - ata_port_resume(to_ata_port(dev), PMSG_AUTO_RESUME); + ata_port_resume(to_ata_port(dev), PMSG_AUTO_RESUME, false); return 0; } @@ -5307,7 +5302,7 @@ EXPORT_SYMBOL_GPL(ata_sas_port_suspend); void ata_sas_port_resume(struct ata_port *ap) { - ata_port_resume_async(ap, PMSG_RESUME); + ata_port_resume(ap, PMSG_RESUME, true); } EXPORT_SYMBOL_GPL(ata_sas_port_resume); From patchwork Fri Sep 15 08:15:02 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 723473 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 65CB7EE6446 for ; Fri, 15 Sep 2023 08:26:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233141AbjIOI05 (ORCPT ); Fri, 15 Sep 2023 04:26:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49282 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233116AbjIOI0t (ORCPT ); Fri, 15 Sep 2023 04:26:49 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E8CAF3AA5; Fri, 15 Sep 2023 01:25:20 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id C32BAC433B8; Fri, 15 Sep 2023 08:15:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765743; bh=0GOi0mloXAEJIC+S6NGXQSMf1o5GU58qEdFx1YeNYX0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=WdmunzguMNXTPhYSlEoHlAMqPrzwRKDNUeWTiYFWy2kNbvyZ/dG7YWWNv/YaV9PcZ Kde3++KNSfxVoZICOYJ3KBkSjKRejHslgiLy2TgqX3p48esXDSWNcgAgVOHSHu7h2y OHOJ5mALKtqjhIfZ1UM1ncsV1oCleoUYQkIdVjoDlcBiRCS6pSOjiZEyFb6wky94fi yLxK4DiNHTk3gtao1FT+uPdPcO/16kq0w9LNRHzAopkqvMXqGxbGG4QWVXnusX2Ve5 rJPEB0a/z3i/Vg7Cf2M/5tiCZsb/WCEqv+VGZVogkmggYndRrLZtLTf4rkXk56csfq 8ZlikM/8czz2A== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 18/23] ata: libata-core: Do not poweroff runtime suspended ports Date: Fri, 15 Sep 2023 17:15:02 +0900 Message-ID: <20230915081507.761711-19-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org When powering off, there is no need to suspend a port that has already been runtime suspended. Skip the EH PM request in ata_port_pm_poweroff() in this case. Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke Tested-by: Chia-Lin Kao (AceLan) --- drivers/ata/libata-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index a4389dd807e5..c3adaa01cbe3 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -5219,7 +5219,8 @@ static int ata_port_pm_freeze(struct device *dev) static int ata_port_pm_poweroff(struct device *dev) { - ata_port_suspend(to_ata_port(dev), PMSG_HIBERNATE, false); + if (!pm_runtime_suspended(dev)) + ata_port_suspend(to_ata_port(dev), PMSG_HIBERNATE, false); return 0; } From patchwork Fri Sep 15 08:15:03 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 723475 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6CED6EE6442 for ; Fri, 15 Sep 2023 08:26:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232963AbjIOI0S (ORCPT ); Fri, 15 Sep 2023 04:26:18 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42288 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233066AbjIOI0O (ORCPT ); Fri, 15 Sep 2023 04:26:14 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6AEE03AB2; Fri, 15 Sep 2023 01:24:55 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 97F83C433BA; Fri, 15 Sep 2023 08:15:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765745; bh=MGyUTr147OXTgvJoJwfeJdXivkMDvvXqnQv8JHunP8o=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=LQGtx68aOodF4YT/Iknf367EzGBH2L8fIAvqeAgpJpu0JPhC+EbxksuBf1gsAimD9 IGJIYTupCgBc6/pR63RKGk+cK6UCG65wsWA1qwPEYb5LxATIimd7gBu4N2bkK8l5KD nOlBGExoYpVRGroZdiSmKHfAtbGrg8PDb3Cja+Veis19tcW4/fGJ0mYYUQMrD0qXf3 LLh0Npb/5M29zC9JZoir2TrdIYm0SmPqJMpl9FhQmgUmPQhAXmrqH4+PDPk5FnbNPl jSX98Z5mowShMKDCt/nsVwvuvhYshr3OoWOU7s6vgwyGKpyI4PYaQzCTD8zgQxUQCP D9Wr8yZ0hiLLQ== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 19/23] ata: libata-core: Do not resume runtime suspended ports Date: Fri, 15 Sep 2023 17:15:03 +0900 Message-ID: <20230915081507.761711-20-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org The scsi disk driver does not resume disks that have been runtime suspended by the user. To be consistent with this behavior, do the same for ata ports and skip the PM request in ata_port_pm_resume() if the port was already runtime suspended. With this change, it is no longer necessary to for the PM state of the port to ACTIVE as the PM core code will take care of that when handling runtime resume. Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke Tested-by: Chia-Lin Kao (AceLan) --- drivers/ata/libata-core.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index c3adaa01cbe3..080b451e7ddd 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -5234,10 +5234,8 @@ static void ata_port_resume(struct ata_port *ap, pm_message_t mesg, static int ata_port_pm_resume(struct device *dev) { - ata_port_resume(to_ata_port(dev), PMSG_RESUME, true); - pm_runtime_disable(dev); - pm_runtime_set_active(dev); - pm_runtime_enable(dev); + if (!pm_runtime_suspended(dev)) + ata_port_resume(to_ata_port(dev), PMSG_RESUME, true); return 0; } From patchwork Fri Sep 15 08:15:04 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 723476 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E8D06EE6447 for ; Fri, 15 Sep 2023 08:25:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233060AbjIOIZ7 (ORCPT ); Fri, 15 Sep 2023 04:25:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59658 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232909AbjIOIZy (ORCPT ); Fri, 15 Sep 2023 04:25:54 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 006073C32; Fri, 15 Sep 2023 01:23:40 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6C609C433BB; Fri, 15 Sep 2023 08:15:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765746; bh=jOxWM3EFcUw8KH8RxeHZOiDuMyEdT2/JG+xBMI/uyoE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=mWGkZpwhbR6kLdzi6bVczTGg7RNcd1UpXHydB8CRhsJzQOESC1KXOIrrTPqP0DpT2 JZU+YeYTMLl7DvA5kT1vs2KHvoSatdlxY1diThcRxcor/vjLzswhB9s+44tOQWeI7A j5XeiR0DeTXYG3E2Y5kZxIYYuJSInjvKxfqcVsMREleAKQ/ciWJ/hQr4h4aMLgjY59 Shv/EyQwletwBDxSdqEILjGhC5av7VcH7gUqdrx7roRMbooKRBHwj443jAiaeWLHvU yHccSdR3etS6FQ55X9+tkKiQwhzq+6SLt9PvGAvNgiga/z28ab8dQagQheVxqCYohv /zh11Vq7R9ZfQ== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 20/23] ata: libata-sata: Improve ata_sas_slave_configure() Date: Fri, 15 Sep 2023 17:15:04 +0900 Message-ID: <20230915081507.761711-21-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org Change ata_sas_slave_configure() to return the return value of ata_scsi_dev_config() to ensure that any error from that function is propagated to libsas. Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke Reviewed-by: John Garry Tested-by: Chia-Lin Kao (AceLan) --- drivers/ata/libata-sata.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c index 5d31c08be013..0748e9ea4f5f 100644 --- a/drivers/ata/libata-sata.c +++ b/drivers/ata/libata-sata.c @@ -1169,8 +1169,8 @@ EXPORT_SYMBOL_GPL(ata_sas_tport_delete); int ata_sas_slave_configure(struct scsi_device *sdev, struct ata_port *ap) { ata_scsi_sdev_config(sdev); - ata_scsi_dev_config(sdev, ap->link.device); - return 0; + + return ata_scsi_dev_config(sdev, ap->link.device); } EXPORT_SYMBOL_GPL(ata_sas_slave_configure); From patchwork Fri Sep 15 08:15:05 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 723481 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0869BEE6448 for ; Fri, 15 Sep 2023 08:23:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232925AbjIOIXq (ORCPT ); Fri, 15 Sep 2023 04:23:46 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38990 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232932AbjIOIXo (ORCPT ); Fri, 15 Sep 2023 04:23:44 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 00C793AA3; Fri, 15 Sep 2023 01:22:38 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3DFADC433BF; Fri, 15 Sep 2023 08:15:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765748; bh=PmA2bDpqIVG0CeoWOBb3D16WnQQLzPKVzEaa31hay1A=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=JVpXXK79LYaTbQ6gVcaSz4irfxL0ozCfPQL8FJHloNk8axDGGLATcHCKzweE8xekS ePHRwHrbw5t0DJWYCsiWlAFdA9Lk/buxatN7W3DiUX+k965vL2t/N8cmRAW31lziiP 02DgE3n1KGTTzStu1/2ecrK/lq0+TGRMEyrQo3thdmuQcg+qmdd4AZ4GPVkRhWH8Gg bbIF52Ki4NqW+Zq7NjTzBxWjBOgD/iIPGWv5GNbmdBB6DuQ09sNBJjOzeuHtevTmtl cT7cdpkMle6Md0zJWc7hQxX0iu3Q+EjH6Tq1/03U8LQ0CK1pTKJWMdhClZjl+xQt+n 0ypkSsI9QqXEQ== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 21/23] ata: libata-eh: Improve reset error messages Date: Fri, 15 Sep 2023 17:15:05 +0900 Message-ID: <20230915081507.761711-22-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org Some drives are really slow to spinup on resume, resulting is a very slow response to COMRESET and to error messages such as: ata1: COMRESET failed (errno=-16) ata1: link is slow to respond, please be patient (ready=0) ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300) ata1.00: configured for UDMA/133 Given that the slowness of the response is indicated with the message "link is slow to respond..." and that resets are retried until the device is detected as online after up to 1min (ata_eh_reset_timeouts), there is no point in printing the "COMRESET failed" error message. Let's not scare the user with non fatal errors and only warn about reset failures in ata_eh_reset() when all reset retries have been exhausted. Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke Tested-by: Chia-Lin Kao (AceLan) --- drivers/ata/libata-eh.c | 2 ++ drivers/ata/libata-sata.c | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index 03c45630a35b..2b933e7a357a 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -2919,6 +2919,8 @@ int ata_eh_reset(struct ata_link *link, int classify, */ if (ata_is_host_link(link)) ata_eh_thaw_port(ap); + ata_link_warn(link, "%s failed\n", + reset == hardreset ? "hardreset" : "softreset"); goto out; } diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c index 0748e9ea4f5f..00674aae1696 100644 --- a/drivers/ata/libata-sata.c +++ b/drivers/ata/libata-sata.c @@ -608,7 +608,6 @@ int sata_link_hardreset(struct ata_link *link, const unsigned int *timing, /* online is set iff link is online && reset succeeded */ if (online) *online = false; - ata_link_err(link, "COMRESET failed (errno=%d)\n", rc); } return rc; } From patchwork Fri Sep 15 08:15:06 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 724111 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 441D7EE6446 for ; Fri, 15 Sep 2023 08:26:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233005AbjIOI0l (ORCPT ); Fri, 15 Sep 2023 04:26:41 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54182 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233111AbjIOI0f (ORCPT ); Fri, 15 Sep 2023 04:26:35 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E2F652719; Fri, 15 Sep 2023 01:24:00 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 10360C433BC; Fri, 15 Sep 2023 08:15:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765750; bh=anjHLV/xNgtacDAnwtkR64IDyr8df5COra7Y6hLZ6M8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=aj3x5tBNBtKSvEqKco9BCJf/BZVIPASbeqzNZr4CM1+DA8L7yfc38wUaFyF5kZK25 WPRk5uuh0dh6FK1S0ZVHFxIZO6k0O+Vc/Q1+VcVqGjnVRP0lkFyNJrzh2qkPgzgt1O 3r9CdNNa2Up69eruc4eeP2fSqU7/bjpMIB8Oj1AdWxvDT18RWLRZGbjm36OSZvlSeX qif55vIvblJUJuyVGuSEkmjzk0PmKGvGlvMMoAS0GfAmXGszJjz2/60MlFc1BtDA+/ XUuq6IqxZtLHyHv4dj5X2xxDtga7kCac8xPV2MsYLhjLO+kwVWvoo/+eZcWEaeULVS JEuPqDKnrI95g== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 22/23] ata: libata-eh: Reduce "disable device" message verbosity Date: Fri, 15 Sep 2023 17:15:06 +0900 Message-ID: <20230915081507.761711-23-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org There is no point in warning about a device being disabled when we expect it to be, that is, on suspend, shutdown or when detaching the device. Suppress the message "disable device" for these cases by introducing the EH static function ata_eh_dev_disable() and by using it in ata_eh_unload() and ata_eh_detach_dev(). ata_dev_disable() code is modified to call this new function after printing the "disable device" message. Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke Tested-by: Chia-Lin Kao (AceLan) --- drivers/ata/libata-eh.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index 2b933e7a357a..9cad16bcbebc 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -494,6 +494,18 @@ void ata_eh_release(struct ata_port *ap) mutex_unlock(&ap->host->eh_mutex); } +static void ata_eh_dev_disable(struct ata_device *dev) +{ + ata_acpi_on_disable(dev); + ata_down_xfermask_limit(dev, ATA_DNXFER_FORCE_PIO0 | ATA_DNXFER_QUIET); + dev->class++; + + /* From now till the next successful probe, ering is used to + * track probe failures. Clear accumulated device error info. + */ + ata_ering_clear(&dev->ering); +} + static void ata_eh_unload(struct ata_port *ap) { struct ata_link *link; @@ -517,8 +529,8 @@ static void ata_eh_unload(struct ata_port *ap) */ ata_for_each_link(link, ap, PMP_FIRST) { sata_scr_write(link, SCR_CONTROL, link->saved_scontrol & 0xff0); - ata_for_each_dev(dev, link, ALL) - ata_dev_disable(dev); + ata_for_each_dev(dev, link, ENABLED) + ata_eh_dev_disable(dev); } /* freeze and set UNLOADED */ @@ -1211,14 +1223,8 @@ void ata_dev_disable(struct ata_device *dev) return; ata_dev_warn(dev, "disable device\n"); - ata_acpi_on_disable(dev); - ata_down_xfermask_limit(dev, ATA_DNXFER_FORCE_PIO0 | ATA_DNXFER_QUIET); - dev->class++; - /* From now till the next successful probe, ering is used to - * track probe failures. Clear accumulated device error info. - */ - ata_ering_clear(&dev->ering); + ata_eh_dev_disable(dev); } EXPORT_SYMBOL_GPL(ata_dev_disable); @@ -1240,12 +1246,12 @@ void ata_eh_detach_dev(struct ata_device *dev) /* * If the device is still enabled, transition it to standby power mode - * (i.e. spin down HDDs). + * (i.e. spin down HDDs) and disable it. */ - if (ata_dev_enabled(dev)) + if (ata_dev_enabled(dev)) { ata_dev_power_set_standby(dev); - - ata_dev_disable(dev); + ata_eh_dev_disable(dev); + } spin_lock_irqsave(ap->lock, flags); From patchwork Fri Sep 15 08:15:07 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 724117 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 78997EE6440 for ; Fri, 15 Sep 2023 08:24:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232932AbjIOIYD (ORCPT ); Fri, 15 Sep 2023 04:24:03 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49302 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233049AbjIOIYA (ORCPT ); Fri, 15 Sep 2023 04:24:00 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7D9414499; Fri, 15 Sep 2023 01:22:37 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id D5484C433BD; Fri, 15 Sep 2023 08:15:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694765752; bh=Ny5m9BSnjoocQpihQVcwfWavMkW7NZYngxvodx+Ozj8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=N0HleOmVsHL0jDai/dBS98DJr9mI6+qZZ686NrsePnipY0EflQnXqcwrqSlh9FrMC S55tB7KxeT1M6za8JlJeABwUpSfm3Y2gdIYMBfvyajKfZR0N9aCXgzdHv6uzUayncD 7PDAHxW7YmECVdZDzc7sG2uRuW1YExcxJ90X28gfMLnFa9BYSytSlgrgAjjdaLxM2S xBorVCzuyrrqV1C7W+TG6ljBIn4IiS/Az/NxWLzN1k7Nx8C6taZbnxmxDitRnZmALx bymlJWFBJH1EBbhZnTKHzfdyQ1AxxTF+kQIpUtJejriyvsTDApmXvU756sm3L5NwAS qq54DOWgiJ1Jw== From: Damien Le Moal To: linux-ide@vger.kernel.org Cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , John Garry , Rodrigo Vivi , Paul Ausbeck , Kai-Heng Feng , Joe Breuer , Geert Uytterhoeven , Chia-Lin Kao Subject: [PATCH v3 23/23] ata: libata: Cleanup inline DMA helper functions Date: Fri, 15 Sep 2023 17:15:07 +0900 Message-ID: <20230915081507.761711-24-dlemoal@kernel.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230915081507.761711-1-dlemoal@kernel.org> References: <20230915081507.761711-1-dlemoal@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org Simplify the inline DMA helper functions ata_using_mwdma(), ata_using_udma() and ata_dma_enabled() to directly return as a boolean the result of their test condition. Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke Tested-by: Chia-Lin Kao (AceLan) --- include/linux/libata.h | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/include/linux/libata.h b/include/linux/libata.h index 6593c79b7290..f48fe27dae94 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -1878,23 +1878,21 @@ static inline unsigned long ata_deadline(unsigned long from_jiffies, change in future hardware and specs, secondly 0xFF means 'no DMA' but is > UDMA_0. Dyma ddreigiau */ -static inline int ata_using_mwdma(struct ata_device *adev) +static inline bool ata_using_mwdma(struct ata_device *adev) { - if (adev->dma_mode >= XFER_MW_DMA_0 && adev->dma_mode <= XFER_MW_DMA_4) - return 1; - return 0; + return adev->dma_mode >= XFER_MW_DMA_0 && + adev->dma_mode <= XFER_MW_DMA_4; } -static inline int ata_using_udma(struct ata_device *adev) +static inline bool ata_using_udma(struct ata_device *adev) { - if (adev->dma_mode >= XFER_UDMA_0 && adev->dma_mode <= XFER_UDMA_7) - return 1; - return 0; + return adev->dma_mode >= XFER_UDMA_0 && + adev->dma_mode <= XFER_UDMA_7; } -static inline int ata_dma_enabled(struct ata_device *adev) +static inline bool ata_dma_enabled(struct ata_device *adev) { - return (adev->dma_mode == 0xFF ? 0 : 1); + return adev->dma_mode != 0xFF; } /**************************************************************************