Hi.

On 05.12.2019 00:00, Ravi Pokala wrote:
> -----Original Message-----
> From: <[email protected]> on behalf of Alexander Motin 
> <[email protected]>
> Date: 2019-12-04, Wednesday at 20:52
> To: <[email protected]>, <[email protected]>, 
> <[email protected]>
> Subject: svn commit: r355412 - head/sys/geom
> 
>     Author: mav
>     Date: Thu Dec  5 04:52:19 2019
>     New Revision: 355412
>     URL: https://svnweb.freebsd.org/changeset/base/355412
>     
>     Log:
>       Wrap g_trace() into a macro to avoid unneeded calls.
>       
>       In most cases with debug disabled this function does nothing, but 
> argument
>       passing and the call still cost measurable time due to cache misses, 
> etc.
> 
> Hi Alexander,
> 
> I'm having trouble understanding this change, on a few levels.
> 
> - Why did you add parentheses around the function declaration and definition?

To make pre-processor not replace them also.

> - How can that function with that name co-exist with a macro of the same name?

The macro is handled first by pre-processor, compiler goes second on the
result.

> - Isn't the "g_debugflags" test in geom_dump.c:g_trace() now redundant?
> - Why not simply convert geom_dump.c:g_trace() into a 'static inline' in 
> geom.h, and not have to bother with the macro at all?

One answer to both questions: compatibility with already built modules
require the function to be present and behave same as before, while
newly built code should be more efficient without changes.

> What am I missing?
> 
> Thanks,
> 
> Ravi (rpokala@)
> 
>       MFC after:      2 weeks
>       Sponsored by:   iXsystems, Inc.
>     
>     Modified:
>       head/sys/geom/geom.h
>       head/sys/geom/geom_dump.c
>     
>     Modified: head/sys/geom/geom.h
>     
> ==============================================================================
>     --- head/sys/geom/geom.h  Thu Dec  5 04:18:22 2019        (r355411)
>     +++ head/sys/geom/geom.h  Thu Dec  5 04:52:19 2019        (r355412)
>     @@ -255,11 +255,15 @@ void g_dev_physpath_changed(void);
>      struct g_provider *g_dev_getprovider(struct cdev *dev);
>      
>      /* geom_dump.c */
>     -void g_trace(int level, const char *, ...);
>     +void (g_trace)(int level, const char *, ...) __printflike(2, 3);
>      #        define G_T_TOPOLOGY     1
>      #        define G_T_BIO          2
>      #        define G_T_ACCESS       4
>     -
>     +extern int g_debugflags;
>     +#define  g_trace(level, fmt, ...) do {                           \
>     + if (__predict_false(g_debugflags & (level)))            \
>     +         (g_trace)(level, fmt, ## __VA_ARGS__);          \
>     +} while (0)
>      
>      /* geom_event.c */
>      typedef void g_event_t(void *, int flag);
>     
>     Modified: head/sys/geom/geom_dump.c
>     
> ==============================================================================
>     --- head/sys/geom/geom_dump.c     Thu Dec  5 04:18:22 2019        
> (r355411)
>     +++ head/sys/geom/geom_dump.c     Thu Dec  5 04:52:19 2019        
> (r355412)
>     @@ -319,7 +319,7 @@ g_confxml(void *p, int flag)
>      }
>      
>      void
>     -g_trace(int level, const char *fmt, ...)
>     +(g_trace)(int level, const char *fmt, ...)
>      {
>       va_list ap;

-- 
Alexander Motin
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "[email protected]"

Reply via email to