Project

General

Profile

Feature #1910

Implement a generic dev_err

Added by Alexey Zaytsev over 8 years ago. Updated almost 8 years ago.

Status:
Resolved
Priority:
Normal
Category:
-
Start date:
2011-12-20
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:

Description

A lot of drivers implement custom cmn_err wrappers that prepend the messages with "device:instance #". Implement a generic dev_err function for the task.

For examples, see

afe_error in common/io/afe/afe.c
e1000g_log in common/io/e1000g/e1000g_debug.c

And no doubt, many more.

Also, implement a dev_debug macro, that is an alias do dev_err in DEBUG kernels, and a nop otherwise.


Related issues

Related to illumos gate - Feature #2922: dev_err() needs manpageClosed2012-06-24

Actions

History

#1

Updated by Alexey Zaytsev over 8 years ago

Now, I'd like to get people's input on this. Currently, I've got it implemented in my virtio drivers in a header file this way:

#if defined (__GNUC__)                                                            
#define dev_err(dip, ce, fmt, ...)      \                                         
        cmn_err(ce, "%s%d: " fmt, ddi_driver_name(dip), \                         
                        ddi_get_instance(dip), ## __VA_ARGS__);                   
#elif defined (__SUNPRO_C)                                                        
        /*                                                                        
         * The Sun Studio does not support the ##__VAR_ARGS__ extension,          
         * so we fall back to the function with a constant size buffer.           
         */                                                                       

        /* Get rid of this when/if we drop SS support */                          

/* PRINTFLIKE3 */                                                                 
static void                                                                       
dev_err(dev_info_t *dip, int ce, char *fmt, ...)                                  
{                                                                                 
        va_list ap;                                                               
        char buf[LOG_MSGSIZE];                                                    

        va_start(ap, fmt);                                                        
        (void) vsnprintf(buf, sizeof (buf), fmt, ap);                             
        va_end(ap);                                                               

        cmn_err(ce, "%s%d: %s", ddi_driver_name(dip),                             
                        ddi_get_instance(dip), buf);                              
}                                                                                 
#else                                                                             
#error "Unknown compiler"                                                         
#endif                                                                            

#ifdef DEBUG                                                                      
#define dev_debug(dip, fmt, arg...) \                                             
        dev_err(dip, fmt, ##arg)                                                  
#else                                                                             
#define dev_debug(dip, fmt, arg...)                                               
#endif 

It boils down to a 2-line macro in gcc, but for ss we've got to use a function, because ss does not support the ## extenstion.

So, what's your opinion?
- Should I keep two implementations, or just use the function in both cases?
- In any case, should I de-static the function and put it in a C file? Which one?
- In which header file should I put the rest?

#2

Updated by Garrett D'Amore over 8 years ago

I would prefer to avoid the macro version altogether. There are lots of reasons to deplore macros, and apart from side effects and namespace pollution, the one in this case is that I'd really not like to see the same object code scattered around. The functional form is more efficient, especially if this function is rarely called -- which we hope will be the case. (Polluting logs is evil. :-)

#3

Updated by Rich Lowe over 8 years ago

I asked for the macro to avoid the bogus static buffer. I'd be just as fine with use of a vprintf, vcmn_err, or whatever exists to facilitate losing the large buffer on the stack.

#4

Updated by Alexey Zaytsev over 8 years ago

Thank you for the comments.

Rich: I'm not sure how you'd implement a buffer-less version with vcmn_err. Could you please elaborate? Unless I'm missing something big-time, there is no way to do this.

#5

Updated by Rich Lowe about 8 years ago

Missed the question: You can't, because of the whacky \n behaviour.

If I were doing it, I'd probably have implemented a dcmn_err() on top of cprintf, like the other cmn_err variants. See, for eg, the z* variants, and vzcmn_err.

-- Rich

#6

Updated by Alexey Zaytsev almost 8 years ago

  • Status changed from New to Resolved
  • % Done changed from 50 to 100

Also available in: Atom PDF