| Message ID | 1332528209.23830.4.camel@ultramagnus.opencreations.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
On Fri, Mar 23, 2012 at 11:43:28AM -0700, Dan Williams wrote: > On Thu, 2012-03-22 at 07:39 -0700, Greg Kroah-Hartman wrote: > > And note, I hate pr_err(), what's wrong with printk() in this instance? > > So how about an end run around the conversion to pr_err() and just make > these proper WARN()s since we end up calling dump_stack in both cases? > > Something like: > > diff --git a/lib/kobject.c b/lib/kobject.c > index e5f86c0..1bd0893 100644 > --- a/lib/kobject.c > +++ b/lib/kobject.c > @@ -192,15 +192,14 @@ static int kobject_add_internal(struct kobject *kobj) > > /* be noisy on error issues */ > if (error == -EEXIST) > - pr_err("%s failed for %s with " > - "-EEXIST, don't try to register things with " > - "the same name in the same directory.\n", > - __func__, kobject_name(kobj)); > + WARN(1, "%s failed for %s with " > + "-EEXIST, don't try to register things with " > + "the same name in the same directory.\n", > + __func__, kobject_name(kobj)); > else > - pr_err("%s failed for %s (error: %d parent: %s)\n", > - __func__, kobject_name(kobj), error, > - parent ? kobject_name(parent) : "'none'"); > - dump_stack(); > + WARN(1, "%s failed for %s (error: %d parent: %s)\n", > + __func__, kobject_name(kobj), error, > + parent ? kobject_name(parent) : "'none'"); I'm missing why this isn't the exact same thing that is currently happening... confused, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Mar 23, 2012 at 1:54 PM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Fri, Mar 23, 2012 at 11:43:28AM -0700, Dan Williams wrote: >> On Thu, 2012-03-22 at 07:39 -0700, Greg Kroah-Hartman wrote: >> > And note, I hate pr_err(), what's wrong with printk() in this instance? >> >> So how about an end run around the conversion to pr_err() and just make >> these proper WARN()s since we end up calling dump_stack in both cases? >> >> Something like: >> >> diff --git a/lib/kobject.c b/lib/kobject.c >> index e5f86c0..1bd0893 100644 >> --- a/lib/kobject.c >> +++ b/lib/kobject.c >> @@ -192,15 +192,14 @@ static int kobject_add_internal(struct kobject *kobj) >> >> /* be noisy on error issues */ >> if (error == -EEXIST) >> - pr_err("%s failed for %s with " >> - "-EEXIST, don't try to register things with " >> - "the same name in the same directory.\n", >> - __func__, kobject_name(kobj)); >> + WARN(1, "%s failed for %s with " >> + "-EEXIST, don't try to register things with " >> + "the same name in the same directory.\n", >> + __func__, kobject_name(kobj)); >> else >> - pr_err("%s failed for %s (error: %d parent: %s)\n", >> - __func__, kobject_name(kobj), error, >> - parent ? kobject_name(parent) : "'none'"); >> - dump_stack(); >> + WARN(1, "%s failed for %s (error: %d parent: %s)\n", >> + __func__, kobject_name(kobj), error, >> + parent ? kobject_name(parent) : "'none'"); > > I'm missing why this isn't the exact same thing that is currently > happening... > > confused, WARN does wrap the printk and backtrace in "------------[ cut here ]------------\n" and "---[ end trace %016llx ]---\n" so automated tools can pick it up, maybe that's useful in a core routine like this? It's still modifying the else case to add the faulting child and parent kobject names, and does so without setting a pr_ conversion precedent in sysfs. -- Dan -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/lib/kobject.c b/lib/kobject.c index e5f86c0..1bd0893 100644 --- a/lib/kobject.c +++ b/lib/kobject.c @@ -192,15 +192,14 @@ static int kobject_add_internal(struct kobject *kobj) /* be noisy on error issues */ if (error == -EEXIST) - pr_err("%s failed for %s with " - "-EEXIST, don't try to register things with " - "the same name in the same directory.\n", - __func__, kobject_name(kobj)); + WARN(1, "%s failed for %s with " + "-EEXIST, don't try to register things with " + "the same name in the same directory.\n", + __func__, kobject_name(kobj)); else - pr_err("%s failed for %s (error: %d parent: %s)\n", - __func__, kobject_name(kobj), error, - parent ? kobject_name(parent) : "'none'"); - dump_stack(); + WARN(1, "%s failed for %s (error: %d parent: %s)\n", + __func__, kobject_name(kobj), error, + parent ? kobject_name(parent) : "'none'"); } else kobj->state_in_sysfs = 1;
On Thu, 2012-03-22 at 07:39 -0700, Greg Kroah-Hartman wrote: > And note, I hate pr_err(), what's wrong with printk() in this instance? So how about an end run around the conversion to pr_err() and just make these proper WARN()s since we end up calling dump_stack in both cases? Something like: -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html