Re: [patch] pci: pci_enable_device_bars() fix

Re: [patch] pci: pci_enable_device_bars() fix

[Jeff Garzik]

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

Re: [patch] pci: pci_enable_device_bars() fix

[James Bottomley]

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

Re: [patch] pci: pci_enable_device_bars() fix

[Ingo Molnar]

* Jeff Garzik <jeff@garzik.org> 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 linux-pci@atrey.karlin.mff.cuni.cz 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

Re: [patch] pci: pci_enable_device_bars() fix

[Jeff Garzik]

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

Re: [patch] pci: pci_enable_device_bars() fix

[Ingo Molnar]

* Jeff Garzik <jeff@garzik.org> 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 linux-pci@atrey.karlin.mff.cuni.cz 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

Re: [patch] pci: pci_enable_device_bars() fix

[James Bottomley]

On Sat, 2008-02-02 at 18:08 +0100, Ingo Molnar wrote:
> * Jeff Garzik <jeff@garzik.org> 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 linux-pci@atrey.karlin.mff.cuni.cz 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 consider your position.  For all our talk of a nice utopian
democracy of meritocracy in Open Source, we're critically dependent on
the tree gatekeepers to maintain this.  None of us is immune from the
subtle lure of the arrogance of power but it's a corrosive poison and
must be avoided at every turn.  You and I follow more rules and are held
to a higher standard that the average patch submitter because of our
position in the community (not in spite of it).

James

Re: [patch] pci: pci_enable_device_bars() fix

[Jeff Garzik]

Ingo Molnar wrote:
> * Jeff Garzik <jeff@garzik.org> 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 linux-pci@atrey.karlin.mff.cuni.cz 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

Re: [patch] pci: pci_enable_device_bars() fix

[Ingo Molnar]

* James Bottomley <James.Bottomley@HansenPartnership.com> 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

Re: [patch] pci: pci_enable_device_bars() fix

[Ingo Molnar]

* Jeff Garzik <jeff@garzik.org> 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

Re: [patch] pci: pci_enable_device_bars() fix

[Jeff Garzik]

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

Re: [patch] pci: pci_enable_device_bars() fix

[Ingo Molnar]

* Jeff Garzik <jeff@garzik.org> 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

Re: [patch] pci: pci_enable_device_bars() fix

[Andrew Morton]

On Mon, 4 Feb 2008 13:57:36 +0100 Ingo Molnar <mingo@elte.hu> wrote:

> 
> * Jeff Garzik <jeff@garzik.org> 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 ;)

Re: [patch] pci: pci_enable_device_bars() fix

[Jeff Garzik]

Ingo Molnar wrote:
> * Jeff Garzik <jeff@garzik.org> 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

Re: [patch] pci: pci_enable_device_bars() fix

[Jeff Garzik]

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