On Fri, 2019-01-04 at 13:34 -0500, Douglas Gilbert wrote: > On 2019-01-04 11:52 a.m., James Bottomley wrote: > > On Fri, 2019-01-04 at 11:18 -0500, Douglas Gilbert wrote: > > > Add a top level "scsi" directory in debugfs (usually at > > > /sys/kernel/debugfs/scsi) with two subdirectories: "uld" and > > > "lld". The idea is to place mid-level related 'knobs' in the > > > "scsi" directory, and for the ULDs to make subsirectories like > > > "scsi/uld/sd" and "scsi/uld/st" as required. LLDs could follow a > > > similar pattern. > > > > I'm really not sure this is useful, but just in case it is, I > > suppose the first question should be why this isn't all simply > > conditioned on BLK_DEBUG_FS? Is there a solid use case where > > someone would use this in a !BLK_DEBUG_FS situation. > > Logically this extension should only depend on CONFIG_DEBUG_FS as > that is the minimum requirement. Yes, but it's not a reason .. you can use that to justify all sorts of micro config options. However to be useful, config options must represent kernels people would actually build, so the pressure is on to reduce the number. > Also it is not blindingly obviously why someone debugging the sg, st > and ses driver (all char devices) needs to select > CONFIG_BLK_DEBUG_FS. Why not have CONFIG_SCSI_DEBUG_FS ? The reason I could see them being tied together is that if you're after debug information for a SCSI command, part of it might be what it went into block as ... i.e. the current request flags which are SCSI specific and enabled on BLK_DEBUG_FS. What I can't see is a reason to build a special kernel that has SCSI debug stuff but not block. > The patch evolved this way due to the kbuild bot splitting the > BLK_DEBUG_FS and DEBUG_FS cases, causing a compile error. Right, but that can be fixed by unifying them. > In review comments on my sg v4 driver rewrite, reviewers comment that > I should not be using the existing SCSI_LOGGING infrastructure since > it is deprecated and "maintain-only". [IMO it works fine]. However > there seems no or very little support within the SCSI subsystem for > any other approach. So this patch is an attempt to make some progress > on that front. That's kind of orthogonal. One of the best ways of getting debugging information out of the kernel is tracepoints and if we're doing that we should probably do it in a way that it plugs nicely into blktrace, so if that's the genesis, we should probably have the discussion first about how we want to best extract debug information about command flow. > When I'm debugging (and I do that a lot lately), I assume that the > calls to the mid-level, block and other layers do what they are > supposed to do. So I'm not interested in their debug or a breakdown > of their objects. So, for example, I have never used scsi_debugfs and > I expect that if I moved /proc/scsi/sg/debug to > /sys/kernel/debug/scsi/sg/debug then I still would not want or need > however scsi_show_rq() is used. but would you actually build a kernel specially with the sole object of not allowing the information to be seen? That's the point: this is a Kconfig option there has to be a good reason to build every possible kernel or else it's not a good option to have. > Does debugging the hisi_sas driver need to access scsi_show_rq() ? It's not about need to, it's about need not. Is there a specific reason not to build with it ... likelihood of looking at it isn't really the best measure. The kernel emits tons of stuff I don't particularly care about ... it even annoys me when it gets in the way of stuff I do care about but it doesn't annoy me enough to want to build a special kernel without it. James
