Re: GFS, what's remaining

[David Teigland <teigland@redhat.com>]

On Fri, Sep 02, 2005 at 10:28:21PM -0700, Greg KH wrote:
> On Fri, Sep 02, 2005 at 05:44:03PM +0800, David Teigland wrote:
> > On Thu, Sep 01, 2005 at 01:35:23PM +0200, Arjan van de Ven wrote:
> > 
> > > +	gfs2_assert(gl->gl_sbd, atomic_read(&gl->gl_count) > 0,);
> > 
> > > what is gfs2_assert() about anyway? please just use BUG_ON directly
> > > everywhere
> > 
> > When a machine has many gfs file systems mounted at once it can be useful
> > to know which one failed.  Does the following look ok?
> > 
> > #define gfs2_assert(sdp, assertion)                                       \
> > do {                                                                      \
> >         if (unlikely(!(assertion))) {                                     \
> >                 printk(KERN_ERR                                           \
> >                         "GFS2: fsid=%s: fatal: assertion \"%s\" failed\n" \
> >                         "GFS2: fsid=%s:   function = %s\n"                \
> >                         "GFS2: fsid=%s:   file = %s, line = %u\n"         \
> >                         "GFS2: fsid=%s:   time = %lu\n",                  \
> >                         sdp->sd_fsname, # assertion,                      \
> >                         sdp->sd_fsname,  __FUNCTION__,                    \
> >                         sdp->sd_fsname, __FILE__, __LINE__,               \
> >                         sdp->sd_fsname, get_seconds());                   \
> >                 BUG();                                                    \
> 
> You will already get the __FUNCTION__ (and hence the __FILE__ info)
> directly from the BUG() dump, as well as the time from the syslog
> message (turn on the printk timestamps if you want a more fine grain
> timestamp), so the majority of this macro is redundant with the BUG()
> macro...

Joern already suggested moving this out of line and into a function (as it
was before) to avoid repeating string constants.  In that case the
function, file and line from BUG aren't useful.  We now have this, does it
look ok?

void gfs2_assert_i(struct gfs2_sbd *sdp, char *assertion, const char *function,
                   char *file, unsigned int line)
{
        panic("GFS2: fsid=%s: fatal: assertion \"%s\" failed\n"
              "GFS2: fsid=%s:   function = %s, file = %s, line = %u\n",
              sdp->sd_fsname, assertion,
              sdp->sd_fsname, function, file, line);
}

#define gfs2_assert(sdp, assertion) \
do { \
        if (unlikely(!(assertion))) { \
                gfs2_assert_i((sdp), #assertion, \
                              __FUNCTION__, __FILE__, __LINE__); \
        } \
} while (0)