[PATCH v1] scsi: Don't select SCSI_PROC_FS by default

[PATCH v1] scsi: Don't select SCSI_PROC_FS by default

[Marc Gonzalez]

According to the option's help message, SCSI_PROC_FS has been
superseded for ~15 years. Don't select it by default anymore.

Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
---
 drivers/scsi/Kconfig | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 73bce9b6d037..8c95e9ad6470 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -54,14 +54,11 @@ config SCSI_NETLINK
 config SCSI_PROC_FS
 	bool "legacy /proc/scsi/ support"
 	depends on SCSI && PROC_FS
-	default y
 	---help---
 	  This option enables support for the various files in
 	  /proc/scsi.  In Linux 2.6 this has been superseded by
 	  files in sysfs but many legacy applications rely on this.
 
-	  If unsure say Y.
-
 comment "SCSI support type (disk, tape, CD-ROM)"
 	depends on SCSI
 
-- 
2.17.1

Re: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default

[Bart Van Assche]

On 6/12/19 6:59 AM, Marc Gonzalez wrote:
> According to the option's help message, SCSI_PROC_FS has been
> superseded for ~15 years. Don't select it by default anymore.
> 
> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
> ---
>   drivers/scsi/Kconfig | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> index 73bce9b6d037..8c95e9ad6470 100644
> --- a/drivers/scsi/Kconfig
> +++ b/drivers/scsi/Kconfig
> @@ -54,14 +54,11 @@ config SCSI_NETLINK
>   config SCSI_PROC_FS
>   	bool "legacy /proc/scsi/ support"
>   	depends on SCSI && PROC_FS
> -	default y
>   	---help---
>   	  This option enables support for the various files in
>   	  /proc/scsi.  In Linux 2.6 this has been superseded by
>   	  files in sysfs but many legacy applications rely on this.
>   
> -	  If unsure say Y.
> -
>   comment "SCSI support type (disk, tape, CD-ROM)"
>   	depends on SCSI

Hi Doug,

If I run grep "/proc/scsi" over the sg3_utils source code then grep 
reports 38 matches for that string. Does sg3_utils break with 
SCSI_PROC_FS=n?

Thanks,

Bart.

Re: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default

[Douglas Gilbert]

On 2019-06-17 5:11 p.m., Bart Van Assche wrote:
> On 6/12/19 6:59 AM, Marc Gonzalez wrote:
>> According to the option's help message, SCSI_PROC_FS has been
>> superseded for ~15 years. Don't select it by default anymore.
>>
>> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
>> ---
>>   drivers/scsi/Kconfig | 3 ---
>>   1 file changed, 3 deletions(-)
>>
>> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
>> index 73bce9b6d037..8c95e9ad6470 100644
>> --- a/drivers/scsi/Kconfig
>> +++ b/drivers/scsi/Kconfig
>> @@ -54,14 +54,11 @@ config SCSI_NETLINK
>>   config SCSI_PROC_FS
>>       bool "legacy /proc/scsi/ support"
>>       depends on SCSI && PROC_FS
>> -    default y
>>       ---help---
>>         This option enables support for the various files in
>>         /proc/scsi.  In Linux 2.6 this has been superseded by
>>         files in sysfs but many legacy applications rely on this.
>> -      If unsure say Y.
>> -
>>   comment "SCSI support type (disk, tape, CD-ROM)"
>>       depends on SCSI
> 
> Hi Doug,
> 
> If I run grep "/proc/scsi" over the sg3_utils source code then grep reports 38 
> matches for that string. Does sg3_utils break with SCSI_PROC_FS=n?

First, the sg driver. If placing
#undef CONFIG_SCSI_PROC_FS

prior to the includes in sg.c is a valid way to test that then the
answer is no. Ah, but you are talking about sg3_utils .

Or are you? For sg3_utils:

$ find . -name '*.c' -exec grep "/proc/scsi" {} \; -print
static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
./src/sg_read.c
static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
./src/sgp_dd.c
static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
./src/sgm_dd.c
static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
./src/sg_dd.c
                 "'echo 1 > /proc/scsi/sg/allow_dio'\n", q_len, dirio_count);
./testing/sg_tst_bidi.c
static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
./examples/sgq_dd.c


That is 6 (not 38) by my count. Those 6 are all for direct IO
(see below) which is off by default. I suspect old scanning
utilities like sg_scan and sg_map might also use /proc/scsi/* .
That is one reason why I wrote lsscsi. However I can't force folks
to use lsscsi. As a related example, I still get bug reports for
sginfo which I inherited from Eric Youngdale.

If I was asked to debug a problem with the sg driver in a
system without CONFIG_SCSI_PROC_FS defined, I would decline.

The absence of /proc/scsi/sg/debug would be my issue. Can this
be set up to do the same thing:
     cat /sys/class/scsi_generic/debug
? Is that breaking any sysfs rules?


Also folks who rely on this to work:
    cat /proc/scsi/sg/devices
0	0	0	0	0	1	255	0	1
0	0	0	1	0	1	255	0	1
0	0	0	2	0	1	255	0	1

would be disappointed. Further I note that setting allow_dio via
/proc/scsi/sg/allow_dio can also be done via /sys/module/sg/allow_dio .
So that would be an interface breakage, but with an alternative.

Doug Gilbert

Re: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default

[Finn Thain]

On Mon, 17 Jun 2019, Douglas Gilbert wrote:

> On 2019-06-17 5:11 p.m., Bart Van Assche wrote:
> > On 6/12/19 6:59 AM, Marc Gonzalez wrote:
> > > According to the option's help message, SCSI_PROC_FS has been
> > > superseded for ~15 years. Don't select it by default anymore.
> > > 
> > > Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
> > > ---
> > >   drivers/scsi/Kconfig | 3 ---
> > >   1 file changed, 3 deletions(-)
> > > 
> > > diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> > > index 73bce9b6d037..8c95e9ad6470 100644
> > > --- a/drivers/scsi/Kconfig
> > > +++ b/drivers/scsi/Kconfig
> > > @@ -54,14 +54,11 @@ config SCSI_NETLINK
> > >   config SCSI_PROC_FS
> > >       bool "legacy /proc/scsi/ support"
> > >       depends on SCSI && PROC_FS
> > > -    default y
> > >       ---help---
> > >         This option enables support for the various files in
> > >         /proc/scsi.  In Linux 2.6 this has been superseded by
> > >         files in sysfs but many legacy applications rely on this.
> > > -      If unsure say Y.
> > > -
> > >   comment "SCSI support type (disk, tape, CD-ROM)"
> > >       depends on SCSI
> > 
> > Hi Doug,
> > 
> > If I run grep "/proc/scsi" over the sg3_utils source code then grep reports
> > 38 matches for that string. Does sg3_utils break with SCSI_PROC_FS=n?
> 
> First, the sg driver. If placing
> #undef CONFIG_SCSI_PROC_FS
> 
> prior to the includes in sg.c is a valid way to test that then the
> answer is no. Ah, but you are talking about sg3_utils .
> 
> Or are you? For sg3_utils:
> 
> $ find . -name '*.c' -exec grep "/proc/scsi" {} \; -print
> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> ./src/sg_read.c
> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> ./src/sgp_dd.c
> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> ./src/sgm_dd.c
> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> ./src/sg_dd.c
>                 "'echo 1 > /proc/scsi/sg/allow_dio'\n", q_len, dirio_count);
> ./testing/sg_tst_bidi.c
> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> ./examples/sgq_dd.c
> 
> 
> That is 6 (not 38) by my count. Those 6 are all for direct IO
> (see below) which is off by default. I suspect old scanning
> utilities like sg_scan and sg_map might also use /proc/scsi/* .
> That is one reason why I wrote lsscsi. However I can't force folks
> to use lsscsi. As a related example, I still get bug reports for
> sginfo which I inherited from Eric Youngdale.
> 
> If I was asked to debug a problem with the sg driver in a
> system without CONFIG_SCSI_PROC_FS defined, I would decline.
> 
> The absence of /proc/scsi/sg/debug would be my issue. Can this
> be set up to do the same thing:
>     cat /sys/class/scsi_generic/debug
>   Is that breaking any sysfs rules?
> 
> 
> Also folks who rely on this to work:
>    cat /proc/scsi/sg/devices
> 0	0	0	0	0	1	255	0	1
> 0	0	0	1	0	1	255	0	1
> 0	0	0	2	0	1	255	0	1
> 
> would be disappointed. Further I note that setting allow_dio via
> /proc/scsi/sg/allow_dio can also be done via /sys/module/sg/allow_dio .
> So that would be an interface breakage, but with an alternative.
> 
> Doug Gilbert
> 

You can grep for /proc/scsi/ across all Debian packages:
https://codesearch.debian.net/

This reveals that /proc/scsi/sg/ appears in smartmontools and other 
packages, for example.

-- 

Re: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default

[Bart Van Assche]

On 6/17/19 5:35 PM, Douglas Gilbert wrote:
> For sg3_utils:
> 
> $ find . -name '*.c' -exec grep "/proc/scsi" {} \; -print
> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> ./src/sg_read.c
> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> ./src/sgp_dd.c
> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> ./src/sgm_dd.c
> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> ./src/sg_dd.c
>                  "'echo 1 > /proc/scsi/sg/allow_dio'\n", q_len, 
> dirio_count);
> ./testing/sg_tst_bidi.c
> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> ./examples/sgq_dd.c
>  
> That is 6 (not 38) by my count.

Hi Doug,

This is the command I ran:

$ git grep /proc/scsi | wc -l
38

I think your query excludes scripts/rescan-scsi-bus.sh.

Bart.

Re: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default

[Marc Gonzalez]

On 18/06/2019 03:08, Finn Thain wrote:

> On Mon, 17 Jun 2019, Douglas Gilbert wrote:
> 
>> On 2019-06-17 5:11 p.m., Bart Van Assche wrote:
>>
>>> On 6/12/19 6:59 AM, Marc Gonzalez wrote:
>>>
>>>> According to the option's help message, SCSI_PROC_FS has been
>>>> superseded for ~15 years. Don't select it by default anymore.
>>>>
>>>> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
>>>> ---
>>>>   drivers/scsi/Kconfig | 3 ---
>>>>   1 file changed, 3 deletions(-)
>>>>
>>>> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
>>>> index 73bce9b6d037..8c95e9ad6470 100644
>>>> --- a/drivers/scsi/Kconfig
>>>> +++ b/drivers/scsi/Kconfig
>>>> @@ -54,14 +54,11 @@ config SCSI_NETLINK
>>>>   config SCSI_PROC_FS
>>>>       bool "legacy /proc/scsi/ support"
>>>>       depends on SCSI && PROC_FS
>>>> -    default y
>>>>       ---help---
>>>>         This option enables support for the various files in
>>>>         /proc/scsi.  In Linux 2.6 this has been superseded by
>>>>         files in sysfs but many legacy applications rely on this.
>>>> -      If unsure say Y.
>>>> -
>>>>   comment "SCSI support type (disk, tape, CD-ROM)"
>>>>       depends on SCSI
>>>
>>> Hi Doug,
>>>
>>> If I run grep "/proc/scsi" over the sg3_utils source code then grep reports
>>> 38 matches for that string. Does sg3_utils break with SCSI_PROC_FS=n?
>>
>> First, the sg driver. If placing
>> #undef CONFIG_SCSI_PROC_FS
>>
>> prior to the includes in sg.c is a valid way to test that then the
>> answer is no. Ah, but you are talking about sg3_utils .
>>
>> Or are you? For sg3_utils:
>>
>> $ find . -name '*.c' -exec grep "/proc/scsi" {} \; -print
>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>> ./src/sg_read.c
>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>> ./src/sgp_dd.c
>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>> ./src/sgm_dd.c
>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>> ./src/sg_dd.c
>>                 "'echo 1 > /proc/scsi/sg/allow_dio'\n", q_len, dirio_count);
>> ./testing/sg_tst_bidi.c
>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>> ./examples/sgq_dd.c
>>
>>
>> That is 6 (not 38) by my count. Those 6 are all for direct IO
>> (see below) which is off by default. I suspect old scanning
>> utilities like sg_scan and sg_map might also use /proc/scsi/* .
>> That is one reason why I wrote lsscsi. However I can't force folks
>> to use lsscsi. As a related example, I still get bug reports for
>> sginfo which I inherited from Eric Youngdale.
>>
>> If I was asked to debug a problem with the sg driver in a
>> system without CONFIG_SCSI_PROC_FS defined, I would decline.
>>
>> The absence of /proc/scsi/sg/debug would be my issue. Can this
>> be set up to do the same thing:
>>     cat /sys/class/scsi_generic/debug
>>   Is that breaking any sysfs rules?
>>
>>
>> Also folks who rely on this to work:
>>    cat /proc/scsi/sg/devices
>> 0	0	0	0	0	1	255	0	1
>> 0	0	0	1	0	1	255	0	1
>> 0	0	0	2	0	1	255	0	1
>>
>> would be disappointed. Further I note that setting allow_dio via
>> /proc/scsi/sg/allow_dio can also be done via /sys/module/sg/allow_dio .
>> So that would be an interface breakage, but with an alternative.
> 
> You can grep for /proc/scsi/ across all Debian packages:
> https://codesearch.debian.net/
> 
> This reveals that /proc/scsi/sg/ appears in smartmontools and other 
> packages, for example.

Hello everyone,

Please note that I am _in no way_ suggesting that we remove any code.

I just think it might be time to stop forcing CONFIG_SCSI_PROC_FS into
every config, and instead require one to explicitly request the aging
feature (which makes CONFIG_SCSI_PROC_FS show up in a defconfig).

Maybe we could add CONFIG_SCSI_PROC_FS to arch/x86/configs/foo ?
(For which foo? In a separate patch or squashed with this one?)

Regards.

Re: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default

[Douglas Gilbert]

On 2019-06-18 3:29 a.m., Marc Gonzalez wrote:
> On 18/06/2019 03:08, Finn Thain wrote:
> 
>> On Mon, 17 Jun 2019, Douglas Gilbert wrote:
>>
>>> On 2019-06-17 5:11 p.m., Bart Van Assche wrote:
>>>
>>>> On 6/12/19 6:59 AM, Marc Gonzalez wrote:
>>>>
>>>>> According to the option's help message, SCSI_PROC_FS has been
>>>>> superseded for ~15 years. Don't select it by default anymore.
>>>>>
>>>>> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
>>>>> ---
>>>>>    drivers/scsi/Kconfig | 3 ---
>>>>>    1 file changed, 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
>>>>> index 73bce9b6d037..8c95e9ad6470 100644
>>>>> --- a/drivers/scsi/Kconfig
>>>>> +++ b/drivers/scsi/Kconfig
>>>>> @@ -54,14 +54,11 @@ config SCSI_NETLINK
>>>>>    config SCSI_PROC_FS
>>>>>        bool "legacy /proc/scsi/ support"
>>>>>        depends on SCSI && PROC_FS
>>>>> -    default y
>>>>>        ---help---
>>>>>          This option enables support for the various files in
>>>>>          /proc/scsi.  In Linux 2.6 this has been superseded by
>>>>>          files in sysfs but many legacy applications rely on this.
>>>>> -      If unsure say Y.
>>>>> -
>>>>>    comment "SCSI support type (disk, tape, CD-ROM)"
>>>>>        depends on SCSI
>>>>
>>>> Hi Doug,
>>>>
>>>> If I run grep "/proc/scsi" over the sg3_utils source code then grep reports
>>>> 38 matches for that string. Does sg3_utils break with SCSI_PROC_FS=n?
>>>
>>> First, the sg driver. If placing
>>> #undef CONFIG_SCSI_PROC_FS
>>>
>>> prior to the includes in sg.c is a valid way to test that then the
>>> answer is no. Ah, but you are talking about sg3_utils .
>>>
>>> Or are you? For sg3_utils:
>>>
>>> $ find . -name '*.c' -exec grep "/proc/scsi" {} \; -print
>>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>>> ./src/sg_read.c
>>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>>> ./src/sgp_dd.c
>>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>>> ./src/sgm_dd.c
>>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>>> ./src/sg_dd.c
>>>                  "'echo 1 > /proc/scsi/sg/allow_dio'\n", q_len, dirio_count);
>>> ./testing/sg_tst_bidi.c
>>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>>> ./examples/sgq_dd.c
>>>
>>>
>>> That is 6 (not 38) by my count. Those 6 are all for direct IO
>>> (see below) which is off by default. I suspect old scanning
>>> utilities like sg_scan and sg_map might also use /proc/scsi/* .
>>> That is one reason why I wrote lsscsi. However I can't force folks
>>> to use lsscsi. As a related example, I still get bug reports for
>>> sginfo which I inherited from Eric Youngdale.
>>>
>>> If I was asked to debug a problem with the sg driver in a
>>> system without CONFIG_SCSI_PROC_FS defined, I would decline.
>>>
>>> The absence of /proc/scsi/sg/debug would be my issue. Can this
>>> be set up to do the same thing:
>>>      cat /sys/class/scsi_generic/debug
>>>    Is that breaking any sysfs rules?
>>>
>>>
>>> Also folks who rely on this to work:
>>>     cat /proc/scsi/sg/devices
>>> 0	0	0	0	0	1	255	0	1
>>> 0	0	0	1	0	1	255	0	1
>>> 0	0	0	2	0	1	255	0	1
>>>
>>> would be disappointed. Further I note that setting allow_dio via
>>> /proc/scsi/sg/allow_dio can also be done via /sys/module/sg/allow_dio .
>>> So that would be an interface breakage, but with an alternative.
>>
>> You can grep for /proc/scsi/ across all Debian packages:
>> https://codesearch.debian.net/
>>
>> This reveals that /proc/scsi/sg/ appears in smartmontools and other
>> packages, for example.
> 
> Hello everyone,
> 
> Please note that I am _in no way_ suggesting that we remove any code.
> 
> I just think it might be time to stop forcing CONFIG_SCSI_PROC_FS into
> every config, and instead require one to explicitly request the aging
> feature (which makes CONFIG_SCSI_PROC_FS show up in a defconfig).
> 
> Maybe we could add CONFIG_SCSI_PROC_FS to arch/x86/configs/foo ?
> (For which foo? In a separate patch or squashed with this one?)

Marc,
Since current sg driver usage seems to depend more on SCSI_PROC_FS
being "y" than other parts of the SCSI subsystem then if
SCSI_PROC_FS is to default to "n" in the future then a new
CONFIG_SG_PROC_FS variable could be added.

If CONFIG_CHR_DEV_SG is "*" or "m" then default CONFIG_SG_PROC_FS
to "y"; if CONFIG_SCSI_PROC_FS is "y" then default CONFIG_SG_PROC_FS
to "y"; else default CONFIG_SG_PROC_FS to "n". Obviously the
sg driver would need to be changed to use CONFIG_SG_PROC_FS instead
of CONFIG_SCSI_PROC_FS .


Does that defeat the whole purpose of your proposal or could it be
seen as a partial step in that direction? What is the motivation
for this proposal?

Doug Gilbert


BTW We still have the non-sg related 'cat /proc/scsi/scsi' usage
and 'cat /proc/scsi/device_info'. And I believe the latter one is
writable even though its permissions say otherwise.

RE: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default

[Elliott, Robert (Servers)]

> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Bart
> Van Assche
> Sent: Monday, June 17, 2019 10:28 PM
> To: dgilbert@interlog.com; Marc Gonzalez <marc.w.gonzalez@free.fr>; James Bottomley
> <jejb@linux.ibm.com>; Martin Petersen <martin.petersen@oracle.com>
> Cc: SCSI <linux-scsi@vger.kernel.org>; LKML <linux-kernel@vger.kernel.org>; Christoph Hellwig
> <hch@lst.de>
> Subject: Re: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default
> 
> On 6/17/19 5:35 PM, Douglas Gilbert wrote:
> > For sg3_utils:
> >
> > $ find . -name '*.c' -exec grep "/proc/scsi" {} \; -print
> > static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> > ./src/sg_read.c
> > static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> > ./src/sgp_dd.c
> > static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> > ./src/sgm_dd.c
> > static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> > ./src/sg_dd.c
> >                  "'echo 1 > /proc/scsi/sg/allow_dio'\n", q_len,
> > dirio_count);
> > ./testing/sg_tst_bidi.c
> > static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> > ./examples/sgq_dd.c
> >
> > That is 6 (not 38) by my count.
> 
> Hi Doug,
> 
> This is the command I ran:
> 
> $ git grep /proc/scsi | wc -l
> 38
> 
> I think your query excludes scripts/rescan-scsi-bus.sh.
> 
> Bart.

Here's the full list to ensure the discussion doesn't overlook anything:

sg3_utils-1.44$ grep -R /proc/scsi .
./src/sg_read.c:static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
./src/sgp_dd.c:static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
./src/sgm_dd.c:static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
./src/sg_dd.c:static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
./scripts/rescan-scsi-bus.sh:# Return hosts. /proc/scsi/HOSTADAPTER/? must exist
./scripts/rescan-scsi-bus.sh:  for driverdir in /proc/scsi/*; do
./scripts/rescan-scsi-bus.sh:    driver=${driverdir#/proc/scsi/}
./scripts/rescan-scsi-bus.sh:      name=${hostdir#/proc/scsi/*/}
./scripts/rescan-scsi-bus.sh:# Get /proc/scsi/scsi info for device $host:$channel:$id:$lun
./scripts/rescan-scsi-bus.sh:    SCSISTR=$(grep -A "$LN" -e "$grepstr" /proc/scsi/scsi)
./scripts/rescan-scsi-bus.sh:    DRV=`grep 'Attached drivers:' /proc/scsi/scsi 2>/dev/null`
./scripts/rescan-scsi-bus.sh:      echo "scsi report-devs 1" >/proc/scsi/scsi
./scripts/rescan-scsi-bus.sh:      DRV=`grep 'Attached drivers:' /proc/scsi/scsi 2>/dev/null`
./scripts/rescan-scsi-bus.sh:      echo "scsi report-devs 0" >/proc/scsi/scsi
./scripts/rescan-scsi-bus.sh:# Outputs description from /proc/scsi/scsi (unless arg passed)
./scripts/rescan-scsi-bus.sh:        echo "scsi remove-single-device $devnr" > /proc/scsi/scsi
./scripts/rescan-scsi-bus.sh:          echo "scsi add-single-device $devnr" > /proc/scsi/scsi
./scripts/rescan-scsi-bus.sh:      echo "scsi add-single-device $devnr" > /proc/scsi/scsi
./scripts/rescan-scsi-bus.sh:      echo "scsi add-single-device $devnr" > /proc/scsi/scsi
./scripts/rescan-scsi-bus.sh:      echo "scsi add-single-device $host $channel $id $SCAN_WILD_CARD" > /proc/scsi/scsi
./scripts/rescan-scsi-bus.sh:if test ! -d /sys/class/scsi_host/ -a ! -d /proc/scsi/; then
./ChangeLog:    /proc/scsi/sg/allow_dio is '0'
./ChangeLog:  - change sg_debug to call system("cat /proc/scsi/sg/debug");
./suse/sg3_utils.changes:  * Support systems without /proc/scsi
./examples/sgq_dd.c:static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
./doc/sg_read.8:If direct IO is selected and /proc/scsi/sg/allow_dio
./doc/sg_read.8:"echo 1 > /proc/scsi/sg/allow_dio". An alternate way to avoid the
./doc/sg_map.8:observing the output of the command: "cat /proc/scsi/scsi".
./doc/sgp_dd.8:at completion. If direct IO is selected and /proc/scsi/sg/allow_dio
./doc/sgp_dd.8:this at completion. If direct IO is selected and /proc/scsi/sg/allow_dio
./doc/sgp_dd.8:mapping to SCSI block devices should be checked with 'cat /proc/scsi/scsi'
./doc/sg_dd.8:notes this at completion. If direct IO is selected and /proc/scsi/sg/allow_dio
./doc/sg_dd.8:this at completion. If direct IO is selected and /proc/scsi/sg/allow_dio
./doc/sg_dd.8:with 'echo 1 > /proc/scsi/sg/allow_dio'.
./doc/sg_dd.8:mapping to SCSI block devices should be checked with 'cat /proc/scsi/scsi',

Re: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default

[Marc Gonzalez]

On 18/06/2019 17:31, Douglas Gilbert wrote:

> On 2019-06-18 3:29 a.m., Marc Gonzalez wrote:
> 
>> Please note that I am _in no way_ suggesting that we remove any code.
>>
>> I just think it might be time to stop forcing CONFIG_SCSI_PROC_FS into
>> every config, and instead require one to explicitly request the aging
>> feature (which makes CONFIG_SCSI_PROC_FS show up in a defconfig).
>>
>> Maybe we could add CONFIG_SCSI_PROC_FS to arch/x86/configs/foo ?
>> (For which foo? In a separate patch or squashed with this one?)
> 
> Since current sg driver usage seems to depend more on SCSI_PROC_FS
> being "y" than other parts of the SCSI subsystem then if
> SCSI_PROC_FS is to default to "n" in the future then a new
> CONFIG_SG_PROC_FS variable could be added.
> 
> If CONFIG_CHR_DEV_SG is "*" or "m" then default CONFIG_SG_PROC_FS
> to "y"; if CONFIG_SCSI_PROC_FS is "y" then default CONFIG_SG_PROC_FS
> to "y"; else default CONFIG_SG_PROC_FS to "n". Obviously the
> sg driver would need to be changed to use CONFIG_SG_PROC_FS instead
> of CONFIG_SCSI_PROC_FS .

I like your idea, and I think it might even be made slightly simpler.

I assume sg3_utils requires CHR_DEV_SG. Is it the case?

If so, we would just need to enable SCSI_PROC_FS when CHR_DEV_SG is enabled.

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 73bce9b6d037..642ca0e7d363 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -54,14 +54,12 @@ config SCSI_NETLINK
 config SCSI_PROC_FS
 	bool "legacy /proc/scsi/ support"
 	depends on SCSI && PROC_FS
-	default y
+	default CHR_DEV_SG
 	---help---
 	  This option enables support for the various files in
 	  /proc/scsi.  In Linux 2.6 this has been superseded by
 	  files in sysfs but many legacy applications rely on this.
 
-	  If unsure say Y.
-
 comment "SCSI support type (disk, tape, CD-ROM)"
 	depends on SCSI
 

Would that work for you?
I checked that SCSI_PROC_FS=y whether CHR_DEV_SG=y or m
I can spin a v2, with a blurb about how sg3_utils relies on SCSI_PROC_FS.

> Does that defeat the whole purpose of your proposal or could it be
> seen as a partial step in that direction? What is the motivation
> for this proposal?

The rationale was just to look for "special-purpose" options that are
enabled by default, and change the default wherever possible, as a
matter of uniformity.

> BTW We still have the non-sg related 'cat /proc/scsi/scsi' usage
> and 'cat /proc/scsi/device_info'. And I believe the latter one is
> writable even though its permissions say otherwise.

Any relation between SG and BSG?

Regards.

Re: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default

[Douglas Gilbert]

On 2019-06-19 5:42 a.m., Marc Gonzalez wrote:
> On 18/06/2019 17:31, Douglas Gilbert wrote:
> 
>> On 2019-06-18 3:29 a.m., Marc Gonzalez wrote:
>>
>>> Please note that I am _in no way_ suggesting that we remove any code.
>>>
>>> I just think it might be time to stop forcing CONFIG_SCSI_PROC_FS into
>>> every config, and instead require one to explicitly request the aging
>>> feature (which makes CONFIG_SCSI_PROC_FS show up in a defconfig).
>>>
>>> Maybe we could add CONFIG_SCSI_PROC_FS to arch/x86/configs/foo ?
>>> (For which foo? In a separate patch or squashed with this one?)
>>
>> Since current sg driver usage seems to depend more on SCSI_PROC_FS
>> being "y" than other parts of the SCSI subsystem then if
>> SCSI_PROC_FS is to default to "n" in the future then a new
>> CONFIG_SG_PROC_FS variable could be added.
>>
>> If CONFIG_CHR_DEV_SG is "*" or "m" then default CONFIG_SG_PROC_FS
>> to "y"; if CONFIG_SCSI_PROC_FS is "y" then default CONFIG_SG_PROC_FS
>> to "y"; else default CONFIG_SG_PROC_FS to "n". Obviously the
>> sg driver would need to be changed to use CONFIG_SG_PROC_FS instead
>> of CONFIG_SCSI_PROC_FS .
> 
> I like your idea, and I think it might even be made slightly simpler.
> 
> I assume sg3_utils requires CHR_DEV_SG. Is it the case?
> 
> If so, we would just need to enable SCSI_PROC_FS when CHR_DEV_SG is enabled.
> 
> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> index 73bce9b6d037..642ca0e7d363 100644
> --- a/drivers/scsi/Kconfig
> +++ b/drivers/scsi/Kconfig
> @@ -54,14 +54,12 @@ config SCSI_NETLINK
>   config SCSI_PROC_FS
>   	bool "legacy /proc/scsi/ support"
>   	depends on SCSI && PROC_FS
> -	default y
> +	default CHR_DEV_SG
>   	---help---
>   	  This option enables support for the various files in
>   	  /proc/scsi.  In Linux 2.6 this has been superseded by
>   	  files in sysfs but many legacy applications rely on this.
>   
> -	  If unsure say Y.
> -
>   comment "SCSI support type (disk, tape, CD-ROM)"
>   	depends on SCSI
>   
> 
> Would that work for you?
> I checked that SCSI_PROC_FS=y whether CHR_DEV_SG=y or m
> I can spin a v2, with a blurb about how sg3_utils relies on SCSI_PROC_FS.

Yes, but (see below) ...

>> Does that defeat the whole purpose of your proposal or could it be
>> seen as a partial step in that direction? What is the motivation
>> for this proposal?
> 
> The rationale was just to look for "special-purpose" options that are
> enabled by default, and change the default wherever possible, as a
> matter of uniformity.
> 
>> BTW We still have the non-sg related 'cat /proc/scsi/scsi' usage
>> and 'cat /proc/scsi/device_info'. And I believe the latter one is
>> writable even though its permissions say otherwise.
> 
> Any relation between SG and BSG?

Only in the sense that writing to /proc/scsi/device_info changes the
way the SCSI mid-level handles the identified device. So that is
in common with, and hence the same relation as,  sd, sr, st, ses, etc
have with the identified device (e.g. a specialized USB dongle).


Example of use of /proc/scsi/scsi

$ cat /proc/scsi/scsi
Attached devices:
Host: scsi0 Channel: 00 Id: 00 Lun: 00
   Vendor: Linux    Model: scsi_debug       Rev: 0188
   Type:   Direct-Access                    ANSI  SCSI revision: 07
Host: scsi0 Channel: 00 Id: 00 Lun: 01
   Vendor: Linux    Model: scsi_debug       Rev: 0188
   Type:   Direct-Access                    ANSI  SCSI revision: 07
Host: scsi0 Channel: 00 Id: 00 Lun: 02
   Vendor: Linux    Model: scsi_debug       Rev: 0188
   Type:   Direct-Access                    ANSI  SCSI revision: 07

Which can be replaced by:

$ lsscsi
[0:0:0:0]    disk    Linux    scsi_debug       0188  /dev/sda
[0:0:0:1]    disk    Linux    scsi_debug       0188  /dev/sdb
[0:0:0:2]    disk    Linux    scsi_debug       0188  /dev/sdc
[N:0:1:1]    disk    INTEL SSDPEKKF256G7L__1                    /dev/nvme0n1

Or if one really likes the "classic" look:

$ lsscsi -c
Attached devices:
Host: scsi0 Channel: 00 Target: 00 Lun: 00
   Vendor: Linux    Model: scsi_debug       Rev: 0188
   Type:   Direct-Access                    ANSI SCSI revision: 07
Host: scsi0 Channel: 00 Target: 00 Lun: 01
   Vendor: Linux    Model: scsi_debug       Rev: 0188
   Type:   Direct-Access                    ANSI SCSI revision: 07
Host: scsi0 Channel: 00 Target: 00 Lun: 02
   Vendor: Linux    Model: scsi_debug       Rev: 0188
   Type:   Direct-Access                    ANSI SCSI revision: 07


Now looking at /proc/scsi/device_info

IMO unless there is a replacement for /proc/scsi/device_info
then your patch should not go ahead . If it does, any reasonable
distro should override it.

$ cat /proc/scsi/device_info
'Aashima' 'IMAGERY 2400SP' 0x1
'CHINON' 'CD-ROM CDS-431' 0x1
'CHINON' 'CD-ROM CDS-535' 0x1
'DENON' 'DRD-25X' 0x1
...
'XYRATEX' 'RS' 0x240
'Zzyzx' 'RocketStor 500S' 0x40
'Zzyzx' 'RocketStor 2000' 0x40


That is a black (or quirks) list that can be added to by writing an
entry to /proc/scsi/device_info . So if a user has a device that needs
one of those quirks defined to stop their system locking up when a
device of that type is plugged in, and the distro or some app (say,
that needs that device) knows about that, then it would be sad if
/proc/scsi/device_info was missing due to the changed default that is
being proposed.

The word "legacy" *** is thrown around a bit too often. It is not
legacy if there is no replacement for that functionality.

Doug Gilbert


*** I'm assuming "aging feature" is a softer form of "legacy feature".
     "Classic" may be going too far the other way from "aged" and
     "legacy".

Re: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default

[Marc Gonzalez]

On 19/06/2019 16:34, Douglas Gilbert wrote:

> On 2019-06-19 5:42 a.m., Marc Gonzalez wrote:
> 
>> I assume sg3_utils requires CHR_DEV_SG. Is it the case?
>>
>> If so, we would just need to enable SCSI_PROC_FS when CHR_DEV_SG is enabled.
>>
>> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
>> index 73bce9b6d037..642ca0e7d363 100644
>> --- a/drivers/scsi/Kconfig
>> +++ b/drivers/scsi/Kconfig
>> @@ -54,14 +54,12 @@ config SCSI_NETLINK
>>   config SCSI_PROC_FS
>>   	bool "legacy /proc/scsi/ support"
>>   	depends on SCSI && PROC_FS
>> -	default y
>> +	default CHR_DEV_SG
>>   	---help---
>>   	  This option enables support for the various files in
>>   	  /proc/scsi.  In Linux 2.6 this has been superseded by
>>   	  files in sysfs but many legacy applications rely on this.
>>   
>> -	  If unsure say Y.
>> -
>>   comment "SCSI support type (disk, tape, CD-ROM)"
>>   	depends on SCSI
>>   
>>
>> Would that work for you?
>> I checked that SCSI_PROC_FS=y whether CHR_DEV_SG=y or m
>> I can spin a v2, with a blurb about how sg3_utils relies on SCSI_PROC_FS.
> 
> Yes, but (see below) ...
> 
> Example of use of /proc/scsi/scsi [...]
> Now looking at /proc/scsi/device_info [...]
> 
> IMO unless there is a replacement for /proc/scsi/device_info
> then your patch should not go ahead . If it does, any reasonable
> distro should override it.
> 
> That is a black (or quirks) list that can be added to by writing an
> entry to /proc/scsi/device_info . So if a user has a device that needs
> one of those quirks defined to stop their system locking up when a
> device of that type is plugged in, and the distro or some app (say,
> that needs that device) knows about that, then it would be sad if
> /proc/scsi/device_info was missing due to the changed default that is
> being proposed.

You've made it clear that SCSI_PROC_FS is important for several classes
of hardware.

You worry that changing the Kconfig default would force distro maintainers
(we are talking about Debian/Redhat/Suse/etc right?) to actually turn the
feature on, instead of relying on the "default y" behavior (as they have
done in the past).

How likely is it that distro kernels would *not* enable CHR_DEV_SG?
(Distros tend to enable everything, and then some.)

CHR_DEV_SG is enabled in the default configs for i386 and x86_64:

$ git grep CHR_DEV_SG arch/x86/configs/
arch/x86/configs/i386_defconfig:CONFIG_CHR_DEV_SG=y
arch/x86/configs/x86_64_defconfig:CONFIG_CHR_DEV_SG=y

=> As soon as CHR_DEV_SG is enabled, SCSI_PROC_FS is also enabled.

(I work on smaller systems where we do use /proc occasionally, but we
don't enable CHR_DEV_SG or SCSI_PROC_FS.)

I think we just need to find a reasonable condition for enabling
SCSI_PROC_FS by default on "your" sytems, and not on "mine" ;-)

Regards.

Re: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default

[Martin K. Petersen]

Marc,

> (I work on smaller systems where we do use /proc occasionally, but we
> don't enable CHR_DEV_SG or SCSI_PROC_FS.)

Many sg apps depend on SCSI_PROC_FS. That doesn't imply that only sg
apps depend on it.

As an example, with SCSI_PROC_FS enabled we don't need your SanDisk
Cruzer Blade patch at all since you can tweak the blacklist flags from
user space.

Also, the "legacy" moniker was wishful thinking. Applied to the Kconfig
option at a time where sysfs was new and shiny and considered the
solution to all the kernel's problems. But that wholesale transition of
all interfaces from /proc simply never took place. What happened was
that *new* functionality largely went to sysfs.

Note that I don't have a problem adding missing knobs to sysfs where it
makes sense. But it will obviously take a while for userland apps to
adopt it.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default

[Finn Thain]

On Thu, 20 Jun 2019, Marc Gonzalez wrote:

> 
> How likely is it that distro kernels would *not* enable CHR_DEV_SG? 
> (Distros tend to enable everything, and then some.)
> 

How likely is it that embedded developers would *not* disable CHR_DEV_SG? 
They tend to disable everything, and then enable only what they need.

-- 

Re: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default

[Marc Gonzalez]

On 21/06/2019 01:43, Finn Thain wrote:

> On Thu, 20 Jun 2019, Marc Gonzalez wrote:
> 
>> How likely is it that distro kernels would *not* enable CHR_DEV_SG?
>> (Distros tend to enable everything, and then some.)
> 
> How likely is it that embedded developers would *not* disable CHR_DEV_SG?
> They tend to disable everything, and then enable only what they need.

I don't see where you're going with this line of reasoning?

Below is my current (as of next-20190612) defconfig.

Notice the options marked as "not set". These are options that are
enabled by default (and which I've disabled).

Everyone thinks "their" option is critical (and it is, *to them*) but, in fact,
few really are -- universally. When an option is enabled by default, it does not
show up in defconfigs that want the option, and shows up as disabled in defconfigs
that don't want it.

Ideally, options would show as enabled in defconfigs that want the option,
and not show in defconfigs that don't.

I'm currently trying to change "enabled by default" for the following options:
# CONFIG_SCSI_PROC_FS is not set
# CONFIG_LCD_CLASS_DEVICE is not set
# CONFIG_BACKLIGHT_CLASS_DEVICE is not set
# CONFIG_COMMON_CLK_XGENE is not set
# CONFIG_QCOM_A53PLL is not set
# CONFIG_QCOM_CLK_APCS_MSM8916 is not set

I guess SCSI_PROC_FS might be the more useful to me, as it might help
debugging USB drive issues? In any event, I would rather have it show
up explicitly in my defconfig, to remind me I've selected it.

As for a few other "default on" options in my defconfig...

- SWAP I guess goes all the way back to Linux 2.0 on x86

- EFI is enabled by default because "big" arm64 systems rely on it.
But that's not true of smaller arm64 systems, based on DT.

- Not sure about the IOSCHED algorithms.

- IPV6 makes sense, to push for broader adoption

TODO: look into HW_RANDOM and CRYPTO_HW


# CONFIG_SWAP is not set
CONFIG_NO_HZ_IDLE=y
CONFIG_HIGH_RES_TIMERS=y
CONFIG_PREEMPT=y
CONFIG_LOG_BUF_SHIFT=20
CONFIG_BLK_DEV_INITRD=y
CONFIG_ARCH_QCOM=y
CONFIG_CMDLINE="ignore_loglevel nosmp"
# CONFIG_EFI is not set
# CONFIG_SUSPEND is not set
CONFIG_PM=y
CONFIG_CPU_IDLE=y
CONFIG_ARM_CPUIDLE=y
CONFIG_MODULES=y
CONFIG_MODULE_UNLOAD=y
# CONFIG_MQ_IOSCHED_DEADLINE is not set
# CONFIG_MQ_IOSCHED_KYBER is not set
CONFIG_NET=y
CONFIG_PACKET=y
CONFIG_UNIX=y
CONFIG_INET=y
# CONFIG_IPV6 is not set
# CONFIG_WIRELESS is not set
CONFIG_PCI=y
CONFIG_PCIEPORTBUS=y
CONFIG_PCIE_QCOM=y
CONFIG_DEVTMPFS=y
CONFIG_DEVTMPFS_MOUNT=y
CONFIG_SCSI=y
# CONFIG_SCSI_PROC_FS is not set
CONFIG_BLK_DEV_SD=y
CONFIG_SCSI_UFSHCD=y
CONFIG_SCSI_UFSHCD_PLATFORM=y
CONFIG_SCSI_UFS_QCOM=y
CONFIG_NETDEVICES=y
CONFIG_ATL1C=y
# CONFIG_WLAN is not set
# CONFIG_INPUT_KEYBOARD is not set
# CONFIG_INPUT_MOUSE is not set
# CONFIG_SERIO_SERPORT is not set
CONFIG_VT_HW_CONSOLE_BINDING=y
CONFIG_LEGACY_PTY_COUNT=16
CONFIG_SERIAL_8250=y
CONFIG_SERIAL_MSM=y
CONFIG_SERIAL_MSM_CONSOLE=y
CONFIG_SERIAL_DEV_BUS=y
# CONFIG_HW_RANDOM is not set
CONFIG_I2C=y
CONFIG_I2C_CHARDEV=y
CONFIG_I2C_QUP=y
CONFIG_SPMI=y
CONFIG_PINCTRL_MSM8998=y
CONFIG_THERMAL=y
CONFIG_QCOM_TSENS=y
CONFIG_REGULATOR=y
CONFIG_REGULATOR_FIXED_VOLTAGE=y
CONFIG_REGULATOR_QCOM_SMD_RPM=y
# CONFIG_LCD_CLASS_DEVICE is not set
# CONFIG_BACKLIGHT_CLASS_DEVICE is not set
CONFIG_SOUND=y
CONFIG_SND=y
CONFIG_SND_SOC=y
CONFIG_SND_SOC_WCD9335=y
# CONFIG_HID is not set
# CONFIG_USB_HID is not set
CONFIG_USB=y
# CONFIG_USB_PCI is not set
CONFIG_USB_XHCI_HCD=y
CONFIG_USB_STORAGE=y
CONFIG_USB_DWC3=y
# CONFIG_COMMON_CLK_XGENE is not set
CONFIG_COMMON_CLK_QCOM=y
# CONFIG_QCOM_A53PLL is not set
# CONFIG_QCOM_CLK_APCS_MSM8916 is not set
CONFIG_QCOM_CLK_SMD_RPM=y
CONFIG_MSM_GCC_8998=y
CONFIG_HWSPINLOCK=y
CONFIG_HWSPINLOCK_QCOM=y
CONFIG_MAILBOX=y
CONFIG_QCOM_APCS_IPC=y
CONFIG_ARM_SMMU=y
CONFIG_RPMSG_QCOM_GLINK_RPM=y
CONFIG_RPMSG_QCOM_GLINK_SMEM=y
CONFIG_RPMSG_QCOM_SMD=y
CONFIG_RPMSG_VIRTIO=y
CONFIG_QCOM_COMMAND_DB=y
CONFIG_QCOM_SMEM=y
CONFIG_QCOM_SMD_RPM=y
CONFIG_IIO=y
CONFIG_PHY_QCOM_QMP=y
CONFIG_PHY_QCOM_QUSB2=y
CONFIG_NVMEM=y
CONFIG_QCOM_QFPROM=y
CONFIG_SLIMBUS=y
CONFIG_SLIM_QCOM_CTRL=y
CONFIG_INTERCONNECT=y
CONFIG_INTERCONNECT_QCOM=y
CONFIG_TMPFS=y
# CONFIG_CRYPTO_HW is not set
CONFIG_DEBUG_FS=y
CONFIG_MAGIC_SYSRQ=y
CONFIG_DEBUG_KERNEL=y
CONFIG_STACKTRACE=y

Re: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default

[Finn Thain]

On Fri, 21 Jun 2019, Marc Gonzalez wrote:

> On 21/06/2019 01:43, Finn Thain wrote:
> 
> > On Thu, 20 Jun 2019, Marc Gonzalez wrote:
> > 
> >> How likely is it that distro kernels would *not* enable CHR_DEV_SG? 
> >> (Distros tend to enable everything, and then some.)
> > 
> > How likely is it that embedded developers would *not* disable 
> > CHR_DEV_SG? They tend to disable everything, and then enable only what 
> > they need.
> 
> I don't see where you're going with this line of reasoning?
> 

Exactly. This sort of reasoning goes nowhere.

> Below is my current (as of next-20190612) defconfig.
> 
> Notice the options marked as "not set". These are options that are 
> enabled by default (and which I've disabled).
> 
> Everyone thinks "their" option is critical (and it is, *to them*) but, 
> in fact, few really are -- universally. 

The relevant policy is documented in 
Documentation/kbuild/kconfig-language.txt.

If there are useful generalizations about the choice of defaults for 
Kconfig symbols (or changing those defaults) that are not documented then 
they probably should be.

-- 

Re: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default

[Hannes Reinecke]

On 6/18/19 5:28 AM, Bart Van Assche wrote:
> On 6/17/19 5:35 PM, Douglas Gilbert wrote:
>> For sg3_utils:
>>
>> $ find . -name '*.c' -exec grep "/proc/scsi" {} \; -print
>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>> ./src/sg_read.c
>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>> ./src/sgp_dd.c
>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>> ./src/sgm_dd.c
>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>> ./src/sg_dd.c
>>                  "'echo 1 > /proc/scsi/sg/allow_dio'\n", q_len,
>> dirio_count);
>> ./testing/sg_tst_bidi.c
>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>> ./examples/sgq_dd.c
>>  
>> That is 6 (not 38) by my count.
> 
> Hi Doug,
> 
> This is the command I ran:
> 
> $ git grep /proc/scsi | wc -l
> 38
> 
> I think your query excludes scripts/rescan-scsi-bus.sh.
> 
You can ignore rescan-scsi-bus.sh.
It's keeping /proc access as a fallback option only (as it's meant to be
kernel-independent); it will be using /sys per default and will happily
work without /proc/scsi.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)

Re: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default

[Hannes Reinecke]

On 6/18/19 7:43 PM, Elliott, Robert (Servers) wrote:
> 
> 
>> -----Original Message-----
>> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Bart
>> Van Assche
>> Sent: Monday, June 17, 2019 10:28 PM
>> To: dgilbert@interlog.com; Marc Gonzalez <marc.w.gonzalez@free.fr>; James Bottomley
>> <jejb@linux.ibm.com>; Martin Petersen <martin.petersen@oracle.com>
>> Cc: SCSI <linux-scsi@vger.kernel.org>; LKML <linux-kernel@vger.kernel.org>; Christoph Hellwig
>> <hch@lst.de>
>> Subject: Re: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default
>>
>> On 6/17/19 5:35 PM, Douglas Gilbert wrote:
>>> For sg3_utils:
>>>
>>> $ find . -name '*.c' -exec grep "/proc/scsi" {} \; -print
>>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>>> ./src/sg_read.c
>>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>>> ./src/sgp_dd.c
>>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>>> ./src/sgm_dd.c
>>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>>> ./src/sg_dd.c
>>>                  "'echo 1 > /proc/scsi/sg/allow_dio'\n", q_len,
>>> dirio_count);
>>> ./testing/sg_tst_bidi.c
>>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>>> ./examples/sgq_dd.c
>>>
>>> That is 6 (not 38) by my count.
>>
>> Hi Doug,
>>
>> This is the command I ran:
>>
>> $ git grep /proc/scsi | wc -l
>> 38
>>
>> I think your query excludes scripts/rescan-scsi-bus.sh.
>>
>> Bart.
> 
> Here's the full list to ensure the discussion doesn't overlook anything:
> 
> sg3_utils-1.44$ grep -R /proc/scsi .
> ./src/sg_read.c:static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> ./src/sgp_dd.c:static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> ./src/sgm_dd.c:static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> ./src/sg_dd.c:static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> ./scripts/rescan-scsi-bus.sh:# Return hosts. /proc/scsi/HOSTADAPTER/? must exist
> ./scripts/rescan-scsi-bus.sh:  for driverdir in /proc/scsi/*; do
> ./scripts/rescan-scsi-bus.sh:    driver=${driverdir#/proc/scsi/}
> ./scripts/rescan-scsi-bus.sh:      name=${hostdir#/proc/scsi/*/}
> ./scripts/rescan-scsi-bus.sh:# Get /proc/scsi/scsi info for device $host:$channel:$id:$lun
> ./scripts/rescan-scsi-bus.sh:    SCSISTR=$(grep -A "$LN" -e "$grepstr" /proc/scsi/scsi)
> ./scripts/rescan-scsi-bus.sh:    DRV=`grep 'Attached drivers:' /proc/scsi/scsi 2>/dev/null`
> ./scripts/rescan-scsi-bus.sh:      echo "scsi report-devs 1" >/proc/scsi/scsi
> ./scripts/rescan-scsi-bus.sh:      DRV=`grep 'Attached drivers:' /proc/scsi/scsi 2>/dev/null`
> ./scripts/rescan-scsi-bus.sh:      echo "scsi report-devs 0" >/proc/scsi/scsi
> ./scripts/rescan-scsi-bus.sh:# Outputs description from /proc/scsi/scsi (unless arg passed)
> ./scripts/rescan-scsi-bus.sh:        echo "scsi remove-single-device $devnr" > /proc/scsi/scsi
> ./scripts/rescan-scsi-bus.sh:          echo "scsi add-single-device $devnr" > /proc/scsi/scsi
> ./scripts/rescan-scsi-bus.sh:      echo "scsi add-single-device $devnr" > /proc/scsi/scsi
> ./scripts/rescan-scsi-bus.sh:      echo "scsi add-single-device $devnr" > /proc/scsi/scsi
> ./scripts/rescan-scsi-bus.sh:      echo "scsi add-single-device $host $channel $id $SCAN_WILD_CARD" > /proc/scsi/scsi
> ./scripts/rescan-scsi-bus.sh:if test ! -d /sys/class/scsi_host/ -a ! -d /proc/scsi/; then
> ./ChangeLog:    /proc/scsi/sg/allow_dio is '0'
> ./ChangeLog:  - change sg_debug to call system("cat /proc/scsi/sg/debug");
> ./suse/sg3_utils.changes:  * Support systems without /proc/scsi
> ./examples/sgq_dd.c:static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> ./doc/sg_read.8:If direct IO is selected and /proc/scsi/sg/allow_dio
> ./doc/sg_read.8:"echo 1 > /proc/scsi/sg/allow_dio". An alternate way to avoid the
> ./doc/sg_map.8:observing the output of the command: "cat /proc/scsi/scsi".
> ./doc/sgp_dd.8:at completion. If direct IO is selected and /proc/scsi/sg/allow_dio
> ./doc/sgp_dd.8:this at completion. If direct IO is selected and /proc/scsi/sg/allow_dio
> ./doc/sgp_dd.8:mapping to SCSI block devices should be checked with 'cat /proc/scsi/scsi'
> ./doc/sg_dd.8:notes this at completion. If direct IO is selected and /proc/scsi/sg/allow_dio
> ./doc/sg_dd.8:this at completion. If direct IO is selected and /proc/scsi/sg/allow_dio
> ./doc/sg_dd.8:with 'echo 1 > /proc/scsi/sg/allow_dio'.
> ./doc/sg_dd.8:mapping to SCSI block devices should be checked with 'cat /proc/scsi/scsi',
> 
> 
As mentioned, rescan-scsi-bus.sh is keeping references to /proc/scsi as
a fall back only, as it's meant to work kernel independent. Per default
it'll be using /sys, and will happily work without /proc/scsi.

So it's really only /proc/scsi/sg which carries some meaningful
information; maybe we should move/copy it to somewhere else.

I personally like getting rid of /proc/scsi.

Cheers,

-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)

Re: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default

[Douglas Gilbert]

On 2019-07-05 3:22 a.m., Hannes Reinecke wrote:
> On 6/18/19 7:43 PM, Elliott, Robert (Servers) wrote:
>>
>>
>>> -----Original Message-----
>>> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Bart
>>> Van Assche
>>> Sent: Monday, June 17, 2019 10:28 PM
>>> To: dgilbert@interlog.com; Marc Gonzalez <marc.w.gonzalez@free.fr>; James Bottomley
>>> <jejb@linux.ibm.com>; Martin Petersen <martin.petersen@oracle.com>
>>> Cc: SCSI <linux-scsi@vger.kernel.org>; LKML <linux-kernel@vger.kernel.org>; Christoph Hellwig
>>> <hch@lst.de>
>>> Subject: Re: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default
>>>
>>> On 6/17/19 5:35 PM, Douglas Gilbert wrote:
>>>> For sg3_utils:
>>>>
>>>> $ find . -name '*.c' -exec grep "/proc/scsi" {} \; -print
>>>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>>>> ./src/sg_read.c
>>>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>>>> ./src/sgp_dd.c
>>>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>>>> ./src/sgm_dd.c
>>>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>>>> ./src/sg_dd.c
>>>>                   "'echo 1 > /proc/scsi/sg/allow_dio'\n", q_len,
>>>> dirio_count);
>>>> ./testing/sg_tst_bidi.c
>>>> static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>>>> ./examples/sgq_dd.c
>>>>
>>>> That is 6 (not 38) by my count.
>>>
>>> Hi Doug,
>>>
>>> This is the command I ran:
>>>
>>> $ git grep /proc/scsi | wc -l
>>> 38
>>>
>>> I think your query excludes scripts/rescan-scsi-bus.sh.
>>>
>>> Bart.
>>
>> Here's the full list to ensure the discussion doesn't overlook anything:
>>
>> sg3_utils-1.44$ grep -R /proc/scsi .
>> ./src/sg_read.c:static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>> ./src/sgp_dd.c:static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>> ./src/sgm_dd.c:static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>> ./src/sg_dd.c:static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>> ./scripts/rescan-scsi-bus.sh:# Return hosts. /proc/scsi/HOSTADAPTER/? must exist
>> ./scripts/rescan-scsi-bus.sh:  for driverdir in /proc/scsi/*; do
>> ./scripts/rescan-scsi-bus.sh:    driver=${driverdir#/proc/scsi/}
>> ./scripts/rescan-scsi-bus.sh:      name=${hostdir#/proc/scsi/*/}
>> ./scripts/rescan-scsi-bus.sh:# Get /proc/scsi/scsi info for device $host:$channel:$id:$lun
>> ./scripts/rescan-scsi-bus.sh:    SCSISTR=$(grep -A "$LN" -e "$grepstr" /proc/scsi/scsi)
>> ./scripts/rescan-scsi-bus.sh:    DRV=`grep 'Attached drivers:' /proc/scsi/scsi 2>/dev/null`
>> ./scripts/rescan-scsi-bus.sh:      echo "scsi report-devs 1" >/proc/scsi/scsi
>> ./scripts/rescan-scsi-bus.sh:      DRV=`grep 'Attached drivers:' /proc/scsi/scsi 2>/dev/null`
>> ./scripts/rescan-scsi-bus.sh:      echo "scsi report-devs 0" >/proc/scsi/scsi
>> ./scripts/rescan-scsi-bus.sh:# Outputs description from /proc/scsi/scsi (unless arg passed)
>> ./scripts/rescan-scsi-bus.sh:        echo "scsi remove-single-device $devnr" > /proc/scsi/scsi
>> ./scripts/rescan-scsi-bus.sh:          echo "scsi add-single-device $devnr" > /proc/scsi/scsi
>> ./scripts/rescan-scsi-bus.sh:      echo "scsi add-single-device $devnr" > /proc/scsi/scsi
>> ./scripts/rescan-scsi-bus.sh:      echo "scsi add-single-device $devnr" > /proc/scsi/scsi
>> ./scripts/rescan-scsi-bus.sh:      echo "scsi add-single-device $host $channel $id $SCAN_WILD_CARD" > /proc/scsi/scsi
>> ./scripts/rescan-scsi-bus.sh:if test ! -d /sys/class/scsi_host/ -a ! -d /proc/scsi/; then
>> ./ChangeLog:    /proc/scsi/sg/allow_dio is '0'
>> ./ChangeLog:  - change sg_debug to call system("cat /proc/scsi/sg/debug");
>> ./suse/sg3_utils.changes:  * Support systems without /proc/scsi
>> ./examples/sgq_dd.c:static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
>> ./doc/sg_read.8:If direct IO is selected and /proc/scsi/sg/allow_dio
>> ./doc/sg_read.8:"echo 1 > /proc/scsi/sg/allow_dio". An alternate way to avoid the
>> ./doc/sg_map.8:observing the output of the command: "cat /proc/scsi/scsi".
>> ./doc/sgp_dd.8:at completion. If direct IO is selected and /proc/scsi/sg/allow_dio
>> ./doc/sgp_dd.8:this at completion. If direct IO is selected and /proc/scsi/sg/allow_dio
>> ./doc/sgp_dd.8:mapping to SCSI block devices should be checked with 'cat /proc/scsi/scsi'
>> ./doc/sg_dd.8:notes this at completion. If direct IO is selected and /proc/scsi/sg/allow_dio
>> ./doc/sg_dd.8:this at completion. If direct IO is selected and /proc/scsi/sg/allow_dio
>> ./doc/sg_dd.8:with 'echo 1 > /proc/scsi/sg/allow_dio'.
>> ./doc/sg_dd.8:mapping to SCSI block devices should be checked with 'cat /proc/scsi/scsi',
>>
>>
> As mentioned, rescan-scsi-bus.sh is keeping references to /proc/scsi as
> a fall back only, as it's meant to work kernel independent. Per default
> it'll be using /sys, and will happily work without /proc/scsi.
> 
> So it's really only /proc/scsi/sg which carries some meaningful
> information; maybe we should move/copy it to somewhere else.
> 
> I personally like getting rid of /proc/scsi.

/proc/scsi/device_info doesn't seem to be in sysfs.

Could the contents of /proc/scsi/sg/* be placed in
/sys/class/scsi_generic/* ? Currently that directory only has symlinks
to the sg devices.

Doug Gilbert

Re: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default

[Hannes Reinecke]

On 7/5/19 7:53 PM, Douglas Gilbert wrote:
> On 2019-07-05 3:22 a.m., Hannes Reinecke wrote:
[ .. ]
>> As mentioned, rescan-scsi-bus.sh is keeping references to /proc/scsi as
>> a fall back only, as it's meant to work kernel independent. Per default
>> it'll be using /sys, and will happily work without /proc/scsi.
>>
>> So it's really only /proc/scsi/sg which carries some meaningful
>> information; maybe we should move/copy it to somewhere else.
>>
>> I personally like getting rid of /proc/scsi.
> 
> /proc/scsi/device_info doesn't seem to be in sysfs.
> 
> Could the contents of /proc/scsi/sg/* be placed in
> /sys/class/scsi_generic/* ? Currently that directory only has symlinks
> to the sg devices.
> 
The sg parameters are already available in /sys/module/sg/parameters;
so from that perspective I feel we're good.
Problem is /proc/scsi/device_info, for which we currently don't have any
other location to store it at.
Hmm.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)

Re: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default

[Douglas Gilbert]

On 2019-07-08 2:01 a.m., Hannes Reinecke wrote:
> On 7/5/19 7:53 PM, Douglas Gilbert wrote:
>> On 2019-07-05 3:22 a.m., Hannes Reinecke wrote:
> [ .. ]
>>> As mentioned, rescan-scsi-bus.sh is keeping references to /proc/scsi as
>>> a fall back only, as it's meant to work kernel independent. Per default
>>> it'll be using /sys, and will happily work without /proc/scsi.
>>>
>>> So it's really only /proc/scsi/sg which carries some meaningful
>>> information; maybe we should move/copy it to somewhere else.
>>>
>>> I personally like getting rid of /proc/scsi.
>>
>> /proc/scsi/device_info doesn't seem to be in sysfs.
>>
>> Could the contents of /proc/scsi/sg/* be placed in
>> /sys/class/scsi_generic/* ? Currently that directory only has symlinks
>> to the sg devices.
>>
> The sg parameters are already available in /sys/module/sg/parameters;
> so from that perspective I feel we're good.

# ls /sys/module/sg/parameters/
allow_dio  def_reserved_size  scatter_elem_sz

# ls /proc/scsi/sg/
allow_dio  debug  def_reserved_size  device_hdr  devices  device_strs
red_debug  version

So that doesn't work, what are in 'parameters' are passed in at
module/driver initialization. Back to my original question: Could the
contents of /proc/scsi/sg/* be placed in /sys/class/scsi_generic/* ?

> Problem is /proc/scsi/device_info, for which we currently don't have any
> other location to store it at.
> Hmm.

Doug Gilbert