| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 27 Oct 2022 11:51:34 +0200
From: Hannes Reinecke <hare@...e.de>
To: Damien Le Moal <damien.lemoal@...nsource.wdc.com>,
John Garry <john.garry@...wei.com>, axboe@...nel.dk,
jejb@...ux.ibm.com, martin.petersen@...cle.com,
jinpu.wang@...ud.ionos.com, bvanassche@....org, hch@....de,
ming.lei@...hat.com, niklas.cassel@....com
Cc: linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-ide@...r.kernel.org, linux-scsi@...r.kernel.org,
linuxarm@...wei.com
Subject: Re: [PATCH RFC v3 16/22] ata: libata-scsi: Allocate sdev early in
port probe
On 10/27/22 11:16, Damien Le Moal wrote:
> On 10/27/22 17:11, Hannes Reinecke wrote:
>> On 10/27/22 03:34, Damien Le Moal wrote:
>>> On 10/25/22 19:18, John Garry wrote:
>>>> Currently the per-ata device sdev is allocated as part of the scsi
>>>> target
>>>> scan, which is after the ata port probe.
>>>>
>>>> However it is useful to have the sdev available in the port probe. As an
>>>> example of an advantage, if the request queue is available in the probe
>>>> (which it would be if the sdev is available), then it is possible to use
>>>> a SCSI cmnd for ATA internal commands. The benefit of this is then we
>>>> can
>>>> put the ATA qc structure in the SCSI cmnd private data. It will also be
>>>> useful if we want to send ATA internal commands as requests.
>>>>
>>>> Export scsi_target_reap() so that it can be used to put the extra
>>>> reference we get when allocating the sdev.
>>>>
>>>> Signed-off-by: John Garry <john.garry@...wei.com>
>>>> ---
>>>> drivers/ata/libata-eh.c | 1 +
>>>> drivers/ata/libata-scsi.c | 23 +++++++++--------------
>>>> drivers/scsi/scsi_scan.c | 1 +
>>>> 3 files changed, 11 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
>>>> index 08e11bc312c2..1ed5b1b64792 100644
>>>> --- a/drivers/ata/libata-eh.c
>>>> +++ b/drivers/ata/libata-eh.c
>>>> @@ -3446,6 +3446,7 @@ static int ata_eh_schedule_probe(struct
>>>> ata_device *dev)
>>>> ata_eh_detach_dev(dev);
>>>> ata_dev_init(dev);
>>>> + ata_scsi_setup_sdev(dev);
>>>> ehc->did_probe_mask |= (1 << dev->devno);
>>>> ehc->i.action |= ATA_EH_RESET;
>>>> ehc->saved_xfer_mode[dev->devno] = 0;
>>>> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
>>>> index efdba852e363..476e0ef4bd29 100644
>>>> --- a/drivers/ata/libata-scsi.c
>>>> +++ b/drivers/ata/libata-scsi.c
>>>> @@ -1109,7 +1109,12 @@ int ata_scsi_dev_config(struct scsi_device
>>>> *sdev, struct ata_device *dev)
>>>> if (dev->flags & ATA_DFLAG_TRUSTED)
>>>> sdev->security_supported = 1;
>>>> - dev->sdev = sdev;
>>>> + /*
>>>> + * Put extra reference which we get when allocating the starget
>>>> + * initially
>>>> + */
>>>> + scsi_target_reap(scsi_target(sdev));
>>>> +
>>>> return 0;
>>>> }
>>>> @@ -4289,26 +4294,16 @@ void ata_scsi_scan_host(struct ata_port
>>>> *ap, int sync)
>>>> repeat:
>>>> ata_for_each_link(link, ap, EDGE) {
>>>> ata_for_each_dev(dev, link, ENABLED) {
>>>> - struct scsi_device *sdev;
>>>> + struct Scsi_Host *shost = ap->scsi_host;
>>>> int channel = 0, id = 0;
>>>> - if (dev->sdev)
>>>> - continue;
>>>> -
>>>> if (ata_is_host_link(link))
>>>> id = dev->devno;
>>>> else
>>>> channel = link->pmp;
>>>> - sdev = __scsi_add_device(ap->scsi_host, channel, id, 0,
>>>> - NULL);
>>>> - if (!IS_ERR(sdev)) {
>>>> - dev->sdev = sdev;
>>>> - ata_scsi_assign_ofnode(dev, ap);
>>>
>>> Is there something equivalent to what this function does inside
>>> scsi_scan_target() ? I had a quick look but did not see anything...
>>>
>> Typically, the SCSI layer has two ways of scanning.
>> One it the old-style serial scanning (originating in the old SCSI
>> parallel model):
>> The scanning code will blindly scan _all_ devices up to max_luns, and
>> attach every device for which the scanning code returns 'OK'.
>> The other one is to issue REPORT_LUNS and scan all LUNs returned there.
>>
>> Mapped to libata we would need to figure out a stable SCSI enumeration,
>> given that we have PMP and slave devices to content with.
>> To my knowledge we have ATA ports, each can have either a 'master' and
>> 'slave' device, _or_ it be a PMP port when it can support up to 16
>> devices, right?
>
> yes
>
>> Point being, master/slave and PMP are exclusive, right?
>
> Never heard of pmp with ide cable :)
>
See?
>> So we can make the master as LUN 0, and the slave as LUN 1.
>
> Yes, but isn't that a little wrong ? That would assume that the ata port
> is the device and the ata devices the luns of that device. But beside
> the "link busy" stuff that needs to be dealt with, master and slave are
> independent devices, unlike LUNs. No ?
> Well; technically, yes.
But we already enumerate the ata ports (which is typically done by the
hardware/PCI layer etc), and if we were try to model slave devices as
independent ports we would either have to insert a numbering (awkward)
or add a number at the en (even more awkward).
And the one key takeaway from the 'multiple actuators' discussion is
that LUNs _are_ independent (cf all the hoops they had to jump through
to define a command spanning several LUNs ...)(which, incidentally, we
could leverage here, too ...), and the target port really only serves as
an enumeration thingie to address the LUNs.
So we _could_ map the master device on LUN 0 and the slave device on LUN
1 with no loss of functionality, _but_ enable a consistent SCSI enumeration.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@...e.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman
Powered by blists - more mailing lists