Re: [PATCH 5/8] scsi: remove the unchecked_isa_dma flag

Re: [PATCH 5/8] scsi: remove the unchecked_isa_dma flag

[Steffen Maier]

On 3/31/21 9:29 AM, Christoph Hellwig wrote:
> Remove the unchecked_isa_dma now that all users are gone.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> ---
>   Documentation/scsi/scsi_mid_low_api.rst |  4 --
>   drivers/scsi/esas2r/esas2r_main.c       |  1 -
>   drivers/scsi/hosts.c                    |  7 +---
>   drivers/scsi/scsi_debugfs.c             |  1 -
>   drivers/scsi/scsi_lib.c                 | 52 +++----------------------
>   drivers/scsi/scsi_scan.c                |  6 +--
>   drivers/scsi/scsi_sysfs.c               |  2 -
>   drivers/scsi/sg.c                       | 10 +----
>   drivers/scsi/sr_ioctl.c                 | 12 ++----
>   drivers/scsi/st.c                       | 20 ++++------
>   drivers/scsi/st.h                       |  2 -
>   include/scsi/scsi_cmnd.h                |  7 ++--
>   include/scsi/scsi_host.h                |  6 ---
>   13 files changed, 25 insertions(+), 105 deletions(-)


> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index b6378c8ca783ea..b71ea1a69c8b60 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -373,7 +373,6 @@ shost_rd_attr(cmd_per_lun, "%hd\n");
>   shost_rd_attr(can_queue, "%d\n");
>   shost_rd_attr(sg_tablesize, "%hu\n");
>   shost_rd_attr(sg_prot_tablesize, "%hu\n");
> -shost_rd_attr(unchecked_isa_dma, "%d\n");
>   shost_rd_attr(prot_capabilities, "%u\n");
>   shost_rd_attr(prot_guard_type, "%hd\n");
>   shost_rd_attr2(proc_name, hostt->proc_name, "%s\n");
> @@ -411,7 +410,6 @@ static struct attribute *scsi_sysfs_shost_attrs[] = {
>   	&dev_attr_can_queue.attr,
>   	&dev_attr_sg_tablesize.attr,
>   	&dev_attr_sg_prot_tablesize.attr,
> -	&dev_attr_unchecked_isa_dma.attr,
>   	&dev_attr_proc_name.attr,
>   	&dev_attr_scan.attr,
>   	&dev_attr_hstate.attr,

Is it OK to remove a long standing sysfs attribute?
Was there any deprecation phase?

I just noticed it in our CI reporting fails due to this change since at least 
linux-next 20210409. Suppose it came via 
https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git/commit/?h=for-5.13/block&id=aaff5ebaa2694f283b7d07fdd55fb287ffc4f1e9

Wanted to ask before I adapt the test cases.

Cf. 
https://lore.kernel.org/linux-scsi/20080224233529.456981B4183@basil.firstfloor.org/T/#u
"An alternative would be to always make it report 0 now."

-- 
Mit freundlichen Gruessen / Kind regards
Steffen Maier

Linux on IBM Z Development

https://www.ibm.com/privacy/us/en/
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: [PATCH 5/8] scsi: remove the unchecked_isa_dma flag

[Christoph Hellwig]

On Fri, Apr 09, 2021 at 05:46:09PM +0200, Steffen Maier wrote:
> Is it OK to remove a long standing sysfs attribute?
> Was there any deprecation phase?
>
> I just noticed it in our CI reporting fails due to this change since at 
> least linux-next 20210409. Suppose it came via 
> https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git/commit/?h=for-5.13/block&id=aaff5ebaa2694f283b7d07fdd55fb287ffc4f1e9
>
> Wanted to ask before I adapt the test cases.

What does your CI do with it?

It would be kinda of sad to carry around this baggage, but if real
userspace is broken by the chance we'll have to do it.  I'm just not
entirely sure testcases qualify as real userspace yet.

Re: [PATCH 5/8] scsi: remove the unchecked_isa_dma flag

[Steffen Maier]

On 4/10/21 8:52 AM, Christoph Hellwig wrote:
> On Fri, Apr 09, 2021 at 05:46:09PM +0200, Steffen Maier wrote:
>> Is it OK to remove a long standing sysfs attribute?
>> Was there any deprecation phase?
>>
>> I just noticed it in our CI reporting fails due to this change since at
>> least linux-next 20210409. Suppose it came via
>> https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git/commit/?h=for-5.13/block&id=aaff5ebaa2694f283b7d07fdd55fb287ffc4f1e9
>>
>> Wanted to ask before I adapt the test cases.
> 
> What does your CI do with it?

We check almost all zfcp-related sysfs attributes for returning expected values 
(depending on possible error inject state transitions as well as steady state).
The Scsi_Host's unchecked_isa_dma is just checked to return '0'.

> It would be kinda of sad to carry around this baggage, but if real
> userspace is broken by the chance we'll have to do it.  I'm just not
> entirely sure testcases qualify as real userspace yet.

I was more pondering whether sysfs is considered stable API.

I'm not aware of any issues with real userspace if unchecked_isa_dma is absent.
Have adapted our test case to accept a missing unchecked_isa_dma sysfs attribute.

-- 
Mit freundlichen Gruessen / Kind regards
Steffen Maier

Linux on IBM Z Development

https://www.ibm.com/privacy/us/en/
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294