OProfile

PiBXaHkgZG8geW91IG5lZWQgdGhpcyBjaGFuZ2U/ICBJdCBhbG1vc3QgbG9va3MgYXMgaWYgeW91
J3JlIHBhcGVyaW5nIG92ZXIKPiBhIGJ1ZyB3aGVyZSBhZGRfc2FtcGxlIHNob3VsZCBub3QgYmUg
Y2FsbGVkIGF0IGFsbC4KClllYWgsdGhpcyBjaGFuZ2UgaXMgdG8gZW5oYW5jZSB0aGUgcm9idXN0
IG9mIG9wcm9maWxlLiBXaGVuIHVzaW5nCnBlcmZvcm1hY2UgY291bnRlciBtYW51YWxseSh3cml0
dGluZyBjb250cm9sIHJlZ2lzdGVyIGluIGEgbW9kdWxlLCBubwpuZWVkIHRvIHVzZSB0aGUgb3By
b2ZpbGUpLEkgdXN1YWxseSBtYWtlIGtlcm5lbCBwYW5pYyBpZiBJIGRvIG5vdAppbml0aWFsaXpl
IHRoZSBvcHJvZmlsZSBhbmQgZW5hYmxlIHRoZSBvdmVyZmxvdyBpbnRlcnJ1cHQgY2FyZWxlc3Ns
eS4KU28sIHRoaXMgY2hhbmdlIGNhbiBhdm9pZCB0aGlzIHBhbmljLiA6RAoKMjAwNy83LzI0LCBS
YWxmIEJhZWNobGUgPHJhbGZAbGludXgtbWlwcy5vcmc+Ogo+IE9uIFR1ZSwgSnVsIDI0LCAyMDA3
IGF0IDAxOjIwOjI3UE0gKzA0MDAsIERhamllIFRhbiB3cm90ZToKPgo+ID4gVGhpcyBwYXRjaCBh
ZGRzIHN1cHBvcnQgZm9yIHByb2ZpbGluZyBMb29uZ3NvbiAyRS4gSXQncyBiZWVuIHRlc3RlZCBv
bgo+ID4gRnVMb25nIG1pbmkgUEMobG9vbmdzb24yZSBpbnNpZGUpLgo+Cj4gRmlyc3Qgb2YgYWxs
LCB5b3VyIHBhdGNoIGhhcyBiZWVuIGdhcmJsZWQgd2hlbiBtYWlsaW5nLgo+Cj4KPiBbLi4gTG90
cyBvZiBhcmNoL21pcHMgY29kZSBkZWxldGVkIC4uLl0KPgo+IE5vIGNvbXBsYWludHMgdXB0byB0
aGlzIHBvaW50LiAgQnV0Ogo+Cj4gPiBkaWZmIC11ck4gYi9kcml2ZXJzL29wcm9maWxlL2NwdV9i
dWZmZXIuYyBhL2RyaXZlcnMvb3Byb2ZpbGUvY3B1X2J1ZmZlci5jCj4gPiAtLS0gYi9kcml2ZXJz
L29wcm9maWxlL2NwdV9idWZmZXIuYyAgIDIwMDctMDctMjQgMTM6MDA6NTQuMDAwMDAwMDAwICsw
ODAwCj4gPiArKysgYS9kcml2ZXJzL29wcm9maWxlL2NwdV9idWZmZXIuYyAgIDIwMDctMDctMTkg
MDg6MjI6MTUuMDAwMDAwMDAwICswODAwCj4gPiBAQCAtMTQ4LDYgKzE0OCwxMCBAQAo+ID4gICAg
ICAgICAgICB1bnNpZ25lZCBsb25nIHBjLCB1bnNpZ25lZCBsb25nIGV2ZW50KQo+ID4gewo+ID4g
ICAgICAgc3RydWN0IG9wX3NhbXBsZSAqIGVudHJ5ID0gJmNwdV9idWYtPmJ1ZmZlcltjcHVfYnVm
LT5oZWFkX3Bvc107Cj4gPiArCj4gPiArICAgICBpZighZW50cnkpCj4gPiArICAgICAgICAgICAg
IHJldHVybjsKPiA+ICsKPiA+ICAgICAgIGVudHJ5LT5laXAgPSBwYzsKPiA+ICAgICAgIGVudHJ5
LT5ldmVudCA9IGV2ZW50Owo+ID4gICAgICAgaW5jcmVtZW50X2hlYWQoY3B1X2J1Zik7Cj4KPiBX
aHkgZG8geW91IG5lZWQgdGhpcyBjaGFuZ2U/ICBJdCBhbG1vc3QgbG9va3MgYXMgaWYgeW91J3Jl
IHBhcGVyaW5nIG92ZXIKPiBhIGJ1ZyB3aGVyZSBhZGRfc2FtcGxlIHNob3VsZCBub3QgYmUgY2Fs
bGVkIGF0IGFsbC4KPgo+ICAgUmFsZgo+CgoKLS0gCuS4uuWkqeWcsOeri+W/gwrkuLrnlJ/msJHn
q4vlkb0K5Li65b6A5Zyj57un57ud5a2mCuS4uuS4h+S4luW8gOWkquW5swo=

On Wed, Jul 25, 2007 at 07:41:21AM +0400, Dajie Tan wrote:

> >Why do you need this change?  It almost looks as if you're papering over
> >a bug where add_sample should not be called at all.
> 
> Yeah,this change is to enhance the robust of oprofile. When using
> performace counter manually(writting control register in a module, no
> need to use the oprofile),I usually make kernel panic if I do not
> initialize the oprofile and enable the overflow interrupt carelessly.
> So, this change can avoid this panic. :D

This panic is good and should stay. It shows that you've made a mistake.

john

2007/7/25, John Levon <levon@...>:
> On Wed, Jul 25, 2007 at 07:41:21AM +0400, Dajie Tan wrote:
>
> > >Why do you need this change?  It almost looks as if you're papering over
> > >a bug where add_sample should not be called at all.
> >
> > Yeah,this change is to enhance the robust of oprofile. When using
> > performace counter manually(writting control register in a module, no
> > need to use the oprofile),I usually make kernel panic if I do not
> > initialize the oprofile and enable the overflow interrupt carelessly.
> > So, this change can avoid this panic. :D
>
> This panic is good and should stay. It shows that you've made a mistake.
>
> john
>

This panic is caused by accessing a null pointer.Do you think that
accessing a null
pointer is allowed in a robust system ?

Dajie Tan 写道:
>> > Yeah,this change is to enhance the robust of oprofile. When using
>> > performace counter manually(writting control register in a module, no
>> > need to use the oprofile),I usually make kernel panic if I do not
>> > initialize the oprofile and enable the overflow interrupt carelessly.
>> > So, this change can avoid this panic. :D
>>
>> This panic is good and should stay. It shows that you've made a mistake.
>>
>> john
>>
>
> This panic is caused by accessing a null pointer.Do you think that
> accessing a null
> pointer is allowed in a robust system ?
I think you don't really understand Ralf and John's responses. The 
problem should not be workarounded in that position.
Your code should ensure that overflow interrupt will not be enabled when 
oprofile is not enabled, or something like this. If everyone choose to 
'fix' problems via patching generic code, it would be a maintainence 
headache. Why other platform s won't have your problem?

It is not to say that the panic is ok, but it is your problem if others 
are not suffering the same panic, and should be fixed in your code.
>
>
>
>

-- 
------------------------------------------------
Fuxin Zhang

Vice Researcher
Microprocessor Research Center, Institute of Computing Technolgy
Chiese Academy of China
Beijing, 100080
Email: fxzhang@...
http://www.ict.ac.cn

------------------------------------------------

On Thu, Jul 26, 2007 at 05:14:14AM +0400, Dajie Tan wrote:

> >> Yeah,this change is to enhance the robust of oprofile. When using
> >> performace counter manually(writting control register in a module, no
> >> need to use the oprofile),I usually make kernel panic if I do not
> >> initialize the oprofile and enable the overflow interrupt carelessly.
> >> So, this change can avoid this panic. :D
> >
> >This panic is good and should stay. It shows that you've made a mistake.
> >
> >john
> >
> 
> This panic is caused by accessing a null pointer.Do you think that
> accessing a null
> pointer is allowed in a robust system ?

Of course it isn't.  From the perspective of us kernel maintainers patches
that add such checks are a red flag which raise concerns about the
correctness of the caller of the function.  So if a patch like this is
submitted the first thing that is likely to happen is that we will ask
why the check is needed.  It does not mean such a patch is fundamentally
a no-no but the code will be looked at a little harder.

  Ralf