Re: [PATCH v6 09/19] scsi: allow enabling and disabling command duration limits

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

 



On Tue, Apr 11, 2023 at 01:59:44PM +0200, Niklas Cassel wrote:
> On Tue, Apr 11, 2023 at 04:38:48PM +0900, Damien Le Moal wrote:
> > On 4/11/23 16:23, Christoph Hellwig wrote:
> > > On Tue, Apr 11, 2023 at 04:09:34PM +0900, Damien Le Moal wrote:
> > >> But yes, I guess we could just unconditionally enable CDL for ATA on device scan
> > >> to be on par with scsi, which has CDL always enabled.
> > > 
> > > I'd prefer that.  With a module option to not enable it just to be
> > > safe.
> > 
> > Then it may be better to move the cdl_enable attribute store definition for ATA
> > devices to libata. That would be less messy that way. Let me see if that can be
> > done cleanly.
> 
> I don't think that the SCSI mode select can just be replaced with simple
> SET FEATURES in libata.
> 
> If we move the SET FEATURES call to a function in libata, and then use a
> function pointer in the scsi_host_template, and let only libata set this
> function pointer (similar to e.g. how the queue_depth sysfs attribute works),
> then the code will no longer work for SATA devices connected to a SAS HBA.
> 
> 
> 
> The current code simply checks if VPD page89 (the ATA Information VPD
> page - which is defined in the SCSI to ATA Translation (SAT) standard)
> exists. This page (and thus the sdev->vpd_pg89 pointer) will only exist
> if either:
> 1) An ATA device is connected to a SATA controller. This page will then
> be implemented by libata.
> 2) An ATA device is connected to a SAS HBA. The SAS HB will then provide
> this page. (The page will not exist for SCSI devices connected to the
> same SAS HBA.)
> 
> For case 1) with the current code, scsi.c will call scsi_mode_select()
> which will be translated by libata before being sent down to the device.
> 
> For case 2) with the current code, scsi.c will send down a SCSI mode
> select to the SAS HBA, and the SAS HBA will be responsible for translating
> the command before sending it to the device.
> 
> So I actually think that the current way to check if vpd page89 exists
> is probably the "cleanest" solution that I can think of.
> 
> If you have a better suggestion that will work for both case 1) and
> case 2), I will be happy to change the code accordingly.


In addition to:
https://github.com/torvalds/linux/blame/v6.3-rc6/drivers/scsi/sd.c#L3066-L3074

checking for the existence of VPD page89 is also currently done by e.g.:
https://github.com/torvalds/linux/blob/v6.3-rc6/drivers/scsi/mpt3sas/mpt3sas_scsih.c#L12607
and
https://github.com/torvalds/linux/blob/v6.3-rc6/drivers/hwmon/drivetemp.c#L340


Kind regards,
Niklas



[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux