Re: [patch] pci: pci_enable_device_bars() fix
Andrew Morton wrote: Actually I (and probably others) generally avoid cc'ing mailing lists on patch traffic. I spew out enough script-generated traffic as it is. You pretty much always ensure the driver author gets CC'd, which is exemplary :) Jeff - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] pci: pci_enable_device_bars() fix
Ingo Molnar wrote: * Jeff Garzik <[EMAIL PROTECTED]> wrote: Ingo Molnar wrote: so please tell me Jeff. If Greg, who is the super-maintainer of your code area, and who deals with your code every day and changes it every minute and hour, simply did not Cc: the SCSI list - how am i, a largely outside party in this matter, supposed to notice that 3 maintainers and 3 mailing lists in the Cc: were somehow not enough and that i was supposed to grow the already sizable Cc: list even more? Because, regardless of the situation, it's both common courtesy and wise practice to CC relevant driver maintainers, when you touch a driver. And it's just common sense: Greg simply does not know the intimate details of every PCI driver. Nor do I. Nor you. In the case of lpfc here, we have an active driver maintainer, and an up-to-date MAINTAINERS entry. Even if you are too slack to read MAINTAINERS, 'git log' would have given you the same info. Don't pretend there is some benefit here to ignoring the people that best know the driver. I don't buy that; it simply makes no engineering sense whatsoever. what you _STILL_ do not realize is the following: you still attribute the lack of Cc:s to some intention of mine. No, it was not my intention. I was never speaking to intent. I was noting that, having been in the kernel community for years, both of you guys should know that you should always CC a driver author, when touching their driver. Even after this thread, I have not even heard a "yes, I agree, I should have CC'd the driver author since they know the most about the driver" from either of you, which is quite disappointing. Instead, I get this long thread in response... is just super fragile and does not serve users at all. Even Greg and i got it wrong accidentally. If _we_ get it wrong, who will get it Sure. But... do you agree the CC list should have included the driver author? Do you agree that a mistake was made in this case? Jeff - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] pci: pci_enable_device_bars() fix
On Mon, 4 Feb 2008 13:57:36 +0100 Ingo Molnar <[EMAIL PROTECTED]> wrote: > > * Jeff Garzik <[EMAIL PROTECTED]> wrote: > > > Ingo Molnar wrote: > >> so please tell me Jeff. If Greg, who is the super-maintainer of your > >> code area, and who deals with your code every day and changes it > >> every minute and hour, simply did not Cc: the SCSI list - how am i, a > >> largely outside party in this matter, supposed to notice that 3 > >> maintainers and 3 mailing lists in the Cc: were somehow not enough > >> and that i was supposed to grow the already sizable Cc: list even > >> more? > > > > Because, regardless of the situation, it's both common courtesy and > > wise practice to CC relevant driver maintainers, when you touch a > > driver. > > > > And it's just common sense: Greg simply does not know the intimate > > details of every PCI driver. Nor do I. Nor you. > > > > In the case of lpfc here, we have an active driver maintainer, and an > > up-to-date MAINTAINERS entry. Even if you are too slack to read > > MAINTAINERS, 'git log' would have given you the same info. > > > > Don't pretend there is some benefit here to ignoring the people that > > best know the driver. I don't buy that; it simply makes no > > engineering sense whatsoever. > > what you _STILL_ do not realize is the following: you still attribute > the lack of Cc:s to some intention of mine. No, it was not my intention. > At first glance the Cc: looked large and complete enough in an > _existing_ discussion and that's was the end of my (brief) attention > regarding the Cc: line. Yes, it would have been a bit better had i > noticed the lack of Cc:s in an existing discussion, but i didnt. Actually I (and probably others) generally avoid cc'ing mailing lists on patch traffic. I spew out enough script-generated traffic as it is. > ... > mailing list aliases to get the 'guaranteed attention' of maintainers > whoa. You must know better mailing lists than I do ;) - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] pci: pci_enable_device_bars() fix
* Jeff Garzik <[EMAIL PROTECTED]> wrote: > Ingo Molnar wrote: >> so please tell me Jeff. If Greg, who is the super-maintainer of your >> code area, and who deals with your code every day and changes it >> every minute and hour, simply did not Cc: the SCSI list - how am i, a >> largely outside party in this matter, supposed to notice that 3 >> maintainers and 3 mailing lists in the Cc: were somehow not enough >> and that i was supposed to grow the already sizable Cc: list even >> more? > > Because, regardless of the situation, it's both common courtesy and > wise practice to CC relevant driver maintainers, when you touch a > driver. > > And it's just common sense: Greg simply does not know the intimate > details of every PCI driver. Nor do I. Nor you. > > In the case of lpfc here, we have an active driver maintainer, and an > up-to-date MAINTAINERS entry. Even if you are too slack to read > MAINTAINERS, 'git log' would have given you the same info. > > Don't pretend there is some benefit here to ignoring the people that > best know the driver. I don't buy that; it simply makes no > engineering sense whatsoever. what you _STILL_ do not realize is the following: you still attribute the lack of Cc:s to some intention of mine. No, it was not my intention. At first glance the Cc: looked large and complete enough in an _existing_ discussion and that's was the end of my (brief) attention regarding the Cc: line. Yes, it would have been a bit better had i noticed the lack of Cc:s in an existing discussion, but i didnt. [ And it might not surprise you if i observe here that i think this little mishap is further (incidental) proof that having this many mailing list aliases to get the 'guaranteed attention' of maintainers is just super fragile and does not serve users at all. Even Greg and i got it wrong accidentally. If _we_ get it wrong, who will get it right? I see several mis-Cc:ed emails every day and there's over a 1000 unfixed bugs in bugzilla for 2.6 alone. It does not take a genius to observe that something is fundamentally wrong ;-) That 2.6 works on your or my box is a _very_ poor metric - we both fix bugs on our systems super fast so we've got a very biased first-hand experience about the stability and reliability of the kernel. We never really feel the kind of frustration that testers feel when their mail to lkml gets ignored. We never really feel the helplessness that comes from unfixed bugs. ] Ingo - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] pci: pci_enable_device_bars() fix
Ingo Molnar wrote: so please tell me Jeff. If Greg, who is the super-maintainer of your code area, and who deals with your code every day and changes it every minute and hour, simply did not Cc: the SCSI list - how am i, a largely outside party in this matter, supposed to notice that 3 maintainers and 3 mailing lists in the Cc: were somehow not enough and that i was supposed to grow the already sizable Cc: list even more? Because, regardless of the situation, it's both common courtesy and wise practice to CC relevant driver maintainers, when you touch a driver. And it's just common sense: Greg simply does not know the intimate details of every PCI driver. Nor do I. Nor you. In the case of lpfc here, we have an active driver maintainer, and an up-to-date MAINTAINERS entry. Even if you are too slack to read MAINTAINERS, 'git log' would have given you the same info. Don't pretend there is some benefit here to ignoring the people that best know the driver. I don't buy that; it simply makes no engineering sense whatsoever. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] pci: pci_enable_device_bars() fix
* Jeff Garzik <[EMAIL PROTECTED]> wrote: >> so i'm still totally befuddled why you think that there was anything >> particularly wrong or unhelpful about me replying to the specific >> pull request that introduced a particular breakage into the kernel. >> Had i mailed to lkml with a terse "kernel build broke" message with >> just an URL to a config and the build breakage, you could rightfully >> have complained that i should have done more to properly direct my >> bugreport. But this breakage was about a PCI API change, the pull >> request had a PCI mailing list Cc:-ed, why should i have thought that >> this needs the attention of any other parties? > > Because the change required knowledge not only of PCI, but of the > hardware in question. As your patch demonstrated. > > And yes -- the original changes should have been CC'd to interested > parties as well. I'm still waiting to hear back from Alan or Bart > whether the ATA/IDE changes in that PCI pile actually work... the > original changeset even noted that relevant parties had not yet been > queried. so please tell me Jeff. If Greg, who is the super-maintainer of your code area, and who deals with your code every day and changes it every minute and hour, simply did not Cc: the SCSI list - how am i, a largely outside party in this matter, supposed to notice that 3 maintainers and 3 mailing lists in the Cc: were somehow not enough and that i was supposed to grow the already sizable Cc: list even more? Ingo - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] pci: pci_enable_device_bars() fix
* James Bottomley <[EMAIL PROTECTED]> wrote: > Are you seriously telling us that it required too much investigation > on your part to figure out that something with a compile failure in > drivers/scsi might belong on the scsi list? This is getting silly. Let me repeat it, because IMO it's really straightforward. My (quick) investigation based on the function name that was in the error message: drivers/scsi/lpfc/lpfc_init.c:1897: error: implicit declaration of function 'pci_enable_device_bars' a straightforward search on "pci_enable_device_bars" led to a recent PCI API related change pushed by Greg, with the following straightforward subject line: [GIT PATCH] PCI patches for 2.6.24 http://lkml.org/lkml/2008/2/1/483 the email had this description: | Some general cleanups, minor tweaks, and a bit of PCI hotplug | updates, and some PCI Express updates for new features, if your | hardware happens to support it. furthermore, the mail already had two PCI mailing lists in its Cc: line. The PCI subsystem regularly does cross-treee changes, by its nature. i had all reasons to believe that this was a (innocious looking) PCI subsystem change and a harmless (but a tad under-tested) API cleanup that went haywire: it smelled like PCI, it walked like PCI and it quacked like PCI. So to me it was clearly a PCI merge not an SCSI merge, and i was really only interested in the first hop, i.e. i was primarily interested in the pull request that clearly changed multiple subsystems, and a seemingly API change that broke the build. Three mailing lists and three maintainers were already on the Cc: line for that pull request. So tell me, exactly what should have let me to believe that i should have added anyone else to the _already_ sizable Cc: line?? I could have done it, had i have more time and had i realized the full scope of the change and the somewhat misleading Cc:s that were on the original pull request, but i clearly was not _required_ to - and your suggestions to the contrary are ridiculous. Furthermore i reject the sometimes derogatory undertone of your mails that implies that i should somehow have done more or different work than i already did. I really hope you treat other contributors and bug-reporters better than you treated me :( Shall this be my last voluntary SCSI contribution for a good while. Ingo - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] pci: pci_enable_device_bars() fix
Ingo Molnar wrote: * Jeff Garzik <[EMAIL PROTECTED]> wrote: Ingo Molnar wrote: it would have been totally appropriate for me to just send a mail to lkml with the proper subject line about the breakage. (I might even have decided to stay completely silent about the issue and fix it for my own build, letting you guys figure it out.) Oh come on... You are smart enough to know to at least CC the driver maintainer, the key POC who should be aware of breakage of their driver. That is a standard courtesy. is there any particular reason why you cut out the most relevant part of my reply, which happens to answer all your questions AFAICS: Instead i did a search of lkml (based on the function name in the build error) and figured out where the pull request was on lkml: Greg. I replied to that mail, he'll obviously know whom else to Cc from that point on (if anyone). I really didnt want to (nor did i need to) figure out whether this was some general driver level API change that happen kernel-wide, or some SCSI specific change. I simply replied to the pull request whose Cc: line seemed well-populated to me already. I also took a look at the commit itself and did a quick hack in a hurry to keep the tests rolling. It really did not occur to me that i should have added anyone else to the Cc: line, as [EMAIL PROTECTED] was Cc:-ed already so i assumed the interest was from that angle. had you read this portion you'd have realized that i did not search for any "owner" of the file, i simply searched for the person who introduced the change, and the on-lkml mail where the change was introduced. And therein lies the problem. The original submittor omitted relevant maintainers, you followed that [incorrect] example, and the end result was clear: an obviously wrong change. Thus, the problem is precisely what you stated: you did not bother to search for people who care about that file. and that's all that should be needed, really. Believe me, i hit tons of Hardly. What part of "this change requires knowledge of the hardware" is difficult to understand? And if you do not have that knowledge, why is it so trying to CC people who actively maintain a driver, and have that knowledge? It's simple common sense to -ask- or at least -notify- in such cases. That the original submittor made the same mistake is no reason to repeat the mistake. bugs all across the kernel, often several bugs a day, and it's hard even for me to figure out who "maintains" a file and when. (and in Linux there's no "ownership" of files anyway) So as a general rule i go after changes instead, and that's exactly what i did here too. I do delta/regression QA - i.e. i watch for _changes_ that break the kernel and hence the general 'owner' of a file is often irrelevant - it's the maintainer who introduces a change who matters, and we do lots of cross-maintain merges. Only if i do not manage to identify a change do i try to figure out who maintains a file at that given moment. (But those mails often go into black holes, they get bounced, subscriber-required email lists, etc. etc.) It's also nontrivial to map the files to the MAINTAINERS file, and it's also quite outdated in some portions. So the MAINTAINERS file is the last resort i use. That's a long list of excuses in an attempt to ignore the facts: Fact 1: The driver you modified is actively maintained Fact 2: The driver maintainer has respectfully indicated, through the standard community mechanism, the useful points of contact. Fact 3: The MAINTAINERS entry is correct and up-to-date. Fact 4: Even if you wanted to ignore MAINTAINERS, 'git log' on the relevant file could have told you who are useful parties to CC. It's just common courtesy to CC a driver maintainer, ESPECIALLY when a change requires knowledge of the driver/hardware in question. so i'm still totally befuddled why you think that there was anything particularly wrong or unhelpful about me replying to the specific pull request that introduced a particular breakage into the kernel. Had i mailed to lkml with a terse "kernel build broke" message with just an URL to a config and the build breakage, you could rightfully have complained that i should have done more to properly direct my bugreport. But this breakage was about a PCI API change, the pull request had a PCI mailing list Cc:-ed, why should i have thought that this needs the attention of any other parties? Because the change required knowledge not only of PCI, but of the hardware in question. As your patch demonstrated. And yes -- the original changes should have been CC'd to interested parties as well. I'm still waiting to hear back from Alan or Bart whether the ATA/IDE changes in that PCI pile actually work... the original changeset even noted that relevant parties had not yet been queried. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body
Re: [patch] pci: pci_enable_device_bars() fix
On Sat, 2008-02-02 at 18:08 +0100, Ingo Molnar wrote: > * Jeff Garzik <[EMAIL PROTECTED]> wrote: > > > Ingo Molnar wrote: > >> === > >> --- linux.orig/drivers/scsi/lpfc/lpfc_init.c > >> +++ linux/drivers/scsi/lpfc/lpfc_init.c > >> @@ -1894,7 +1894,7 @@ lpfc_pci_probe_one(struct pci_dev *pdev, > >>uint16_t iotag; > >>int bars = pci_select_bars(pdev, IORESOURCE_MEM); > >> - if (pci_enable_device_bars(pdev, bars)) > >> + if (pci_enable_device_io(pdev)) > >>goto out; > > > > Look at the line right above it... AFAICS you want > > pci_enable_device_mem(), if the mask is selecting IORESOURCE_MEM BARs. > > > > Also a CC to linux-scsi and the driver author would be nice, as they > > are the ones with hardware and can verify. > > it would have been totally appropriate for me to just send a mail to > lkml with the proper subject line about the breakage. (I might even have > decided to stay completely silent about the issue and fix it for my own > build, letting you guys figure it out.) We agreed to differ a while ago on your head in the sand, post it to LKML because everybody follows that list attitude. Some nice soul will always (eventually) forward to the correct list, so this practice more or less works (just not in as timely fashion as actually posting to the relevant list). > Instead i did a search of lkml (based on the function name in the build > error) and figured out where the pull request was on lkml: Greg. I > replied to that mail, he'll obviously know whom else to Cc from that > point on (if anyone). I really didnt want to (nor did i need to) figure > out whether this was some general driver level API change that happen > kernel-wide, or some SCSI specific change. I simply replied to the pull > request whose Cc: line seemed well-populated to me already. I also took > a look at the commit itself and did a quick hack in a hurry to keep the > tests rolling. It really did not occur to me that i should have added > anyone else to the Cc: line, as [EMAIL PROTECTED] was > Cc:-ed already so i assumed the interest was from that angle. > > ( And as this was spent from my family's weekend time and i had no time > and no interest to dig any further than to figure out the "first hop" > of the change that broke the build, and the parties who initiated that > hop. I'm in fact surprised that your and James's answer to my > bugreport is hostility. ) What's hostile about telling you your patch is wrong and pointing you at the correct one? > So i find your suggestion that i should have added more people to the > Cc: line unfair on several levels. > > > This set of changes seemed like 50% guesswork to me, without > > consulting the authors :( And unlike many changes, you actually have > > to know the hardware [or get clues from surrounding code] to make the > > change. > > you mean the whole set of changes? Or just mine? Mine was a 30 seconds > guesswork of course, i dont have the hardware, i didnt do the change, > nor did i do anything near that code - i just saw the build failure stop > my testing and sent in this notification and a hack. That's why i sent > it to Greg, as a reply to the mail where he pushed these changes > upstream and who'll know what to do with it from that point on. My patch > can be totally thrown away (and should be, apparently). > > but ... i guess next time i'll think twice before sending any bugreports > about or related to the SCSI code anywhere, unless they become really > annoying. Who needs this hassle? Oh good grief, don't come the raw prawn with us! You're a kernel maintainer, you are expected to know the rules and follow them. The very fact that a well known maintainer posts a patch with a signed-off-by to Andrew and Linus means that it likely gets applied. Because of this, maintainers and other people with similarly trusted positions are expected to go via the lists and subsystems in part as general courtesy, but also to verify that their patch is actually valid before it gets applied. Are you seriously telling us that it required too much investigation on your part to figure out that something with a compile failure in drivers/scsi might belong on the scsi list? Let me tell you exactly what would have happened if that patch had been applied: Because the wrong bars were enabled, the device would have attached but been non-functional. Likely Emulex would have picked this up in their -rc1 testing and spent several days trying to trace the cause in their labs. Chances are, that, because this type of breakage is extremely subtle, they wouldn't have noticed the fact that the enable should have been _mem not _io. Then they would have raised the issue to linux-scsi. It would probably take at least a person week of effort before anyone finally noticed what the issue was. If you can't see that your behaviour needs modifying, I suggest you seriously conside
Re: [patch] pci: pci_enable_device_bars() fix
* Jeff Garzik <[EMAIL PROTECTED]> wrote: > Ingo Molnar wrote: >> it would have been totally appropriate for me to just send a mail to lkml >> with the proper subject line about the breakage. (I might even have >> decided to stay completely silent about the issue and fix it for my own >> build, letting you guys figure it out.) > > Oh come on... You are smart enough to know to at least CC the driver > maintainer, the key POC who should be aware of breakage of their > driver. That is a standard courtesy. is there any particular reason why you cut out the most relevant part of my reply, which happens to answer all your questions AFAICS: >> Instead i did a search of lkml (based on the function name in the >> build error) and figured out where the pull request was on lkml: >> Greg. I replied to that mail, he'll obviously know whom else to Cc >> from that point on (if anyone). I really didnt want to (nor did i >> need to) figure out whether this was some general driver level API >> change that happen kernel-wide, or some SCSI specific change. I >> simply replied to the pull request whose Cc: line seemed >> well-populated to me already. I also took a look at the commit itself >> and did a quick hack in a hurry to keep the tests rolling. It really >> did not occur to me that i should have added anyone else to the Cc: >> line, as [EMAIL PROTECTED] was Cc:-ed already so i >> assumed the interest was from that angle. had you read this portion you'd have realized that i did not search for any "owner" of the file, i simply searched for the person who introduced the change, and the on-lkml mail where the change was introduced. and that's all that should be needed, really. Believe me, i hit tons of bugs all across the kernel, often several bugs a day, and it's hard even for me to figure out who "maintains" a file and when. (and in Linux there's no "ownership" of files anyway) So as a general rule i go after changes instead, and that's exactly what i did here too. I do delta/regression QA - i.e. i watch for _changes_ that break the kernel and hence the general 'owner' of a file is often irrelevant - it's the maintainer who introduces a change who matters, and we do lots of cross-maintain merges. Only if i do not manage to identify a change do i try to figure out who maintains a file at that given moment. (But those mails often go into black holes, they get bounced, subscriber-required email lists, etc. etc.) It's also nontrivial to map the files to the MAINTAINERS file, and it's also quite outdated in some portions. So the MAINTAINERS file is the last resort i use. so i'm still totally befuddled why you think that there was anything particularly wrong or unhelpful about me replying to the specific pull request that introduced a particular breakage into the kernel. Had i mailed to lkml with a terse "kernel build broke" message with just an URL to a config and the build breakage, you could rightfully have complained that i should have done more to properly direct my bugreport. But this breakage was about a PCI API change, the pull request had a PCI mailing list Cc:-ed, why should i have thought that this needs the attention of any other parties? Ingo - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] pci: pci_enable_device_bars() fix
Ingo Molnar wrote: it would have been totally appropriate for me to just send a mail to lkml with the proper subject line about the breakage. (I might even have decided to stay completely silent about the issue and fix it for my own build, letting you guys figure it out.) Oh come on... You are smart enough to know to at least CC the driver maintainer, the key POC who should be aware of breakage of their driver. That is a standard courtesy. ( And as this was spent from my family's weekend time and i had no time and no interest to dig any further than to figure out the "first hop" of the change that broke the build, and the parties who initiated that hop. I'm in fact surprised that your and James's answer to my bugreport is hostility. ) I'm sorry you read "would be nice" as hostility. So i find your suggestion that i should have added more people to the Cc: line unfair on several levels. As I noted, it is an obvious courtesy to CC the driver maintainer, at the very least. _Especially_ when it is a change that requires some knowledge of the hardware, as was this case. This set of changes seemed like 50% guesswork to me, without consulting the authors :( And unlike many changes, you actually have to know the hardware [or get clues from surrounding code] to make the change. you mean the whole set of changes? The whole set of changes, yes, not just yours. but ... i guess next time i'll think twice before sending any bugreports about or related to the SCSI code anywhere, unless they become really annoying. Who needs this hassle? The fact is, each larger subsystem (net, scsi, ata I know) has several vendor contacts and driver maintainers who for various reasons prefer a more focused -- and often less hostile -- mailing list to LKML. I have a hard enough time as it is, trying to convince hardware vendors to work with us, that we are not all assholes. How about respecting the preferences of certain segments of a very large community, even when they differ from your own? We have a community-accepted method of expressing these preferences, the MAINTAINERS file. IMO, standard practice should be: * To or CC: driver maintainer mentioned in MAINTAINERS (if any) * CC: LKML, any list mentioned in MAINTAINERS So, how about CC'ing the targets that have nicely requested to be CC'd? Jeff - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] pci: pci_enable_device_bars() fix
* Jeff Garzik <[EMAIL PROTECTED]> wrote: > Ingo Molnar wrote: >> === >> --- linux.orig/drivers/scsi/lpfc/lpfc_init.c >> +++ linux/drivers/scsi/lpfc/lpfc_init.c >> @@ -1894,7 +1894,7 @@ lpfc_pci_probe_one(struct pci_dev *pdev, >> uint16_t iotag; >> int bars = pci_select_bars(pdev, IORESOURCE_MEM); >> - if (pci_enable_device_bars(pdev, bars)) >> +if (pci_enable_device_io(pdev)) >> goto out; > > Look at the line right above it... AFAICS you want > pci_enable_device_mem(), if the mask is selecting IORESOURCE_MEM BARs. > > Also a CC to linux-scsi and the driver author would be nice, as they > are the ones with hardware and can verify. it would have been totally appropriate for me to just send a mail to lkml with the proper subject line about the breakage. (I might even have decided to stay completely silent about the issue and fix it for my own build, letting you guys figure it out.) Instead i did a search of lkml (based on the function name in the build error) and figured out where the pull request was on lkml: Greg. I replied to that mail, he'll obviously know whom else to Cc from that point on (if anyone). I really didnt want to (nor did i need to) figure out whether this was some general driver level API change that happen kernel-wide, or some SCSI specific change. I simply replied to the pull request whose Cc: line seemed well-populated to me already. I also took a look at the commit itself and did a quick hack in a hurry to keep the tests rolling. It really did not occur to me that i should have added anyone else to the Cc: line, as [EMAIL PROTECTED] was Cc:-ed already so i assumed the interest was from that angle. ( And as this was spent from my family's weekend time and i had no time and no interest to dig any further than to figure out the "first hop" of the change that broke the build, and the parties who initiated that hop. I'm in fact surprised that your and James's answer to my bugreport is hostility. ) So i find your suggestion that i should have added more people to the Cc: line unfair on several levels. > This set of changes seemed like 50% guesswork to me, without > consulting the authors :( And unlike many changes, you actually have > to know the hardware [or get clues from surrounding code] to make the > change. you mean the whole set of changes? Or just mine? Mine was a 30 seconds guesswork of course, i dont have the hardware, i didnt do the change, nor did i do anything near that code - i just saw the build failure stop my testing and sent in this notification and a hack. That's why i sent it to Greg, as a reply to the mail where he pushed these changes upstream and who'll know what to do with it from that point on. My patch can be totally thrown away (and should be, apparently). but ... i guess next time i'll think twice before sending any bugreports about or related to the SCSI code anywhere, unless they become really annoying. Who needs this hassle? Ingo - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] pci: pci_enable_device_bars() fix
On Sat, 2008-02-02 at 10:51 -0500, Jeff Garzik wrote: > Ingo Molnar wrote: > > === > > --- linux.orig/drivers/scsi/lpfc/lpfc_init.c > > +++ linux/drivers/scsi/lpfc/lpfc_init.c > > @@ -1894,7 +1894,7 @@ lpfc_pci_probe_one(struct pci_dev *pdev, > > uint16_t iotag; > > int bars = pci_select_bars(pdev, IORESOURCE_MEM); > > > > - if (pci_enable_device_bars(pdev, bars)) > > + if (pci_enable_device_io(pdev)) > > goto out; > > Look at the line right above it... AFAICS you want > pci_enable_device_mem(), if the mask is selecting IORESOURCE_MEM BARs. > > Also a CC to linux-scsi and the driver author would be nice, as they are > the ones with hardware and can verify. > > This set of changes seemed like 50% guesswork to me, without consulting > the authors :( And unlike many changes, you actually have to know the > hardware [or get clues from surrounding code] to make the change. Jeff is right, this fix is wrong. The correct fix is here: http://marc.info/?l=linux-scsi&m=120196768605031 Ingo, could you please cc linux-scsi on your mails ... it would have been a complete disaster to have this incorrect patch go in. James - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] pci: pci_enable_device_bars() fix
Ingo Molnar wrote: === --- linux.orig/drivers/scsi/lpfc/lpfc_init.c +++ linux/drivers/scsi/lpfc/lpfc_init.c @@ -1894,7 +1894,7 @@ lpfc_pci_probe_one(struct pci_dev *pdev, uint16_t iotag; int bars = pci_select_bars(pdev, IORESOURCE_MEM); - if (pci_enable_device_bars(pdev, bars)) + if (pci_enable_device_io(pdev)) goto out; Look at the line right above it... AFAICS you want pci_enable_device_mem(), if the mask is selecting IORESOURCE_MEM BARs. Also a CC to linux-scsi and the driver author would be nice, as they are the ones with hardware and can verify. This set of changes seemed like 50% guesswork to me, without consulting the authors :( And unlike many changes, you actually have to know the hardware [or get clues from surrounding code] to make the change. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
