xfs
[Top] [All Lists]

[PATCH] avoid memory allocations in xfs_fs_vcmn_err

To: xfs@xxxxxxxxxxx
Subject: [PATCH] avoid memory allocations in xfs_fs_vcmn_err
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Wed, 17 Dec 2008 12:27:36 -0500
Cc: Alexander Beregalov <a.beregalov@xxxxxxxxx>
User-agent: Mutt/1.5.18 (2008-05-17)
xfs_fs_vcmn_err can be called under a spinlock, but does a sleeping memory
allocation to create buffer for it's internal sprintf.  Fortunately it's
the only caller of icmn_err, so we can merge the two and have one single
static buffer and spinlock protecting it.  While we're at it make sure
we proper __attribute__ format annotations so that the compiler can detect
mismatched format strings.


Reported-by: Alexander Beregalov <a.beregalov@xxxxxxxxx>
Signed-off-by: Christoph Hellwig <hch@xxxxxx>

Index: xfs/fs/xfs/support/debug.c
===================================================================
--- xfs.orig/fs/xfs/support/debug.c     2008-12-17 14:59:49.000000000 +0100
+++ xfs/fs/xfs/support/debug.c  2008-12-17 15:10:13.000000000 +0100
@@ -18,6 +18,13 @@
 #include <xfs.h>
 #include "debug.h"
 
+/* xfs_mount.h drags a lot of crap in, sorry.. */
+#include "xfs_sb.h"
+#include "xfs_inum.h"
+#include "xfs_ag.h"
+#include "xfs_dmapi.h"
+#include "xfs_mount.h"
+
 static char            message[1024];  /* keep it off the stack */
 static DEFINE_SPINLOCK(xfs_err_lock);
 
@@ -55,22 +62,42 @@ cmn_err(register int level, char *fmt, .
 }
 
 void
-icmn_err(register int level, char *fmt, va_list ap)
+xfs_fs_vcmn_err(
+       int                     level,
+       struct xfs_mount        *mp,
+       char                    *fmt,
+       va_list                 ap)
 {
-       ulong   flags;
-       int     len;
+       unsigned long           flags;
+       int                     len = 0;
 
        level &= XFS_ERR_MASK;
-       if(level > XFS_MAX_ERR_LEVEL)
+       if (level > XFS_MAX_ERR_LEVEL)
                level = XFS_MAX_ERR_LEVEL;
+
        spin_lock_irqsave(&xfs_err_lock,flags);
-       len = vsnprintf(message, sizeof(message), fmt, ap);
+
+       if (mp) {
+               len = sprintf(message, "Filesystem \"%s\": ", mp->m_fsname);
+
+               /*
+                * Skip the printk if we can't print anything useful
+                * due to an over-long device name.
+                */
+               if (len >= sizeof(message))
+                       goto out;
+       }
+
+       len = vsnprintf(message + len, sizeof(message) - len, fmt, ap);
        if (len >= sizeof(message))
                len = sizeof(message) - 1;
        if (message[len-1] == '\n')
                message[len-1] = 0;
+
        printk("%s%s\n", err_level[level], message);
+ out:
        spin_unlock_irqrestore(&xfs_err_lock,flags);
+
        BUG_ON(level == CE_PANIC);
 }
 
Index: xfs/fs/xfs/support/debug.h
===================================================================
--- xfs.orig/fs/xfs/support/debug.h     2008-12-17 15:05:34.000000000 +0100
+++ xfs/fs/xfs/support/debug.h  2008-12-17 15:07:03.000000000 +0100
@@ -27,8 +27,6 @@
 #define CE_ALERT        1               /* alert        */
 #define CE_PANIC        0               /* panic        */
 
-extern void icmn_err(int, char *, va_list)
-       __attribute__ ((format (printf, 2, 0)));
 extern void cmn_err(int, char *, ...)
        __attribute__ ((format (printf, 2, 3)));
 extern void assfail(char *expr, char *f, int l);
Index: xfs/fs/xfs/xfs_error.c
===================================================================
--- xfs.orig/fs/xfs/xfs_error.c 2008-12-17 14:58:02.000000000 +0100
+++ xfs/fs/xfs/xfs_error.c      2008-12-17 15:12:03.000000000 +0100
@@ -153,21 +153,6 @@ xfs_errortag_clearall(xfs_mount_t *mp, i
 }
 #endif /* DEBUG */
 
-static void
-xfs_fs_vcmn_err(int level, xfs_mount_t *mp, char *fmt, va_list ap)
-{
-       if (mp != NULL) {
-               char    *newfmt;
-               int     len = 16 + mp->m_fsname_len + strlen(fmt);
-
-               newfmt = kmem_alloc(len, KM_SLEEP);
-               sprintf(newfmt, "Filesystem \"%s\": %s", mp->m_fsname, fmt);
-               icmn_err(level, newfmt, ap);
-               kmem_free(newfmt);
-       } else {
-               icmn_err(level, fmt, ap);
-       }
-}
 
 void
 xfs_fs_cmn_err(int level, xfs_mount_t *mp, char *fmt, ...)
Index: xfs/fs/xfs/xfs_error.h
===================================================================
--- xfs.orig/fs/xfs/xfs_error.h 2008-12-17 15:05:46.000000000 +0100
+++ xfs/fs/xfs/xfs_error.h      2008-12-17 15:12:01.000000000 +0100
@@ -159,11 +159,15 @@ extern int xfs_errortag_clearall(xfs_mou
 #define                XFS_PTAG_FSBLOCK_ZERO           0x00000080
 
 struct xfs_mount;
-/* PRINTFLIKE4 */
+
+extern void xfs_fs_vcmn_err(int level, struct xfs_mount *mp,
+               char *fmt, va_list ap)
+       __attribute__ ((format (printf, 3, 0)));
 extern void xfs_cmn_err(int panic_tag, int level, struct xfs_mount *mp,
-                       char *fmt, ...);
-/* PRINTFLIKE3 */
-extern void xfs_fs_cmn_err(int level, struct xfs_mount *mp, char *fmt, ...);
+                       char *fmt, ...)
+       __attribute__ ((format (printf, 4, 5)));
+extern void xfs_fs_cmn_err(int level, struct xfs_mount *mp, char *fmt, ...)
+       __attribute__ ((format (printf, 3, 4)));
 
 extern void xfs_hex_dump(void *p, int length);
 

<Prev in Thread] Current Thread [Next in Thread>