| Message ID | 2de15293-b9be-4d41-bc67-a69417f27f7a@free.fr (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v1] scsi: Don't select SCSI_PROC_FS by default | expand |
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.
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
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. --
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.
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.
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.
> -----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',
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.
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".
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.
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.
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. --
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
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. --
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
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,
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
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
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
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
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(-)