Re: CVS commit: src/sys/dev/scsipi
I don't think this should be reverted, because LUN 0 must exist, but if there is no device on it, it will report "NOT PRESENT". We do not want the scan to stop in this case, but it should continue with other LUNs (such as those found through REPORT LUNS in the future). Kind regards, + Kimmo On Fri, Sep 18, 2020 at 03:04:25PM +, Jonathan A. Kollasch wrote: > Module Name: src > Committed By: jakllsch > Date: Fri Sep 18 15:04:25 UTC 2020 > > Modified Files: > src/sys/dev/scsipi: scsiconf.c > > Log Message: > Revert scsiconf.c 1.288, it only worked for LUN 1. > > vioscsi(4) now sets PQUIRK_FORCELUNS, which fixes the original issue for > all LUNs. > > To-do: should issue REPORT LUNS and use the information it returns to > probe LUNs in an optimized way. > > > To generate a diff of this commit: > cvs rdiff -u -r1.289 -r1.290 src/sys/dev/scsipi/scsiconf.c > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. >
Re: CVS commit: src/sys/dev/scsipi
On Sun, Jul 12, 2020 at 12:05:37AM +0700, Robert Elz wrote: > Just to make things clear here, the LUN you're talking about is not > the scsi unit number (which is what I think Martin was referring to) > but a sub-device number within a single scsi ID. Right? Correct. I should have written "SCSI target" instead of "SCSI bus" in my commit message to avoid confusion. In both of the use cases I highlighted (Proxmox and Linode), the disks are always on SCSI ID 0. However, the "problematic" disks are on LUN 1 instead of LUN 0. Out of curiosity, I just added a "scsi2" disk to the Proxmox VM. It has been placed on LUN 2, and NetBSD does not pick it up even with this patch... FreeBSD does (although the earlier LUNs are clearly causing unexpected output from the "pass" attachments), so I guess I might be looking at this a bit more. It would be nice to have (at least debug) output about the LUN that terminates the scan loop. Will probably open a ticket with Proxmox, too, in an attempt to put a stop to this unnecessary wasteful skipping of perfectly good LUN numbers. + Kimmo
Re: CVS commit: src/sys/dev/scsipi
Date:Sat, 11 Jul 2020 18:24:51 +0300 From:Kimmo Suominen Message-ID: <20200711152451.ga1...@homeworld.netbsd.org> | On Sat, Jul 11, 2020 at 05:00:02PM +0200, Martin Husemann wrote: | > I don't understand the change. When was this broken? This has always worked | > for me e.g. with the sd0 at LUN 3 and the controller at 6 or 7. | | I think all real SCSI hardware I've had has always just only had LUN 0, | and each disk has been on its own SCSI ID (target). Just to make things clear here, the LUN you're talking about is not the scsi unit number (which is what I think Martin was referring to) but a sub-device number within a single scsi ID. Right? In real scsi hardware, the only place I think I've ever seen other than LUN 0 is in a raid array device, where there is a single scsi bus attachment, with a single ID, and then each raid volume created gets a different LUN (not all scsi raids work that way I think, but some do). kre
Re: CVS commit: src/sys/dev/scsipi
On Sat, Jul 11, 2020 at 06:24:51PM +0300, Kimmo Suominen wrote: > I think all real SCSI hardware I've had has always just only had LUN 0, > and each disk has been on its own SCSI ID (target). Yes, I confused ID and LUN here - just ignore me. Martin
Re: CVS commit: src/sys/dev/scsipi
On Sat, Jul 11, 2020 at 05:00:02PM +0200, Martin Husemann wrote: > I don't understand the change. When was this broken? This has always worked > for me e.g. with the sd0 at LUN 3 and the controller at 6 or 7. I think all real SCSI hardware I've had has always just only had LUN 0, and each disk has been on its own SCSI ID (target). > Or is there something special in your setup? Well, this is how Proxmox and Linode do it, and with this change it is possible to install and use NetBSD on those platforms. Of course, anyone running their own Proxmox could just avoid the "Virtio SCSI single" controller type (or SCSI disks altogether), but there is no user or admin control for specifying the LUN ID of a disk. With a platform like Linode, you are stuck with the configuration it creates for you from a boot profile (or other UI object). Although Linode is usually responsive to bug reports, here it is not clear if the bug is in how they do things or how NetBSD behaves. With this change it is possible to run the NetBSD installer just like the FreeBSD one (or any other "custom distro") on Linode. Kind regards, + Kimmo
Re: CVS commit: src/sys/dev/scsipi
On Sat, Jul 11, 2020 at 05:57:46PM +0300, Kimmo Suominen wrote: > On Sat, Jul 11, 2020 at 05:47:34PM +0300, Jukka Ruohonen wrote: > > I'd reckon a pullup to NetBSD 9 would be in order? > > Yes, I was just waiting to be able to link to mail-index. I had > already checked that the patch applies cleanly to both netbsd-9 > and netbsd-8. I don't understand the change. When was this broken? This has always worked for me e.g. with the sd0 at LUN 3 and the controller at 6 or 7. Or is there something special in your setup? Martin
Re: CVS commit: src/sys/dev/scsipi
On Sat, Jul 11, 2020 at 05:47:34PM +0300, Jukka Ruohonen wrote: > I'd reckon a pullup to NetBSD 9 would be in order? Yes, I was just waiting to be able to link to mail-index. I had already checked that the patch applies cleanly to both netbsd-9 and netbsd-8. http://releng.netbsd.org/cgi-bin/req-9.cgi?show=1000 http://releng.netbsd.org/cgi-bin/req-8.cgi?show=1571 Cheers, + Kimmo
Re: CVS commit: src/sys/dev/scsipi
On Sat, Jul 11, 2020 at 02:31:46PM +, Kimmo Suominen wrote: > Use case 2: A Linode boot profile with multiple disks results in > the first disk ("sda") on LUN 1, while the second disk ("sdb") is > on LUN 0, each on their own bus. As Linode is quite popular, and supposedly uses a rather similar setup to its competitors (e.g., Vultr), I'd reckon a pullup to NetBSD 9 would be in order? Some of these (e.g., netcup.eu in Europe) even offer off-the-shelf NetBSD 9 images. - Jukka
Re: CVS commit: src/sys/dev/scsipi
On 24.03.2018 16:47, Martin Husemann wrote: > On Sat, Mar 24, 2018 at 09:38:15AM +0100, Thomas Klausner wrote: >> On Sat, Mar 24, 2018 at 09:06:25AM +0100, Michael van Elst wrote: >>> On Sat, Mar 24, 2018 at 02:50:05AM +0100, Kamil Rytarowski wrote: >>> I had to revert this in order to unbreak build. >>> >>> >>> Please never do that. >> >> I think it's appropriate if it's a small patch that breaks the build >> and can easily be backed out without side effects. > > IIRC the official way is: > > - ping the commiter and notify them of the problem > - give a bit of time (like 24h or so) > - invoke core to revert the commit > > See commit guidelines point 11: > > Do not revert other developer's commits. > > If you do not agree with another developer's > commit, do not revert it on your own. Contact the > other developer and explain to him or her the > issues you have with the commit in question. Ask > the other developer to back out the changes > instead of doing it yourself. > > Martin > Thanks! I was trying to reach developers yesterday twice and get feedback what to do, but no echo was received. It was almost a whole-day breakage. Next time I will try with a mail. signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/sys/dev/scsipi
On Sat, Mar 24, 2018 at 09:38:15AM +0100, Thomas Klausner wrote: > On Sat, Mar 24, 2018 at 09:06:25AM +0100, Michael van Elst wrote: > > On Sat, Mar 24, 2018 at 02:50:05AM +0100, Kamil Rytarowski wrote: > > > > > I had to revert this in order to unbreak build. > > > > > > Please never do that. > > I think it's appropriate if it's a small patch that breaks the build > and can easily be backed out without side effects. IIRC the official way is: - ping the commiter and notify them of the problem - give a bit of time (like 24h or so) - invoke core to revert the commit See commit guidelines point 11: Do not revert other developer's commits. If you do not agree with another developer's commit, do not revert it on your own. Contact the other developer and explain to him or her the issues you have with the commit in question. Ask the other developer to back out the changes instead of doing it yourself. Martin
Re: CVS commit: src/sys/dev/scsipi
On Sat, Mar 24, 2018 at 09:06:25AM +0100, Michael van Elst wrote: > On Sat, Mar 24, 2018 at 02:50:05AM +0100, Kamil Rytarowski wrote: > > > I had to revert this in order to unbreak build. > > > Please never do that. I think it's appropriate if it's a small patch that breaks the build and can easily be backed out without side effects. Thomas
Re: CVS commit: src/sys/dev/scsipi
On Sat, Mar 24, 2018 at 02:50:05AM +0100, Kamil Rytarowski wrote: > I had to revert this in order to unbreak build. Please never do that. -- Michael van Elst Internet: mlel...@serpens.de "A potential Snark may lurk in every tree."
Re: CVS commit: src/sys/dev/scsipi
Date:Sat, 24 Mar 2018 02:50:05 +0100 From:Kamil RytarowskiMessage-ID: <8b2f90cf-0c7f-1154-b3fb-c4c600d07...@gmx.com> | > To generate a diff of this commit: | > cvs rdiff -u -r1.231 -r1.232 src/sys/dev/scsipi/st.c | | I had to revert this in order to unbreak build. It looks as if an accompanying .h file update missed getting committed... kre
Re: CVS commit: src/sys/dev/scsipi
On 23.03.2018 07:01, Michael van Elst wrote: > Module Name: src > Committed By: mlelstv > Date: Fri Mar 23 06:01:07 UTC 2018 > > Modified Files: > src/sys/dev/scsipi: st.c > > Log Message: > Use separate lock to protect internal state and release locks when > calling biodone. > > > To generate a diff of this commit: > cvs rdiff -u -r1.231 -r1.232 src/sys/dev/scsipi/st.c > I had to revert this in order to unbreak build. /usr/src/sys/dev/scsipi/st.c: In function 'stattach': /usr/src/sys/dev/scsipi/st.c:398:16: error: 'struct st_softc' has no member named 'buf_defer'; did you mean 'buf_queue'? bufq_alloc(>buf_defer, "fcfs", 0); ^~ /usr/src/sys/dev/scsipi/st.c:400:16: error: 'struct st_softc' has no member named 'sc_iolock'; did you mean 'sc_callout'? mutex_init(>sc_iolock, MUTEX_DEFAULT, IPL_VM); ^~ /usr/src/sys/dev/scsipi/st.c: In function 'stdetach': /usr/src/sys/dev/scsipi/st.c:451:15: error: 'struct st_softc' has no member named 'buf_defer'; did you mean 'buf_queue'? bufq_drain(st->buf_defer); ^~ /usr/src/sys/dev/scsipi/st.c:459:14: error: 'struct st_softc' has no member named 'buf_defer'; did you mean 'buf_queue'? bufq_free(st->buf_defer); ^~ /usr/src/sys/dev/scsipi/st.c:461:19: error: 'struct st_softc' has no member named 'sc_iolock'; did you mean 'sc_callout'? mutex_destroy(>sc_iolock); ^~ /usr/src/sys/dev/scsipi/st.c: In function 'ststart': /usr/src/sys/dev/scsipi/st.c:1270:17: error: 'struct st_softc' has no member named 'sc_iolock'; did you mean 'sc_callout'? mutex_enter(>sc_iolock); ^~ /usr/src/sys/dev/scsipi/st.c:1272:26: error: 'struct st_softc' has no member named 'buf_defer'; did you mean 'buf_queue'? while ((bp = bufq_get(st->buf_defer)) != NULL ^~ /usr/src/sys/dev/scsipi/st.c:1276:17: error: 'struct st_softc' has no member named 'sc_iolock'; did you mean 'sc_callout'? mutex_exit(>sc_iolock); ^~ /usr/src/sys/dev/scsipi/st.c:1280:18: error: 'struct st_softc' has no member named 'sc_iolock'; did you mean 'sc_callout'? mutex_enter(>sc_iolock); ^~ /usr/src/sys/dev/scsipi/st.c:1285:15: error: 'struct st_softc' has no member named 'buf_defer'; did you mean 'buf_queue'? bufq_put(st->buf_defer, bp); ^~ /usr/src/sys/dev/scsipi/st.c:1288:17: error: 'struct st_softc' has no member named 'sc_iolock'; did you mean 'sc_callout'? mutex_exit(>sc_iolock); ^~ /usr/src/sys/dev/scsipi/st.c:1296:18: error: 'struct st_softc' has no member named 'sc_iolock'; did you mean 'sc_callout'? mutex_enter(>sc_iolock); ^~ /usr/src/sys/dev/scsipi/st.c:1299:16: error: 'struct st_softc' has no member named 'sc_iolock'; did you mean 'sc_callout'? mutex_exit(>sc_iolock); ^~ /usr/src/sys/dev/scsipi/st.c: In function 'stdone': /usr/src/sys/dev/scsipi/st.c:1331:18: error: 'struct st_softc' has no member named 'sc_iolock'; did you mean 'sc_callout'? mutex_enter(>sc_iolock); ^~ /usr/src/sys/dev/scsipi/st.c:1353:17: error: 'struct st_softc' has no member named 'sc_iolock'; did you mean 'sc_callout'? mutex_exit(>sc_iolock); ^~ --- st.o --- *** [st.o] Error code 1 nbmake: stopped in /public/netbsd-root/sys/arch/amd64/compile/GENERIC 1 error nbmake: stopped in /public/netbsd-root/sys/arch/amd64/compile/GENERIC ERROR: Failed to make all in "/public/netbsd-root/sys/arch/amd64/compile/GENERIC" signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/sys/dev/scsipi
Hi, Today, I found my environment panic while rebooting. As bisecting, it seems the cause is below commit. On 2017/06/18 7:35, Michael van Elst wrote: > Module Name: src > Committed By: mlelstv > Date: Sat Jun 17 22:35:50 UTC 2017 > > Modified Files: > src/sys/dev/scsipi: atapiconf.c cd.c scsi_base.c scsiconf.c > scsipi_base.c sd.c ss.c st.c > > Log Message: > The atapibus detach path did hold the channel mutex while calling into > autoconf, > which would trigger a panic when unplugging a USB ATAPI CDROM. > > Align detach code for scsibus and atapibus to fix this. > > Also avoid races when detaching devices by replacing callout_stop with > callout_halt. > > > To generate a diff of this commit: > cvs rdiff -u -r1.90 -r1.91 src/sys/dev/scsipi/atapiconf.c > cvs rdiff -u -r1.340 -r1.341 src/sys/dev/scsipi/cd.c > cvs rdiff -u -r1.91 -r1.92 src/sys/dev/scsipi/scsi_base.c > cvs rdiff -u -r1.279 -r1.280 src/sys/dev/scsipi/scsiconf.c > cvs rdiff -u -r1.175 -r1.176 src/sys/dev/scsipi/scsipi_base.c > cvs rdiff -u -r1.324 -r1.325 src/sys/dev/scsipi/sd.c > cvs rdiff -u -r1.88 -r1.89 src/sys/dev/scsipi/ss.c > cvs rdiff -u -r1.230 -r1.231 src/sys/dev/scsipi/st.c > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. The panic message is here. // snip Jun 19 22:48:28 syncing disks... done config_detach: detached device scsibus0 has children sd0 Skipping crash dump on recursive panic panic: config_detach fatal breakpoint trap in supervisor mode trap type 1 code 0 rip 0x802249f5 cs 0x8 rflags 0x246 cr2 0x70c8884aa38f ilevel 0 rsp 0xe4011099cc80 curlwp 0xe4027de12140 pid 632.1 lowest kstack 0xe401109992c0 Stopped in pid 632.1 (reboot) atnetbsd:breakpoint+0x5: leave db{2}> bt breakpoint() at netbsd:breakpoint+0x5 vpanic() at netbsd:vpanic+0x140 snprintf() at netbsd:snprintf config_detach() at netbsd:config_detach+0x218 config_detach_all() at netbsd:config_detach_all+0x97 cpu_reboot() at netbsd:cpu_reboot+0x173 sys_reboot() at netbsd:sys_reboot+0x75 syscall() at netbsd:syscall+0x1d8 --- syscall (number 208) --- 70c88843e5ba: The addr2line of config_detach()+0x218 is here. https://nxr.netbsd.org/xref/src/sys/kern/subr_autoconf.c#1728 My environment is amd64 which use sd0(USB Flash) as root filesystem. Could someone have any solution? Thanks, -- // Internet Initiative Japan Inc. Device Engineering Section, IoT Platform Development Department, Network Division, Technology Unit Kengo NAKAHARA
Re: CVS commit: src/sys/dev/scsipi
On Tue, Apr 11, 2017 at 02:13:42AM +, Christos Zoulas wrote: > Can't we fix this a different way? What's the problem? The log description might have been confusing. It just makes no sense to try to autoload any module while / is not yet available. It may make sense to move the check for this into the module auto load code though. Martin
Re: CVS commit: src/sys/dev/scsipi
In article <20170410215338.12a45f...@cvs.netbsd.org>, Jaromir Dolecekwrote: >-=-=-=-=-=- > >just do not autoload scsiverbose module, it causes deadlock if it happens >while root fs is being mounted > >adresses second part of PR kern/52147 by Michael van Elst, thank you Can't we fix this a different way? What's the problem? christos
Re: CVS commit: src/sys/dev/scsipi
Hi Erik ! I agree that 5 minutes is a really long time. Actually the inventory command will wait even longer (5 min per element + 10 minutes as safeguard - I didn't change that). Generating a uprintf would mean to manage a separate callout and using, I believe, the tprintf() call from the callout function. I am not sure whether that is worth the trouble. What timeout should we take for the uprintf ? People with a slow device with a waiting indicator every 10 seconds will not gain any additional information other that they happen to have a slow device. Mostly changers are used in automated environments. I am not looking at night at the operations of our changers, but I am really annoyed when work is being aborted because of the scsi timeout being too short. tl;dr I'd prefer not to add 'interactive experience' support to the kernel, Maybe someone can wip up some tools(commands) for interactive usage. Maybe a warning about possible long operation times in the chio man page could help. Frank On 08/10/13 00:06, Erik Fair wrote: On Aug 9, 2013, at 12:58 , Frank Kardel kar...@netbsd.org wrote: Module Name:src Committed By: kardel Date: Fri Aug 9 19:58:44 UTC 2013 Modified Files: src/sys/dev/scsipi: ch.c Log Message: bump command timeout to 5 minutes. several types of changers (Overland PowerLoader, Dell PowerVault) have been exceeding the 100 sec limit aborting a perfectly (slowly) progressing operation. I think the kernel should uprintf(9) a notice to the effect that it has exceeded some (sooner than the 5 minute timeout) threshold and that it's really waiting for the device. Five minutes is a very long time for a timeout involving nominally local I/O devices. Without some progress indicator, users are likely to begin beating the system up (power cycle, etc) absent some persuasion to be patient. Erik f...@netbsd.org
Re: CVS commit: src/sys/dev/scsipi
Erik, I agree with this very much. Actually I have been toying around with this idea quite some time. My thoughts where to build a sysctl tree for all scsi commands with their default values and another level of sysctl timeout nodes where the vendor and device name or driver name is part of the hierarchy to override timeouts for some commands. Solaris has, I believe some of those knobs. So far I didn't manage to invest the time for a complete implementation. But maybe, as we currently are only suffering with changers, it would be sufficient to add a single timeout knob for the changers. Like dev.ch0.scsi_timeout=300 (s) How about that? Frank On 08/10/13 00:14, Erik Fair wrote: On Aug 9, 2013, at 12:58 , Frank Kardel kar...@netbsd.org wrote: Module Name:src Committed By: kardel Date: Fri Aug 9 19:58:44 UTC 2013 Modified Files: src/sys/dev/scsipi: ch.c Log Message: bump command timeout to 5 minutes. several types of changers (Overland PowerLoader, Dell PowerVault) have been exceeding the 100 sec limit aborting a perfectly (slowly) progressing operation. Upon further reflection, I believe that this timeout value should be a device-specific tunable parameter because there is such wide variation in changer behavior/performance; perhaps by kernel config, or by sysctl(8) on a per-device-node basis. Or some other mechanism you prefer. I believe that baking it into the kernel as a constant is not good design because we'll just see another commit just like this one at some later date, continuing to patch around the design problem. Erik f...@netbsd.org
Re: CVS commit: src/sys/dev/scsipi
On Aug 9, 2013, at 12:58 , Frank Kardel kar...@netbsd.org wrote: Module Name: src Committed By: kardel Date: Fri Aug 9 19:58:44 UTC 2013 Modified Files: src/sys/dev/scsipi: ch.c Log Message: bump command timeout to 5 minutes. several types of changers (Overland PowerLoader, Dell PowerVault) have been exceeding the 100 sec limit aborting a perfectly (slowly) progressing operation. I think the kernel should uprintf(9) a notice to the effect that it has exceeded some (sooner than the 5 minute timeout) threshold and that it's really waiting for the device. Five minutes is a very long time for a timeout involving nominally local I/O devices. Without some progress indicator, users are likely to begin beating the system up (power cycle, etc) absent some persuasion to be patient. Erik f...@netbsd.org
Re: CVS commit: src/sys/dev/scsipi
On Aug 9, 2013, at 12:58 , Frank Kardel kar...@netbsd.org wrote: Module Name: src Committed By: kardel Date: Fri Aug 9 19:58:44 UTC 2013 Modified Files: src/sys/dev/scsipi: ch.c Log Message: bump command timeout to 5 minutes. several types of changers (Overland PowerLoader, Dell PowerVault) have been exceeding the 100 sec limit aborting a perfectly (slowly) progressing operation. Upon further reflection, I believe that this timeout value should be a device-specific tunable parameter because there is such wide variation in changer behavior/performance; perhaps by kernel config, or by sysctl(8) on a per-device-node basis. Or some other mechanism you prefer. I believe that baking it into the kernel as a constant is not good design because we'll just see another commit just like this one at some later date, continuing to patch around the design problem. Erik f...@netbsd.org
Re: CVS commit: src/sys/dev/scsipi
On Thu, Apr 19, 2012 at 06:39:29PM -0600, Warner Losh wrote: FreeBSD started out with MPSAFE and then went to NEEDS_GIANT as flags for the drivers. I'm with Matthew: patch the drivers that are broken (or not known to be safe) and then you have a convenient thing to grep for when you want to expand the drivers that are safe. Much better that way, and it turned out to be a big win in FreeBSD when we went from opt-in MPSAFE to out-out... But we're still in the MPSAFE world, not yet at NEEDS_GIANT. Very few drivers are MPSAFE in NetBSD, so almost all of them need to be patched. -- Manuel Bouyer bou...@antioche.eu.org NetBSD: 26 ans d'experience feront toujours la difference --
Re: CVS commit: src/sys/dev/scsipi
On Fri, Apr 20, 2012 at 09:37:00AM +0200, Manuel Bouyer wrote: we've never had autoconfig run with the kernel lock AFAICT, so this assumption has never been true. So this is a bug. The contract was really that spl-locked drivers would continue to work as is when fine-grained locking was introduced. I don't think it's reasonable to require every (but the few already MP-safe) drivers to take KERNEL_LOCK in their attach routine. So autoconf should check and take the kernel lock before calling the attach routine of a non-MPSAFE driver. If it doesn't already do this, it's wrong. Meanwhile, since scsipi is not MPSAFE, whoever is calling into scsipi without taking the kernel lock first is wrong, so the change at the beginning of this thread should be reverted. (Furthermore, scsipi is itself wrong. It is a core component; it should be MPSAFE.) -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/sys/dev/scsipi
On Fri, Apr 20, 2012 at 07:46:35AM +, David Holland wrote: On Fri, Apr 20, 2012 at 09:37:00AM +0200, Manuel Bouyer wrote: we've never had autoconfig run with the kernel lock AFAICT, so this assumption has never been true. So this is a bug. The contract was really that spl-locked drivers would continue to work as is when fine-grained locking was introduced. I don't think it's reasonable to require every (but the few already MP-safe) drivers to take KERNEL_LOCK in their attach routine. So autoconf should check and take the kernel lock before calling the attach routine of a non-MPSAFE driver. If it doesn't already do this, it's wrong. I agree :) Meanwhile, since scsipi is not MPSAFE, whoever is calling into scsipi without taking the kernel lock first is wrong, so the change at the beginning of this thread should be reverted. I also agree, in light of this discussion (I though that KERNEL_LOCK was not held while cold on purpose, but it seems that the issue is at another level, and more annoying). But I find it acceptable as an interim solution, it at last allows attaching scsi drivers that are probed at boot. (Furthermore, scsipi is itself wrong. It is a core component; it should be MPSAFE.) I also agree. So should be ata, if_ethersubr, etc ... that's a lot of work. -- Manuel Bouyer bou...@antioche.eu.org NetBSD: 26 ans d'experience feront toujours la difference --
Re: CVS commit: src/sys/dev/scsipi
On Fri, Apr 20, 2012 at 06:06:35PM +1000, matthew green wrote: we've never had autoconfig run with the kernel lock AFAICT, so this assumption has never been true. So this is a bug. The contract was really that spl-locked drivers would continue to work as is when fine-grained locking was introduced. there are no actual bugs or races here. this is about cleanup and ensuring code runs how it expects. And that's the point: all code that use spl-style locking expects to be called with KERNEL_LOCK held. And this includes the attach routine. I don't think it's reasonable to require every (but the few already MP-safe) drivers to take KERNEL_LOCK in their attach routine. i'm not saying that they should. i'm saying that scsipi drivers, regardless of whether they are fully MPSAFE or not, should now arrange to have the kernel lock held when calling into scsipi at all times. this really has nothing to do with attach, except that attach is problem to fix currently in some drivers. Almost all scsipi drivers ... It's not only IPL_VM drivers, it'a all driver wich don't register and interrupt handler explicitely marked MPSAFE (for example, with PCI_INTR_MPSAFE for PCI drivers). interesting; i hadn't seen PCI_INTR_MPSAFE before. however, it seems ignored / unused. the x86 pci_intr_setattr() does do something but it's never called. none of the rest do anything. this just means there's no PCI MP-SAFE driver yet ? my point is that drivers get interrupt protection from asking for an IPL_VM interrupt. that's what tells the MD code to make sure the kernel lock is held for these interrupt handlers. Or explicitely declared MPSAFE. at last x86 allows this. these KASSERT()s have been in place for a long time now and, until your change, we've fixed it at the caller level not worked around it at the scsipi level. AFAIK the only drivers that has been changed are ncr53c9x and USB, and they are MP-safe (they already use mutex instead of spl locking). usb is not MP safe in -current, only on the usbmp branch. all those drivers take care of taking the kernel lock when calling into the scsipi code since they almost *always* run when not cold anyway. So things were fixed at the wrong place. Any non-MPSAFE driver is at risk right now i don't understand. eg, the usbmp code never calls into scsipi during autoconfig. usb is special in this regard. these asserts aren't specific to autoconfig, and IIRC, most drivers do not call into scsipi during attach. Most if not all regular scsi drivers do: they do a config_found() in their attach routine to attach the scsibus. ATA does it from a kernel thread. your change can't work with the drivers that are broken without the major regression of holding kernel lock over autoconfig, or, never ever having them be modules or detached/reattached. Maybe non-MPSAFE drivers can't be modules or detached/reattached, indeed. that's not true. eg, radeondrm works just fine being MP safe or not. (it actually is MP safe, though it doesn't announce this to any of the rest of the system currently.) Yes, of course, unless there's a KASSERT, things will run fine most of the time. That's the problem with race conditions: it doesn't reliably fail. that's not what i said. what i said is that it is very trivial to have a non MPSAFE-driver handle being a module, being detached, and being reattached. The discussion is about KERNEL_LOCK, and possible race of running non-MPSAFE code without it. No, it's not specific to scsipi. It happens that scsipi asserts that KERNEL_LOCK is held, but any non-MPSAFE drivers needs to be called with KERNEL_LOCK held. why? we've never done this for several years now and there's no reason why we think there is a problem with it currently. why did you add this KASSERT to scsipi then ? we've actually run the interrupt part of autoconfig with multiple threads since sometime between netbsd 4.0 and 5.0. this doesn't mean the races don't exists, just that we don't run into them most of the time. Maybe that in some common case there's no possible race, but that would be by accident, not by design. as in aside, you should make drvctl -d work for any drivers you maintain. everyone (tm) should. irrespective of modules or otherwise, the end result of it working tends to be bug fixes. It's hard to drvctl -d the root device ... eh, pxe is your friend. :-) I can pxe boot the kernel; having root on NFS is another things. If we want this to work for non-MPSAFE driver, then this has to be fixed in autoconf (if the problem really exists, which I've not looked at yet). i don't believe there is a problem in autoconfig. there is a problem with some (probably most) scsi drivers here, but that is about it. it is true that the contracts have changed, but
re: CVS commit: src/sys/dev/scsipi
Module Name: src Committed By: bouyer Date: Wed Apr 18 20:37:49 UTC 2012 Modified Files: src/sys/dev/scsipi: scsipi_base.c Log Message: Fix KASSERT(): autoconf doesn't run under the KERNEL_LOCK this is true, but can you please fix it differently? ie autoconf isn't cold. most of scsi autoconf runs after cold is gone, so i'd rather make whatever callers deal with it so that they work cold or not. thanks. .mrg.
Re: CVS commit: src/sys/dev/scsipi
On Thu, Apr 19, 2012 at 04:41:04PM +1000, matthew green wrote: Module Name:src Committed By: bouyer Date: Wed Apr 18 20:37:49 UTC 2012 Modified Files: src/sys/dev/scsipi: scsipi_base.c Log Message: Fix KASSERT(): autoconf doesn't run under the KERNEL_LOCK this is true, but can you please fix it differently? ie autoconf isn't cold. most of scsi autoconf runs after cold is gone, so i'd rather make whatever callers deal with it so that they work cold or not. But if we're not cold, interrupts are enabled so I hope whatever calls into scsi autoconf runs under the big lock if the underlying driver isn't SMP-safe (and if it is, it's its job to take the KERNEL_LOCK). The problem I've seen is with a driver I'm working on, similar to mfi(4) (and I suspect mfi(4) would have had the same problem): the driver's attach calls config_found_sm_loc() to attach the scsibus while cold, so the big lock isn't helt at this point. If the driver's attach is called after cold (e.g. after a detach/rescan of the pci bus), the driver's attach should be called with KERNEL_LOCK held, or bad things may happen when interrupts are enabled for this driver. What kind of senario do you have in mind ? -- Manuel Bouyer bou...@antioche.eu.org NetBSD: 26 ans d'experience feront toujours la difference --
re: CVS commit: src/sys/dev/scsipi
Module Name: src Committed By: bouyer Date: Wed Apr 18 20:37:49 UTC 2012 Modified Files: src/sys/dev/scsipi: scsipi_base.c Log Message: Fix KASSERT(): autoconf doesn't run under the KERNEL_LOCK this is true, but can you please fix it differently? ie autoconf isn't cold. most of scsi autoconf runs after cold is gone, so i'd rather make whatever callers deal with it so that they work cold or not. But if we're not cold, interrupts are enabled so I hope whatever calls into scsi autoconf runs under the big lock if the underlying driver isn't SMP-safe (and if it is, it's its job to take the KERNEL_LOCK). the point of the KASSERT()'s it to make sure that, until scsipi is made smp safe, calls into scsi are all done with the kernel lock. The problem I've seen is with a driver I'm working on, similar to mfi(4) (and I suspect mfi(4) would have had the same problem): the driver's attach calls config_found_sm_loc() to attach the scsibus while cold, so the big lock isn't helt at this point. yeah - i'm pretty sure i've seen this and fixed it in a few other places rather than the above change. If the driver's attach is called after cold (e.g. after a detach/rescan of the pci bus), the driver's attach should be called with KERNEL_LOCK held, or bad things may happen when interrupts are enabled for this driver. there should be no reliance on cold being set for normal driver attach. it might be a module loaded after boot. how ever the driver is loaded, it will need to work without cold being set. in my mind, the scsi code should try to run regardless of the value of cold and that's why i replied above. What kind of senario do you have in mind ? modules, as above. or simply drvctl -d / -r. IMO, only platform specific code really should depend upon cold. scsipi, as a relatively high level subsystem, should not. thanks. .mrg.
Re: CVS commit: src/sys/dev/scsipi
On Thu, Apr 19, 2012 at 06:25:54PM +1000, matthew green wrote: [...] If the driver's attach is called after cold (e.g. after a detach/rescan of the pci bus), the driver's attach should be called with KERNEL_LOCK held, or bad things may happen when interrupts are enabled for this driver. there should be no reliance on cold being set for normal driver attach. it might be a module loaded after boot. how ever the driver is loaded, it will need to work without cold being set. If it's a module laoded after boot, and the driver is not SMP-safe, its attach function has to be called with KERNEL_LOCK held. Or you may have problems when this driver starts receiving interrupts before its attach function has returned (which is typically the case, interrupt enable occurs somewhere in _attach()). in my mind, the scsi code should try to run regardless of the value of cold and that's why i replied above. What kind of senario do you have in mind ? modules, as above. or simply drvctl -d / -r. IMO, only platform specific code really should depend upon cold. scsipi, as a relatively high level subsystem, should not. But I don't think it's the job of non-SMP-safe drivers to make sure scsipi is called with KERNEL_LOCK held (and scsipi may be the only subsystem to check for KERNEL_LOCK(), but others do rely on it as well - e.g. sys/dev/ata). Either attach() functions should always be called with KERNEL_LOCK(), or for checks as above we have to accept that when cold, locking is not fully up yet and lock checks should be bypassed. -- Manuel Bouyer bou...@antioche.eu.org NetBSD: 26 ans d'experience feront toujours la difference --
re: CVS commit: src/sys/dev/scsipi
If the driver's attach is called after cold (e.g. after a detach/rescan of the pci bus), the driver's attach should be called with KERNEL_LOCK held, or bad things may happen when interrupts are enabled for this driver. there should be no reliance on cold being set for normal driver attach. it might be a module loaded after boot. how ever the driver is loaded, it will need to work without cold being set. If it's a module laoded after boot, and the driver is not SMP-safe, its attach function has to be called with KERNEL_LOCK held. Or you may have problems when this driver starts receiving interrupts before its attach function has returned (which is typically the case, interrupt enable occurs somewhere in _attach()). i don't see how this matters. drivers should never enable interrupts if they're not ready to service them. if this is happening, then it is a driver bug. in my mind, the scsi code should try to run regardless of the value of cold and that's why i replied above. What kind of senario do you have in mind ? modules, as above. or simply drvctl -d / -r. IMO, only platform specific code really should depend upon cold. scsipi, as a relatively high level subsystem, should not. But I don't think it's the job of non-SMP-safe drivers to make sure scsipi is called with KERNEL_LOCK held (and scsipi may be the only subsystem to check for KERNEL_LOCK(), but others do rely on it as well - e.g. sys/dev/ata). Either attach() functions should always be called with KERNEL_LOCK(), or for checks as above we have to accept that when cold, locking is not fully up yet and lock checks should be bypassed. the lack of checks elsewhere is not the point here. we should add them to dev/ata (and more) as well, if they depend upon these. currently, autoconf isn't run with the kernel lock held at any point and i don't think that saying that we should hold it during autoconf is a good change. we should fix the cases that trigger problems when they appear, and it sounds like you've hit one in your new driver. holding the kernel lock here would be a major step backwards. scsipi depends upon kernel lock. thus, callers should arrange for it to be held. since these are drivers calling, it is upto each driver to do this currently. this is what we've done with other problems we have hit as they've arrived. it may be that we have this problem in a large number of drivers but these KASSERT()s have been in place for a long time now and, until your change, we've fixed it at the caller level not worked around it at the scsipi level. your change can't work with the drivers that are broken without the major regression of holding kernel lock over autoconfig, or, never ever having them be modules or detached/reattached. please make drvctl -d and rescan work for your drivers. this will make the checks against cold go away. .mrg.
Re: CVS commit: src/sys/dev/scsipi
On Thu, Apr 19, 2012 at 07:00:56PM +1000, matthew green wrote: If the driver's attach is called after cold (e.g. after a detach/rescan of the pci bus), the driver's attach should be called with KERNEL_LOCK held, or bad things may happen when interrupts are enabled for this driver. there should be no reliance on cold being set for normal driver attach. it might be a module loaded after boot. how ever the driver is loaded, it will need to work without cold being set. If it's a module laoded after boot, and the driver is not SMP-safe, its attach function has to be called with KERNEL_LOCK held. Or you may have problems when this driver starts receiving interrupts before its attach function has returned (which is typically the case, interrupt enable occurs somewhere in _attach()). i don't see how this matters. drivers should never enable interrupts if they're not ready to service them. if this is happening, then it is a driver bug. It looks like we're talking about different things. I'm talking about this senario, with a non-smp-safe driver. foo_attach() { do software and hardware initialisation(); s = splbio(); enable_interrupt(foo_intr()); finish_init_and_attach_children(); splx(s); return; } foo_intr() { } In this context, the driver enables interrupt, but don't expect to have its interrupt handler called before the splx(), because finish_init_and_attach_children() is running at IPL_BIO. But if foo_attach() is not called with KERNEL_LOCK held, another CPU may call foo_intr(), breaking the driver's spl protection. And I don't think it's the driver's business to make sure KERNEL_LOCK is held in it's attach routine: it's a non-MPSAFE driver and it should not even have to know about KERNEL_LOCK. If attach routines can be called without KERNEL_LOCK held after cold, then all non-MPSAFE drivers (i.e. almost all of them) needs to be fixed. in my mind, the scsi code should try to run regardless of the value of cold and that's why i replied above. What kind of senario do you have in mind ? modules, as above. or simply drvctl -d / -r. IMO, only platform specific code really should depend upon cold. scsipi, as a relatively high level subsystem, should not. But I don't think it's the job of non-SMP-safe drivers to make sure scsipi is called with KERNEL_LOCK held (and scsipi may be the only subsystem to check for KERNEL_LOCK(), but others do rely on it as well - e.g. sys/dev/ata). Either attach() functions should always be called with KERNEL_LOCK(), or for checks as above we have to accept that when cold, locking is not fully up yet and lock checks should be bypassed. the lack of checks elsewhere is not the point here. we should add them to dev/ata (and more) as well, if they depend upon these. currently, autoconf isn't run with the kernel lock held at any point and i don't think that saying that we should hold it during autoconf is a good change. we should fix the cases that trigger problems when they appear, and it sounds like you've hit one in your new driver. holding the kernel lock here would be a major step backwards. scsipi depends upon kernel lock. thus, callers should arrange for it to be held. since these are drivers calling, it is upto each driver to do this currently. this is what we've done with other problems we have hit as they've arrived. if the caller is MPSAFE, I agree. Non-MPSAFE driver should not have to care about KERNEL_LOCK (precisely because they are not MPSAFE). The basic assumption about kernel locking was that that driver and subsystems using spl locking would continue to work as it, without changes. If we change this assumption a lot of code needs to be changed (that is, almost all our drivers). it may be that we have this problem in a large number of drivers but these KASSERT()s have been in place for a long time now and, until your change, we've fixed it at the caller level not worked around it at the scsipi level. AFAIK the only drivers that has been changed are ncr53c9x and USB, and they are MP-safe (they already use mutex instead of spl locking). your change can't work with the drivers that are broken without the major regression of holding kernel lock over autoconfig, or, never ever having them be modules or detached/reattached. Maybe non-MPSAFE drivers can't be modules or detached/reattached, indeed. But that's a problem with autoconf not dealing with non-MPSAFE drivers, not the driver themselve (they were working before the MP changes, they should continue to work as-is). If you want autoconf to not run under the KERNEL_LOCK when not needed, then is has to be extended so that MPSAFE drivers can register themselves as MPSAFE, so that non-MPSAFE drivers are still called with KERNEL_LOCK. please make drvctl -d and rescan work for your drivers. this will make the checks against cold go away. I don't
re: CVS commit: src/sys/dev/scsipi
scsipi depends upon kernel lock. thus, callers should arrange for it to be held. since these are drivers calling, it is upto each driver to do this currently. this is what we've done with other problems we have hit as they've arrived. if the caller is MPSAFE, I agree. Non-MPSAFE driver should not have to care about KERNEL_LOCK (precisely because they are not MPSAFE). The basic assumption about kernel locking was that that driver and subsystems using spl locking would continue to work as it, without changes. If we change this assumption a lot of code needs to be changed (that is, almost all our drivers). we've never had autoconfig run with the kernel lock AFAICT, so this assumption has never been true. the thing that does work is that IPL_VM using drivers will automatically get kernel locked in their interrupt handlers. these KASSERT()s have been in place for a long time now and, until your change, we've fixed it at the caller level not worked around it at the scsipi level. AFAIK the only drivers that has been changed are ncr53c9x and USB, and they are MP-safe (they already use mutex instead of spl locking). usb is not MP safe in -current, only on the usbmp branch. all those drivers take care of taking the kernel lock when calling into the scsipi code since they almost *always* run when not cold anyway. infact, the asserts were added in -current due to the usbmp work and the understanding that they would advance the rest of the system. your change can't work with the drivers that are broken without the major regression of holding kernel lock over autoconfig, or, never ever having them be modules or detached/reattached. Maybe non-MPSAFE drivers can't be modules or detached/reattached, indeed. that's not true. eg, radeondrm works just fine being MP safe or not. (it actually is MP safe, though it doesn't announce this to any of the rest of the system currently.) But that's a problem with autoconf not dealing with non-MPSAFE drivers, not the driver themselve (they were working before the MP changes, they should continue to work as-is). If you want autoconf to not run under the KERNEL_LOCK when not needed, then is has to be extended so that MPSAFE drivers can register themselves as MPSAFE, so that non-MPSAFE drivers are still called with KERNEL_LOCK. it seems the wrong answer to me. instead of fixing the one or two drivers you've found problems in, you want to add more infrastructure (IMO clutter) to the system and patch all the drivers that *are* ok. considering that this problem is very specific to scsipi, i don't think that is a viable solution. please make drvctl -d and rescan work for your drivers. this will make the checks against cold go away. I don't want to touch each driver in our tree for drvctl -d/rescan to work. as in aside, you should make drvctl -d work for any drivers you maintain. everyone (tm) should. irrespective of modules or otherwise, the end result of it working tends to be bug fixes. If we want this to work for non-MPSAFE driver, then this has to be fixed in autoconf (if the problem really exists, which I've not looked at yet). i don't believe there is a problem in autoconfig. there is a problem with some (probably most) scsi drivers here, but that is about it. it is true that the contracts have changed, but this is progress after all. the assert exists solely to find places that are wrong and fix them. your change hides this. this isn't about SMPifing a whole driver, but adding a call to taken/release the kernel lock around a few (often just one) calls in a driver. it has never been a difficult change to make. if you have drivers that needs it, i am happy to send patches for testing. .mrg.
Re: CVS commit: src/sys/dev/scsipi
On Apr 19, 2012, at 6:03 PM, matthew green wrote: But that's a problem with autoconf not dealing with non-MPSAFE drivers, not the driver themselve (they were working before the MP changes, they should continue to work as-is). If you want autoconf to not run under the KERNEL_LOCK when not needed, then is has to be extended so that MPSAFE drivers can register themselves as MPSAFE, so that non-MPSAFE drivers are still called with KERNEL_LOCK. it seems the wrong answer to me. instead of fixing the one or two drivers you've found problems in, you want to add more infrastructure (IMO clutter) to the system and patch all the drivers that *are* ok. considering that this problem is very specific to scsipi, i don't think that is a viable solution. FreeBSD started out with MPSAFE and then went to NEEDS_GIANT as flags for the drivers. I'm with Matthew: patch the drivers that are broken (or not known to be safe) and then you have a convenient thing to grep for when you want to expand the drivers that are safe. Much better that way, and it turned out to be a big win in FreeBSD when we went from opt-in MPSAFE to out-out... Warner
Re: CVS commit: src/sys/dev/scsipi
On Mon, Apr 25, 2011 at 02:14:23PM +, Juergen Hannken-Illjes wrote: Module Name: src Committed By: hannken Date: Mon Apr 25 14:14:22 UTC 2011 Modified Files: src/sys/dev/scsipi: scsiconf.c Log Message: Don't kill outstanding requests when detaching a scsibus on shutdown. Both the controller and tyhe targets are still running. I don't think you have fixed the actual bug. It sounds like the detachment routine is broken in every case where the controller was not abruptly detached, but the driver relinquishes control of the controller in an orderly fashion because of an operator command. Dave -- David Young OJC Technologies dyo...@ojctech.com Urbana, IL * (217) 344-0444 x24
Re: CVS commit: src/sys/dev/scsipi
On Mon, Apr 25, 2011 at 10:33:14AM -0500, David Young wrote: On Mon, Apr 25, 2011 at 02:14:23PM +, Juergen Hannken-Illjes wrote: Module Name:src Committed By: hannken Date: Mon Apr 25 14:14:22 UTC 2011 Modified Files: src/sys/dev/scsipi: scsiconf.c Log Message: Don't kill outstanding requests when detaching a scsibus on shutdown. Both the controller and tyhe targets are still running. I don't think you have fixed the actual bug. It sounds like the detachment routine is broken in every case where the controller was not abruptly detached, but the driver relinquishes control of the controller in an orderly fashion because of an operator command. The actual bug was i386 shutdown, a loop of vfs_unmountall1, config_detach_all and vfs_unmount_forceone. Here config_detach_all() detaches devices from the leafs up. For me it sometimes happend that the (scsi) root disk had outstanding xfers when it came to config_detach_all(). The disk would not detach but the bus detach would kill all outstanding operations. I don't want these xfers to abort but the disk continue operations until all xfers are done. And yes, this detach routine looks bogus at least ... -- Juergen Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/dev/scsipi
On Mon, Apr 25, 2011 at 06:26:09PM +0200, Juergen Hannken-Illjes wrote: On Mon, Apr 25, 2011 at 10:33:14AM -0500, David Young wrote: On Mon, Apr 25, 2011 at 02:14:23PM +, Juergen Hannken-Illjes wrote: Module Name: src Committed By: hannken Date: Mon Apr 25 14:14:22 UTC 2011 Modified Files: src/sys/dev/scsipi: scsiconf.c Log Message: Don't kill outstanding requests when detaching a scsibus on shutdown. Both the controller and tyhe targets are still running. I don't think you have fixed the actual bug. It sounds like the detachment routine is broken in every case where the controller was not abruptly detached, but the driver relinquishes control of the controller in an orderly fashion because of an operator command. The actual bug was i386 shutdown, a loop of vfs_unmountall1, config_detach_all and vfs_unmount_forceone. Here config_detach_all() detaches devices from the leafs up. For me it sometimes happend that the (scsi) root disk had outstanding xfers when it came to config_detach_all(). The disk would not detach but the bus detach would kill all outstanding operations. I don't want these xfers to abort but the disk continue operations until all xfers are done. And yes, this detach routine looks bogus at least ... The bug was in scsibusdetach(), which is not doing things in the proper order: it has to detach its children and check for error. If no error, then it can release the resources that the children were using. See attached patch. Dave -- David Young OJC Technologies dyo...@ojctech.com Urbana, IL * (217) 344-0444 x24 Index: scsiconf.c === RCS file: /cvsroot/src/sys/dev/scsipi/scsiconf.c,v retrieving revision 1.261 diff -u -p -r1.261 scsiconf.c --- scsiconf.c 25 Apr 2011 14:14:22 - 1.261 +++ scsiconf.c 25 Apr 2011 17:30:31 - @@ -256,11 +256,20 @@ scsibusdetach(device_t self, int flags) struct scsipi_xfer *xs; int error; + /* +* Detach all of the periphs. +*/ + if ((error = scsipi_target_detach(chan, -1, -1, flags)) != 0) + return error; + pmf_device_deregister(self); /* * Process outstanding commands (which will never complete as the * controller is gone). +* +* XXX Surely this is redundant? If we get this far, the +* XXX peripherals have all been detached. */ for (ctarget = 0; ctarget chan-chan_ntargets; ctarget++) { if (ctarget == chan-chan_id) @@ -269,8 +278,6 @@ scsibusdetach(device_t self, int flags) periph = scsipi_lookup_periph(chan, ctarget, clun); if (periph == NULL) continue; - if ((flags DETACH_SHUTDOWN) != 0) - return EBUSY; TAILQ_FOREACH(xs, periph-periph_xferq, device_q) { callout_stop(xs-xs_callout); xs-error = XS_DRIVER_STUFFUP; @@ -280,16 +287,10 @@ scsibusdetach(device_t self, int flags) } /* -* Detach all of the periphs. -*/ - error = scsipi_target_detach(chan, -1, -1, flags); - - /* * Now shut down the channel. -* XXX only if no errors ? */ scsipi_channel_shutdown(chan); - return (error); + return 0; } /*
Re: CVS commit: src/sys/dev/scsipi
On May 13, 7:56am, christoph_eg...@gmx.de (Christoph Egger) wrote: -- Subject: Re: CVS commit: src/sys/dev/scsipi | Christos Zoulas wrote: | Module Name:src | Committed By: christos | Date: Wed May 13 02:35:25 UTC 2009 | | Modified Files: | src/sys/dev/scsipi: scsipiconf.h | | Log Message: | sprinkle #ifdef _KERNEL to make scsictl compile. | | | Ah, the device_t change exposed something that shouldn't | be visible in userspace, it seems. | | Thanks for fixing it and sorry for build breakage. | I should have also compile-tested userland of one arch at least, | not just many kernels. Not a problem. I think that the parts of this file that are needed from scsictl don't really belong there, since none of the structures seem to be useable from userland. christos