On 09/01/2014 10:57 AM, Hannes Reinecke wrote: > On 09/01/2014 12:19 AM, Christoph Hellwig wrote: >> On Thu, Aug 28, 2014 at 07:33:27PM +0200, Hannes Reinecke wrote: >>> SCSI opcode printing is tricky and needs to take into account >>> several different corner cases. So instead of trying to come >>> up with an elaborate printk() statement we should be printing >>> it into a local buffer. >> >> scsi_print_command callers are usually deep down the call chain., so I'd >> rather avoid using up even more stack here. What are the exact reasons >> that we need the local buffer? >> > I know, and I'm not happy with this patch, either. > > However, we really, _really_, want to print the decode command name > and the CDB in one line ie with one printk() statement. > If we don't we'll end up with individual logging messages with one > byte per line under high load. > Making it impossible to figure out to which CDB these individual > bytes are related to. > > It would be possible to unroll the CDB decoding loop, as we're > typically have only 3 formats. But it's near impossible to find some > common ground when decoding the command; I've ended up having > really convoluted #defines calling into each other, making debugging > that code really horrible. > Actually, I'm not sure we _can_ avoid increase stack usage when printing the CDB. At the end of the day, we want to print a CDB with one printk() statement. So we'll be having at least 9 arguments to printk per CDB (format, device name, opcode name, and CDB bytes). And as we only have a limited register set available we'll be ending up putting most things on the stack _anyway_, especially for larger CDBs. Hiding that behind an elaborate set of preprocessor definitions and va_format thingies just serves to obfuscate the matter. Hence I'll be voting to make this explicit and use a local buffer and only two or three function arguments. We can optimize it by calling printk() directly for smaller CDB lengths, but with 16-byte CDBs we'll be using probably the stack size as with a local buffer. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage hare@xxxxxxx +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
