Martin, On 2/16/17 12:36, Martin K. Petersen wrote: > Damien, > > Damien> So the conclusion may be that we need to drop everything ? The > Damien> mpt3sas patch breaks ZBC now, so that must be removed too. > > Nah. > > But it's important that we restrict the rounding to TYPE_FS requests > (i.e. I/Os issued by the kernel where we control what's going on). Your > patch already does this by virtue of changing sd_done(). So for that > reason I'm OK with relocating the rounding from mpt3sas to sd.c. > > However, I still think it's a fundamental problem that mpt3sas reports > completion in terms of "SAS frame bytes (successfully?) transferred" > instead of "blocks acknowledged by the target". The drive can obviously > not write a partial sector. So that approach to completion reporting > seems completely bogus to me. > > Anyway. My main comment is that it seems like the rounding logic would > be better contained in sd_completed_bytes() since that is already doing > sanity checking on what's reported. Indeed, that would be nicer. But sd_completed_bytes is only called when sense data (a sense key) was also returned with the command. It is not clear to me if that is always the case when an unaligned resid is reported by the bogus mpt3sas HBA. Handling all possible (bogus) cases may require a more extensive rewrite of sd_completed_bytes, which may not be the best of choice this late in the RC cycle. Should I keep the fix in its current place and defer the move to sd_completed_bytes to a later patch for 4.11 ? > Also, I am no fan of perpetuating the arcane SCSI logging stuff now that > we have tracing. So just make it a regular sd_printk(). Sure, will do. Thank you. -- Damien Le Moal, Ph.D. Sr. Manager, System Software Research Group, Western Digital Corporation Damien.LeMoal@xxxxxxx (+81) 0466-98-3593 (ext. 513593) 1 kirihara-cho, Fujisawa, Kanagawa, 252-0888 Japan www.wdc.com, www.hgst.com