Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Jeff Garzik wrote: > Tejun Heo wrote: >> Jeff Garzik wrote: >>> A few days before that, both PMP and SAS /were/ slated for 2.6.24, and >>> after I fix the design problems, they will be again. >>> >>> One way or another, upstream will /not/ be doing polling PMP in 2.6.24. >> >> Just an update to let you know that I've been working on it. sata_sil24 >> works okay but ahci still craps itself after resetting downstream ports. This turns out to be a different issue. SIMG 5723/5744 doesn't like being driven by ahci controllers (both ICH9 and JMB) and the problem isn't related to whether polling is used or not. It's very weird because SATA tracer doesn't show much difference on the host side wire between being driven by ahci and sil24/32, but on the fan-out side, things are seriously broken (repeated COMRESET/COMWAKE/COMINIT and no FIS successfully being relayed from the host side). Other than that, I've tested things on various combinations and am fairly confident with it. I think we can go for 2.6.24 merge. Even things turn out to be bad, we at least have pretty good bisection point. > Thanks for your patience and perseverance. Thanks. :-) > It looks like it would be too difficult to get SAS PMP working for > 2.6.24 merge window open, so I think it is only fair to rescind my > assertion of "polling PMP not in 2.6.24 release." > > Removing the polling remains a design requirement for SAS, but the more > I look at old-EH-encrusted libsas, the more work I feel it needs before > its ready for PMP. Sorry. I wish libata EH was easier to deal with from SAS side. I think being able to present SAS end point as an independent ATA host to libata EH would probably make life easier for both sides, but I don't have any actual experience with SAS (yet). Thanks. -- tejun - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Tejun Heo wrote: Jeff Garzik wrote: A few days before that, both PMP and SAS /were/ slated for 2.6.24, and after I fix the design problems, they will be again. One way or another, upstream will /not/ be doing polling PMP in 2.6.24. Just an update to let you know that I've been working on it. sata_sil24 works okay but ahci still craps itself after resetting downstream ports. Thanks for your patience and perseverance. It looks like it would be too difficult to get SAS PMP working for 2.6.24 merge window open, so I think it is only fair to rescind my assertion of "polling PMP not in 2.6.24 release." Removing the polling remains a design requirement for SAS, but the more I look at old-EH-encrusted libsas, the more work I feel it needs before its ready for PMP. Sorry. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Jeff Garzik wrote: > A few days before that, both PMP and SAS /were/ slated for 2.6.24, and > after I fix the design problems, they will be again. > > One way or another, upstream will /not/ be doing polling PMP in 2.6.24. Just an update to let you know that I've been working on it. sata_sil24 works okay but ahci still craps itself after resetting downstream ports. Thanks. -- tejun - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Alan Cox wrote: Polling PMP 2.6.24 is completely unacceptable. It screws the 2.6.24 SAS driver releases out of PMP. No PMP in 2.6.24 screws PMP users, who outnumber SAS users by a few thousand to one I suspect. no-PMP-in-2.6.24 is not happening, so that's rather irrelevant. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
> Polling PMP 2.6.24 is completely unacceptable. It screws the 2.6.24 SAS > driver releases out of PMP. No PMP in 2.6.24 screws PMP users, who outnumber SAS users by a few thousand to one I suspect. - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Mark Lord wrote: Jeff Garzik wrote: Tejun Heo wrote: Jeff Garzik wrote: Mark Lord wrote: Linux kernel development is supposed to happen incrementally nowadays. Get a nice working solution in place, and then enhance/tune it. It's not about enhancing and tuning. It's about me (and/or James B) having to __undo__ the current code, just to get things working on an entire class of SATA-capable controllers out in the field. Hmmm... Simpy not setting ATA_FLAG_PMP isn't enough? The point was that I was going to turn on PMP support for SAS controllers in 2.6.24 to coincide with the merge, but now cannot without fixing things. So, don't. Just a few days ago PMP was slated to not be enabled for *any* controllers until 2.6.25 or so. Now we have it a release early, for all but SAS. A few days before that, both PMP and SAS /were/ slated for 2.6.24, and after I fix the design problems, they will be again. One way or another, upstream will /not/ be doing polling PMP in 2.6.24. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Jeff Garzik wrote: Tejun Heo wrote: Jeff Garzik wrote: Mark Lord wrote: Linux kernel development is supposed to happen incrementally nowadays. Get a nice working solution in place, and then enhance/tune it. It's not about enhancing and tuning. It's about me (and/or James B) having to __undo__ the current code, just to get things working on an entire class of SATA-capable controllers out in the field. Hmmm... Simpy not setting ATA_FLAG_PMP isn't enough? The point was that I was going to turn on PMP support for SAS controllers in 2.6.24 to coincide with the merge, but now cannot without fixing things. So, don't. Just a few days ago PMP was slated to not be enabled for *any* controllers until 2.6.25 or so. Now we have it a release early, for all but SAS. Cheers - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Tejun Heo wrote: Jeff Garzik wrote: Mark Lord wrote: Linux kernel development is supposed to happen incrementally nowadays. Get a nice working solution in place, and then enhance/tune it. It's not about enhancing and tuning. It's about me (and/or James B) having to __undo__ the current code, just to get things working on an entire class of SATA-capable controllers out in the field. Hmmm... Simpy not setting ATA_FLAG_PMP isn't enough? The point was that I was going to turn on PMP support for SAS controllers in 2.6.24 to coincide with the merge, but now cannot without fixing things. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Jeff Garzik wrote: > Mark Lord wrote: >> Linux kernel development is supposed to happen incrementally nowadays. >> Get a nice working solution in place, and then enhance/tune it. > > It's not about enhancing and tuning. > > It's about me (and/or James B) having to __undo__ the current code, just > to get things working on an entire class of SATA-capable controllers out > in the field. Hmmm... Simpy not setting ATA_FLAG_PMP isn't enough? -- tejun - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Jeff Garzik wrote: > Polling PMP 2.6.24 is completely unacceptable. It screws the 2.6.24 SAS > driver releases out of PMP. Not merging PMP has about the same effect for SAS, right? > I pulled your last PMP patchset, and will now endeavor to fix the API > prior to 2.6.24 merge window opening. I have a preliminary patch. The patch is pretty simple too. I'll submit after some testing but I'm not sure whether doing this right before the merge window is a good idea. I'd prefer not to support PMP on SAS for 2.6.24. > Linux high level message-submit / message-complete APIs should never > _require_ polling, even if its 100% polling under the hood. There are > far too many cases in the field where you don't have direct access to > hardware registers to poll. Or such polling would interfere with the > operation of other ports. Or any of a myriad of other reasons. I don't think that's true for actual ATA controllers but, for SAS, probably true. Is the frozen reset mechanism a headache too for SAS? Thanks. -- tejun - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Tejun Heo wrote: Jeff Garzik wrote: Polling ALREADY makes the job of fixing SAS/SATA exception handling difficult. Expanding polling to something SAS/SATA controllers treat as fundamentally irq-driven and integrated with the rest of the command flow is moving in the wrong direction. To re-re-re-summarize, polling in PMP is fundamentally broken for an ENTIRE CLASS OF HARDWARE that we actively support today. And jgarzik/misc-2.6.git#sas is adding two more controllers to that list. As an interim solution, it doesn't make anything worse tho. Those drivers don't support PMP anyway. After rc1 merge, polling PMP access can be replaced with new qc_issue (probably ata_exec_internal) based code. The question here is whether it's worth to include PMP support with polling PMP register access as an interim solution for 2.6.24. I think it will be beneficial for both user convenience and testing as long as the said change is made soon after -rc1. Polling PMP 2.6.24 is completely unacceptable. It screws the 2.6.24 SAS driver releases out of PMP. I pulled your last PMP patchset, and will now endeavor to fix the API prior to 2.6.24 merge window opening. Linux high level message-submit / message-complete APIs should never _require_ polling, even if its 100% polling under the hood. There are far too many cases in the field where you don't have direct access to hardware registers to poll. Or such polling would interfere with the operation of other ports. Or any of a myriad of other reasons. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Mark Lord wrote: Linux kernel development is supposed to happen incrementally nowadays. Get a nice working solution in place, and then enhance/tune it. It's not about enhancing and tuning. It's about me (and/or James B) having to __undo__ the current code, just to get things working on an entire class of SATA-capable controllers out in the field. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Alan Cox wrote: ty today, with 3+ month kernel release cycles (ugh!!). I'm very much interested in hearing suggestions and comments. I think PMP should go in for 2.6.24 and then get revised over time to not poll and to go via qc_issue. Which seems to be the consensus of everyone but you on this one. It's not an "over time" issue. SAS drivers that will be ready for 2.6.24 (broadsas, mvsas) have both been coded to support PMP transparently -- but the current libata PMP code requires that PMP support in SAS be TURNED OFF, because it is fundamentally incompatible by its design. This is an issue that has been present and known for many, many months -- as long as drivers/scsi/libsas/sas_ata.c has been in the tree. The more we walk down the polling path, the more incompatible we become with SATA-capable controllers that are in users' hands today. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
ty today, with 3+ month kernel release cycles (ugh!!). > > I'm very much interested in hearing suggestions and comments. I think PMP should go in for 2.6.24 and then get revised over time to not poll and to go via qc_issue. Which seems to be the consensus of everyone but you on this one. In general I have no big problems with the way its going - need to get stuff through -mm into libata faster but thats something I need to sort out with you specifically and I can accept your view its more my problem. But PMP is a definite candidate for in ASAP Alan - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
On 9/28/07, Jeff Garzik <[EMAIL PROTECTED]> wrote: > (my last response only addressed -mm) > > Mark Lord wrote: > > I believe the point was that getting things into libata is glacial > > IMHO would say that there are two causes of that: > 1) I am sometimes slow in merging, part of which is my own fault, and I > can only promise to try and do better there. Part of which is a result > of stuff being dependent on -mm (which requires plenty of time > hand-merging) rather than libata-dev.git#upstream. > > 2) I have been intentionally staging major libata behavior changes > between Linux releases. Luckily most of these are behind us, but, in > several cases with things like ACPI on/off (hopefully 'on' in 2.6.24), > probing changes (switchover to new EH for probing via hotplug, etc.), > interrupt handling changes. > > I dislike getting "too much" into a single release, because of the > difficulty of getting large scale feedback without a major kernel release. > > So far I think the kernel releases have been pretty darn successful in > "not breaking everybody" but that clearly conflicts with desired > development speed, given the glacial pace of each kernel release. If > each kernel release were 1-2 months apart, we would have many more > testing points, and I think you guys and I would both be happy. But > that's not the reality today, with 3+ month kernel release cycles (ugh!!). > > I'm very much interested in hearing suggestions and comments. > > Jeff Perhaps you could more fully leverage the various distro alpha/beta/rcs/release kernels. Specific to the PMP patches, they went into the openSUSE beta/rc kernels (2.6.23 based) in early Aug. I think. So they have already been through a significant set of Novell internal and community testing. Likely similar testing to what it would get in -mm. I believe they are part of the OpenSUSE 10.3 (2.6.23 based) full release that is set for Thursday (Oct. 4) and thus will see a huge amount of public testing. Greg -- Greg Freemyer Litigation Triage Solutions Specialist http://www.linkedin.com/in/gregfreemyer The Norcross Group The Intersection of Evidence & Technology http://www.norcrossgroup.com - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Jeff Garzik wrote: I certainly deserve plenty of blame for not catching this fact earlier, much to my chagrin. But there are real technical issues at hand: Polling ALREADY makes the job of fixing SAS/SATA exception handling difficult. Expanding polling to something SAS/SATA controllers treat as fundamentally irq-driven and integrated with the rest of the command flow is moving in the wrong direction. Fine. But the exising PMP patchset has already taken close to a year to become mature and safe enough to pass "Jeff review". Tons of us are awaiting it in mainline, and to rework it to meet your latest concerns will delay it until 2.6.25 at least --> mid 2008. Not Acceptable. It doesn't *break* anything (we know of). It doesn't affect existing SAS functionality. It *does* add working, critical functionality to libata. And.. Tejun isn't just going to lay down and be happy with it in 2.6.24. He's fully committed to the rework you're demanding, but in a more timely and correct fashion. Linux kernel development is supposed to happen incrementally nowadays. Get a nice working solution in place, and then enhance/tune it. The current implementation is certainly kernel-worthy, and will be reworked over time as is the rest of libata. The barrier to entry for new libata functionality is way too high. Yes, we want readable, *reliable* code, because screwing people's filesystems is not an option. Fine. Tejun's current code is more than "good enough", and "perfect" code doesn't exist. But we're all striving for it anyway. Cheers - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Jeff Garzik wrote: > Polling ALREADY makes the job of fixing SAS/SATA exception handling > difficult. Expanding polling to something SAS/SATA controllers treat as > fundamentally irq-driven and integrated with the rest of the command > flow is moving in the wrong direction. > > To re-re-re-summarize, polling in PMP is fundamentally broken for an > ENTIRE CLASS OF HARDWARE that we actively support today. And > jgarzik/misc-2.6.git#sas is adding two more controllers to that list. As an interim solution, it doesn't make anything worse tho. Those drivers don't support PMP anyway. After rc1 merge, polling PMP access can be replaced with new qc_issue (probably ata_exec_internal) based code. The question here is whether it's worth to include PMP support with polling PMP register access as an interim solution for 2.6.24. I think it will be beneficial for both user convenience and testing as long as the said change is made soon after -rc1. My vote is yes but this kind of decision ultimately falls on the subsystem maintainer, so it's your call, Jeff. Thanks. -- tejun - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Andrew Morton wrote: On Fri, 28 Sep 2007 23:29:44 -0400 Jeff Garzik <[EMAIL PROTECTED]> wrote: (my last response only addressed -mm) Mark Lord wrote: I believe the point was that getting things into libata is glacial IMHO would say that there are two causes of that: 1) I am sometimes slow in merging, part of which is my own fault, and I can only promise to try and do better there. Part of which is a result of stuff being dependent on -mm (which requires plenty of time hand-merging) rather than libata-dev.git#upstream. 2) I have been intentionally staging major libata behavior changes between Linux releases. Luckily most of these are behind us, but, in several cases with things like ACPI on/off (hopefully 'on' in 2.6.24), probing changes (switchover to new EH for probing via hotplug, etc.), interrupt handling changes. I dislike getting "too much" into a single release, because of the difficulty of getting large scale feedback without a major kernel release. So far I think the kernel releases have been pretty darn successful in "not breaking everybody" but that clearly conflicts with desired development speed, given the glacial pace of each kernel release. If each kernel release were 1-2 months apart, we would have many more testing points, and I think you guys and I would both be happy. But that's not the reality today, with 3+ month kernel release cycles (ugh!!). I'm very much interested in hearing suggestions and comments. There's an easy fix... The releases are slow because a) there's so much stuff in them and b) it takes so long to stabilise it all. If all developers were to be more careful in their work, and take more time to review and test others' work, both problems get fixed: less code, higher quality. We don't know how to make this happen. We haven't even tried. I think you missed part of my point, with regards to libata: Even though stuff gets tested in -mm and in mainline -rc releases, we simply do not have the test audience that a major kernel release does, when it comes to determining whether new libata probe behavior will break millions of boxes (or not). Unlike CPUs and other hardware, our attached devices (disks, cd-roms) are entirely black boxes of [mis]behavior, with plenty of libata behavior based _entirely_ on observations in the field, not stuff documented in a specification somewhere. 3-4 true "trial and error" periods per year (kernel releases) give libata a similar number of test points per years. Not a whole lot of room when you consider that we must actively (but carefully!) experimental with new kernel features in the each kernel release. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
On Fri, 28 Sep 2007 23:29:44 -0400 Jeff Garzik <[EMAIL PROTECTED]> wrote: > (my last response only addressed -mm) > > Mark Lord wrote: > > I believe the point was that getting things into libata is glacial > > IMHO would say that there are two causes of that: > 1) I am sometimes slow in merging, part of which is my own fault, and I > can only promise to try and do better there. Part of which is a result > of stuff being dependent on -mm (which requires plenty of time > hand-merging) rather than libata-dev.git#upstream. > > 2) I have been intentionally staging major libata behavior changes > between Linux releases. Luckily most of these are behind us, but, in > several cases with things like ACPI on/off (hopefully 'on' in 2.6.24), > probing changes (switchover to new EH for probing via hotplug, etc.), > interrupt handling changes. > > I dislike getting "too much" into a single release, because of the > difficulty of getting large scale feedback without a major kernel release. > > So far I think the kernel releases have been pretty darn successful in > "not breaking everybody" but that clearly conflicts with desired > development speed, given the glacial pace of each kernel release. If > each kernel release were 1-2 months apart, we would have many more > testing points, and I think you guys and I would both be happy. But > that's not the reality today, with 3+ month kernel release cycles (ugh!!). > > I'm very much interested in hearing suggestions and comments. > There's an easy fix... The releases are slow because a) there's so much stuff in them and b) it takes so long to stabilise it all. If all developers were to be more careful in their work, and take more time to review and test others' work, both problems get fixed: less code, higher quality. We don't know how to make this happen. We haven't even tried. - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
(my last response only addressed -mm) Mark Lord wrote: I believe the point was that getting things into libata is glacial IMHO would say that there are two causes of that: 1) I am sometimes slow in merging, part of which is my own fault, and I can only promise to try and do better there. Part of which is a result of stuff being dependent on -mm (which requires plenty of time hand-merging) rather than libata-dev.git#upstream. 2) I have been intentionally staging major libata behavior changes between Linux releases. Luckily most of these are behind us, but, in several cases with things like ACPI on/off (hopefully 'on' in 2.6.24), probing changes (switchover to new EH for probing via hotplug, etc.), interrupt handling changes. I dislike getting "too much" into a single release, because of the difficulty of getting large scale feedback without a major kernel release. So far I think the kernel releases have been pretty darn successful in "not breaking everybody" but that clearly conflicts with desired development speed, given the glacial pace of each kernel release. If each kernel release were 1-2 months apart, we would have many more testing points, and I think you guys and I would both be happy. But that's not the reality today, with 3+ month kernel release cycles (ugh!!). I'm very much interested in hearing suggestions and comments. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Mark Lord wrote: Jeff Garzik wrote: Alan Cox wrote: This doesn't affect Tejun or anyone else really, but going through -mm tends to gum up the works. Your patches wind up not applying to libata#upstream, in which case they get dropped and I assume Andrew or someone will resend usable versions. Its the only way to get testing done and get them into distributions in time. Not true... libata gets auto-propagated into -mm, and Andrew pulls regularly. I believe the point was that getting things into libata is glacial compared with getting them into -mm. Witness the PMP patchset. I don't think PMP was ever in -mm? But regardless, people can send me "for test only" or WIP stuff that I can auto-propagate to -mm via libata-dev.git. I keep many libata-dev.git branches alive with testing and WIP code, and it doesn't consume much time at all. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Alan Cox wrote: Jeff, seeing as Tejun's commitment is never in doubt here, I really believe we should go with the existing PMP patchset for 2.6.24 (unless the respin happens quickly enough). This functionality is way overdue, and we shouldn't be impeding it as long as we have been. I would second this. Its far too important to not get this stuff upstream and usable NOW. Yes the model will have to change a bit but the entire libata today has almost no resemblence to the one a year ago. It can evolve and then the old one can die off just as we did with EH (except for libsas anyway) I certainly deserve plenty of blame for not catching this fact earlier, much to my chagrin. But there are real technical issues at hand: Polling ALREADY makes the job of fixing SAS/SATA exception handling difficult. Expanding polling to something SAS/SATA controllers treat as fundamentally irq-driven and integrated with the rest of the command flow is moving in the wrong direction. To re-re-re-summarize, polling in PMP is fundamentally broken for an ENTIRE CLASS OF HARDWARE that we actively support today. And jgarzik/misc-2.6.git#sas is adding two more controllers to that list. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
> Jeff, seeing as Tejun's commitment is never in doubt here, > I really believe we should go with the existing PMP patchset > for 2.6.24 (unless the respin happens quickly enough). > > This functionality is way overdue, and we shouldn't be impeding it > as long as we have been. I would second this. Its far too important to not get this stuff upstream and usable NOW. Yes the model will have to change a bit but the entire libata today has almost no resemblence to the one a year ago. It can evolve and then the old one can die off just as we did with EH (except for libsas anyway) Alan - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
On Friday 28 September 2007, Mark Lord wrote: > Tejun Heo wrote: > > Jeff Garzik wrote: > >>> Aieee... Another merge delay. I wish the review process proceeded a bit > >>> swifter. The patchset has been around literally for years now and > >>> submitted for review six times if I have the take number right. :-( > >> Well the vast majority of the patches are in, what five out of six > >> original patchsets? > > > > Yeah, I'm frustrated mainly because I've been telling people that > > mainline will probably have PMP support when 2.6.24 comes out and it > > seems we'll miss the merge window again. Oh, well... > > > >> Sorry I didn't catch the polling requirement beforehand, it was not > >> really clear from a quick read. > > > > ->pmp_read/write stuff is something which I've been meaning to change > > anyway. When developing the PMP code, PMP register access while frozen > > seemed necessary but now I think we can be just as safe without it. I > > was thinking about changing it after merge because the current code > > received a lot of testing and I didn't want to destabilize it right > > before merging. This is an excellent point for merging the PMP code as it is currently and doing revamp later. PMP patchset in the current form has got quite a lot of testing in -mm and "last minute" changes have a tendency to bring up some nasty surprises. > > I'll be back home mid next week. I'll try to re-test and re-submit the > > changes ASAP. > > Jeff, seeing as Tejun's commitment is never in doubt here, > I really believe we should go with the existing PMP patchset > for 2.6.24 (unless the respin happens quickly enough). > > This functionality is way overdue, and we shouldn't be impeding it > as long as we have been. It is way, way overdue... > Tejun will definitely continue to rework the changes you've asked for > in time for the next release, but let's not hold things up unreasonably here. Seconded. Bart - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Tejun Heo wrote: Jeff Garzik wrote: Aieee... Another merge delay. I wish the review process proceeded a bit swifter. The patchset has been around literally for years now and submitted for review six times if I have the take number right. :-( Well the vast majority of the patches are in, what five out of six original patchsets? Yeah, I'm frustrated mainly because I've been telling people that mainline will probably have PMP support when 2.6.24 comes out and it seems we'll miss the merge window again. Oh, well... Sorry I didn't catch the polling requirement beforehand, it was not really clear from a quick read. ->pmp_read/write stuff is something which I've been meaning to change anyway. When developing the PMP code, PMP register access while frozen seemed necessary but now I think we can be just as safe without it. I was thinking about changing it after merge because the current code received a lot of testing and I didn't want to destabilize it right before merging. I'll be back home mid next week. I'll try to re-test and re-submit the changes ASAP. Jeff, seeing as Tejun's commitment is never in doubt here, I really believe we should go with the existing PMP patchset for 2.6.24 (unless the respin happens quickly enough). This functionality is way overdue, and we shouldn't be impeding it as long as we have been. Tejun will definitely continue to rework the changes you've asked for in time for the next release, but let's not hold things up unreasonably here. Cheers - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Jeff Garzik wrote: Alan Cox wrote: This doesn't affect Tejun or anyone else really, but going through -mm tends to gum up the works. Your patches wind up not applying to libata#upstream, in which case they get dropped and I assume Andrew or someone will resend usable versions. Its the only way to get testing done and get them into distributions in time. Not true... libata gets auto-propagated into -mm, and Andrew pulls regularly. I believe the point was that getting things into libata is glacial compared with getting them into -mm. Witness the PMP patchset. Cheers - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Jeff Garzik wrote: >> Aieee... Another merge delay. I wish the review process proceeded a bit >> swifter. The patchset has been around literally for years now and >> submitted for review six times if I have the take number right. :-( > > Well the vast majority of the patches are in, what five out of six > original patchsets? Yeah, I'm frustrated mainly because I've been telling people that mainline will probably have PMP support when 2.6.24 comes out and it seems we'll miss the merge window again. Oh, well... > Sorry I didn't catch the polling requirement beforehand, it was not > really clear from a quick read. ->pmp_read/write stuff is something which I've been meaning to change anyway. When developing the PMP code, PMP register access while frozen seemed necessary but now I think we can be just as safe without it. I was thinking about changing it after merge because the current code received a lot of testing and I didn't want to destabilize it right before merging. I'll be back home mid next week. I'll try to re-test and re-submit the changes ASAP. Thanks. -- tejun - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
> So, there just hasn't been a real need so far. If interrupt overhead > becomes an issue, my first step would be to start programming the > interrupt coalescing hardware that comes in modern SATA controllers. Another factor is queue sizes. You don't get 1024 entry queues on your typical disk controller and that makes NAPI type stuff less viable, and sensible IRQ colaescing on the card more sane. Alan PS: Jeff - if you want stuff from -mm and it's got cross dependancies, clashes just ask me for "patch xyz versus libata head" and I'll sort it out - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Alan Cox wrote: This doesn't affect Tejun or anyone else really, but going through -mm tends to gum up the works. Your patches wind up not applying to libata#upstream, in which case they get dropped and I assume Andrew or someone will resend usable versions. Its the only way to get testing done and get them into distributions in time. Not true... libata gets auto-propagated into -mm, and Andrew pulls regularly. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
> This doesn't affect Tejun or anyone else really, but going through -mm > tends to gum up the works. Your patches wind up not applying to > libata#upstream, in which case they get dropped and I assume Andrew or > someone will resend usable versions. Its the only way to get testing done and get them into distributions in time. Alan - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Mark Lord wrote: Jeff Garzik wrote: .. As such, polling is simply an outmoded concept. It does not make sense on new hardware, and forcing design decisions down that path only lead to a cascade of similar design decisions -- pmp_read polling being just one example of such a result. A related tangent here, but I wonder if someday we might consider something like NAPI for libata. Our I/O rates can far exceed those of the network stack, and there polling (NAPI) has helped quite a bit. I certainly think about these issues each time I write a new storage driver, given that most new hardware has interrupt mitigation/coalescing capability built into it -- in AHCI's case, at my urging. Storage is a different profile, with quite a bit few interrupts. Storage interrupt rates are way, way below that of networking. Ethernet is basically forced to deal with an interrupt event every 1K. By design, storage gathers up as much I/O as possible, such that each interrupt could be completing 128K, 512K, 1MB or more. Finally, for storage I/O workloads with lots of smaller I/Os, those workloads are often seek-heavy, where any interrupt overhead is few and far between, while you're waiting on your slow disk to seek. You /don't/ want to be polling for long milliseconds, waiting for a seek. That's a good way to increase your CPU usage, rather than decrease it. So, there just hasn't been a real need so far. If interrupt overhead becomes an issue, my first step would be to start programming the interrupt coalescing hardware that comes in modern SATA controllers. PCI MSI, found on all new controllers, tends to mitigate interrupt overhead even further. But our data behaviour is mostly synchronous, whereas network stuff is more asynchronous. But still, with multiple drives all using NCQ on a port, a polled interface option might give higher throughput and lower CPU loading. There are definitely options worth exploring. And with full ports all doing NCQ, you certainly have a lot of events to deal with. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Tejun Heo wrote: Jeff Garzik wrote: Is this NACK on the patchset or can we update PMP access later? Sorry, yes, it is a NAK: polling should not be requirement. I considered making multi-protocol ->qc_issue() a requirement too, but that seemed like it might delay things too much. But consider this a strong to-do item... ->pmp_read and ->pmp_write hooks should be folded into ->qc_issue and ata_qc_complete(), because quite often a PMP read/write packet can be delivered just like any other packet. We want a single "submit packet to hardware" interface, not multiple ones. Aieee... Another merge delay. I wish the review process proceeded a bit swifter. The patchset has been around literally for years now and submitted for review six times if I have the take number right. :-( Well the vast majority of the patches are in, what five out of six original patchsets? Sorry I didn't catch the polling requirement beforehand, it was not really clear from a quick read. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Alan Cox wrote: Aieee... Another merge delay. I wish the review process proceeded a bit swifter. The patchset has been around literally for years now and submitted for review six times if I have the take number right. :-( Agreed - thats one reason I stick everything in -mm. Its taking several months for a known fix to get from me to the outside. I'm not sure its entirely a Jeff problem however. This doesn't affect Tejun or anyone else really, but going through -mm tends to gum up the works. Your patches wind up not applying to libata#upstream, in which case they get dropped and I assume Andrew or someone will resend usable versions. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
> Aieee... Another merge delay. I wish the review process proceeded a bit > swifter. The patchset has been around literally for years now and > submitted for review six times if I have the take number right. :-( Agreed - thats one reason I stick everything in -mm. Its taking several months for a known fix to get from me to the outside. I'm not sure its entirely a Jeff problem however. - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Jeff Garzik wrote: >> Is this NACK on the patchset or can we update PMP access later? > > Sorry, yes, it is a NAK: polling should not be requirement. > > I considered making multi-protocol ->qc_issue() a requirement too, but > that seemed like it might delay things too much. But consider this a > strong to-do item... ->pmp_read and ->pmp_write hooks should be folded > into ->qc_issue and ata_qc_complete(), because quite often a PMP > read/write packet can be delivered just like any other packet. We want > a single "submit packet to hardware" interface, not multiple ones. Aieee... Another merge delay. I wish the review process proceeded a bit swifter. The patchset has been around literally for years now and submitted for review six times if I have the take number right. :-( -- tejun - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Jeff Garzik wrote: .. As such, polling is simply an outmoded concept. It does not make sense on new hardware, and forcing design decisions down that path only lead to a cascade of similar design decisions -- pmp_read polling being just one example of such a result. A related tangent here, but I wonder if someday we might consider something like NAPI for libata. Our I/O rates can far exceed those of the network stack, and there polling (NAPI) has helped quite a bit. But our data behaviour is mostly synchronous, whereas network stuff is more asynchronous. But still, with multiple drives all using NCQ on a port, a polled interface option might give higher throughput and lower CPU loading. Just like with the networking code. Not that this specifically relates to concerns with the current PMP patches, but I'm curious where Jeff sees this all going, as he works on both sides. Cheers - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Tejun Heo wrote: Jeff Garzik wrote: [--snip--] There, even the concept of "port" is fluid, and the libata EH model of freezing and thawing a port (with the desired irq-off result) just doesn't fit the hardware. Well, the current model was developed while struggling with the first generation PMP hardware which had a lot of quirks. More focus was on making the system run and keeping it that way. Anyways, now that the quirks are better understood, I think we can make PMP register access irq-driven safely. [--snip--] As such, polling is simply an outmoded concept. It does not make sense on new hardware, and forcing design decisions down that path only lead to a cascade of similar design decisions -- pmp_read polling being just one example of such a result. For pmp read/write, I agree that IRQ driven access would be better but even for fairly advanced controllers like ahci or sil24, I think resetting-by-polling is a good idea. Not for SAS/SATA controllers. These SAS+SATA controllers allow you set a bit initiating phy reset, and then receive an interrupt when something interesting happens. The reason why I (we?) am just finding out about this is: both drivers/scsi/libsas and drivers/scsi/ipr are still using the old-EH :( But overall, anything sufficiently "high level" is not friendly to polling -- if a polling model is even possible. Just like the Linux kernel MM platform API presents 3 levels of page table entries, even when the hardware may only have 2, libata high level API _must_ be implemented as 100% asynchronous event driven API. Or allow both? If the default implementation chooses to use polling -- i.e. all SFF Note the line quoted above, immediately following your response... such a setup does permit both: an asynchronous submit/complete API can be implementing under the hood via polling or irq, as the driver chooses. The main point is to not /require/ polling. controllers -- that's fine. But in the new SAS/SATA world its clear that we have far too many polling-related assumptions as it is. Polling just flat out doesn't make sense on modern SAS/SATA -- and even a couple modern SATA controllers. On such controllers, we are notified immediately via interrupt even in the event of errors. For SAS, I don't have any strong opinion. For SATA, I think we definitely need to allow or even prefer polling for host port resets. Is this NACK on the patchset or can we update PMP access later? Sorry, yes, it is a NAK: polling should not be requirement. I considered making multi-protocol ->qc_issue() a requirement too, but that seemed like it might delay things too much. But consider this a strong to-do item... ->pmp_read and ->pmp_write hooks should be folded into ->qc_issue and ata_qc_complete(), because quite often a PMP read/write packet can be delivered just like any other packet. We want a single "submit packet to hardware" interface, not multiple ones. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Tejun Heo wrote: > Is this NACK on the patchset or can we update PMP access later? Ping? -- tejun - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Jeff Garzik wrote: [--snip--] > There, even the concept of "port" is fluid, and the libata EH model of > freezing and thawing a port (with the desired irq-off result) just > doesn't fit the hardware. Well, the current model was developed while struggling with the first generation PMP hardware which had a lot of quirks. More focus was on making the system run and keeping it that way. Anyways, now that the quirks are better understood, I think we can make PMP register access irq-driven safely. [--snip--] > As such, polling is simply an outmoded concept. It does not make sense > on new hardware, and forcing design decisions down that path only lead > to a cascade of similar design decisions -- pmp_read polling being just > one example of such a result. For pmp read/write, I agree that IRQ driven access would be better but even for fairly advanced controllers like ahci or sil24, I think resetting-by-polling is a good idea. > Just like the Linux kernel MM platform API presents 3 levels of page > table entries, even when the hardware may only have 2, libata high level > API _must_ be implemented as 100% asynchronous event driven API. Or allow both? > If the default implementation chooses to use polling -- i.e. all SFF > controllers -- that's fine. But in the new SAS/SATA world its clear > that we have far too many polling-related assumptions as it is. > > Polling just flat out doesn't make sense on modern SAS/SATA -- and even > a couple modern SATA controllers. On such controllers, we are notified > immediately via interrupt even in the event of errors. For SAS, I don't have any strong opinion. For SATA, I think we definitely need to allow or even prefer polling for host port resets. Is this NACK on the patchset or can we update PMP access later? -- tejun - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Jeff Garzik wrote: Just like the Linux kernel MM platform API presents 3 levels of page table entries, even when the hardware may only have 2, libata high level API _must_ be implemented as 100% asynchronous event driven API. To be more clear, non-taskfile packets/commands should be delivered via ->qc_issue, just like taskfiles. This is similar to sas_task in libsas, which can be a SATA packet, a SAS packet, a management packet, ... In the long run, we should probably look at integrating ata_queued_cmd and sas_task. Once ata_queued_cmd becomes multi-protocol (tf, pmp, sgpio, ...), it becomes more generic. sas_task is also multi-protocol. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
