Whamcloud - gitweb
LU-12861 libcfs: Cleanup libcfs_debug_msg use of snprintf
[fs/lustre-release.git] / libcfs / libcfs / tracefile.c
index 555a5bb..7aba3ac 100644 (file)
@@ -58,6 +58,7 @@ static DEFINE_MUTEX(cfs_trace_thread_mutex);
 static int thread_running = 0;
 
 static atomic_t cfs_tage_allocated = ATOMIC_INIT(0);
+static DECLARE_RWSEM(cfs_tracefile_sem);
 
 static void put_pages_on_tcd_daemon_list(struct page_collection *pc,
                                        struct cfs_trace_cpu_data *tcd);
@@ -243,68 +244,71 @@ static struct cfs_trace_page *cfs_trace_get_tage(struct cfs_trace_cpu_data *tcd,
 }
 
 int libcfs_debug_msg(struct libcfs_debug_msg_data *msgdata,
-                     const char *format, ...)
+                    const char *format, ...)
 {
-        struct cfs_trace_cpu_data *tcd = NULL;
-        struct ptldebug_header     header = {0};
-        struct cfs_trace_page     *tage;
-        /* string_buf is used only if tcd != NULL, and is always set then */
-        char                      *string_buf = NULL;
-        char                      *debug_buf;
-        int                        known_size;
-        int                        needed = 85; /* average message length */
-        int                        max_nob;
-        va_list                    ap;
-        int                        i;
-        int                        remain;
-        int                        mask = msgdata->msg_mask;
-        char                      *file = (char *)msgdata->msg_file;
+       struct cfs_trace_cpu_data *tcd = NULL;
+       struct ptldebug_header header = {0};
+       struct cfs_trace_page *tage;
+       /* string_buf is used only if tcd != NULL, and is always set then */
+       char *string_buf = NULL;
+       char *debug_buf;
+       int known_size;
+       int needed = 85; /* seeded with average message length */
+       int max_nob;
+       va_list ap;
+       int retry;
+       int mask = msgdata->msg_mask;
+       char *file = (char *)msgdata->msg_file;
        struct cfs_debug_limit_state *cdls = msgdata->msg_cdls;
 
-        if (strchr(file, '/'))
-                file = strrchr(file, '/') + 1;
+       if (strchr(file, '/'))
+               file = strrchr(file, '/') + 1;
 
-        tcd = cfs_trace_get_tcd();
+       tcd = cfs_trace_get_tcd();
 
-        /* cfs_trace_get_tcd() grabs a lock, which disables preemption and
-         * pins us to a particular CPU.  This avoids an smp_processor_id()
-         * warning on Linux when debugging is enabled. */
-        cfs_set_ptldebug_header(&header, msgdata, CDEBUG_STACK());
+       /* cfs_trace_get_tcd() grabs a lock, which disables preemption and
+        * pins us to a particular CPU.  This avoids an smp_processor_id()
+        * warning on Linux when debugging is enabled.
+        */
+       cfs_set_ptldebug_header(&header, msgdata, CDEBUG_STACK());
 
-        if (tcd == NULL)                /* arch may not log in IRQ context */
-                goto console;
+       if (!tcd)                /* arch may not log in IRQ context */
+               goto console;
 
-        if (tcd->tcd_cur_pages == 0)
-                header.ph_flags |= PH_FLAG_FIRST_RECORD;
+       if (tcd->tcd_cur_pages == 0)
+               header.ph_flags |= PH_FLAG_FIRST_RECORD;
 
-        if (tcd->tcd_shutting_down) {
-                cfs_trace_put_tcd(tcd);
-                tcd = NULL;
-                goto console;
-        }
+       if (tcd->tcd_shutting_down) {
+               cfs_trace_put_tcd(tcd);
+               tcd = NULL;
+               goto console;
+       }
 
        known_size = strlen(file) + 1;
-        if (msgdata->msg_fn)
-                known_size += strlen(msgdata->msg_fn) + 1;
+       if (msgdata->msg_fn)
+               known_size += strlen(msgdata->msg_fn) + 1;
 
-        if (libcfs_debug_binary)
-                known_size += sizeof(header);
+       if (libcfs_debug_binary)
+               known_size += sizeof(header);
 
-        /*/
-         * '2' used because vsnprintf return real size required for output
-         * _without_ terminating NULL.
-         * if needed is to small for this format.
-         */
-        for (i = 0; i < 2; i++) {
-                tage = cfs_trace_get_tage(tcd, needed + known_size + 1);
-                if (tage == NULL) {
+       /*
+        * May perform an additional pass to update 'needed' and increase
+        * tage buffer size to match vsnprintf reported size required
+        * On the second pass (retry=1) use vscnprintf [which returns
+        * number of bytes written not including the terminating nul]
+        * to clarify `needed` is used as number of bytes written
+        * for the remainder of this function
+        */
+       for (retry = 0; retry < 2; retry++) {
+               tage = cfs_trace_get_tage(tcd, needed + known_size + 1);
+               if (!tage) {
                        if (needed + known_size > PAGE_SIZE)
-                                mask |= D_ERROR;
+                               mask |= D_ERROR;
 
-                        cfs_trace_put_tcd(tcd);
-                        tcd = NULL;
-                        goto console;
-                }
+                       cfs_trace_put_tcd(tcd);
+                       tcd = NULL;
+                       goto console;
+               }
 
                string_buf = (char *)page_address(tage->page) +
                             tage->used + known_size;
@@ -319,20 +323,18 @@ int libcfs_debug_msg(struct libcfs_debug_msg_data *msgdata,
                        goto console;
                }
 
-                needed = 0;
-               remain = max_nob - needed;
-               if (remain < 0)
-                       remain = 0;
-
                va_start(ap, format);
-               needed += vsnprintf(string_buf + needed, remain,
-                                   format, ap);
+               if (retry)
+                       needed = vscnprintf(string_buf, max_nob, format, ap);
+               else
+                       needed = vsnprintf(string_buf, max_nob, format, ap);
                va_end(ap);
 
-                if (needed < max_nob) /* well. printing ok.. */
-                        break;
-        }
+               if (needed < max_nob) /* well. printing ok.. */
+                       break;
+       }
 
+       /* `needed` is actual bytes written to string_buf */
        if (*(string_buf + needed - 1) != '\n') {
                pr_info("Lustre: format at %s:%d:%s doesn't end in newline\n",
                        file, msgdata->msg_line, msgdata->msg_fn);
@@ -343,37 +345,37 @@ int libcfs_debug_msg(struct libcfs_debug_msg_data *msgdata,
                                file, msgdata->msg_line, msgdata->msg_fn);
        }
 
-        header.ph_len = known_size + needed;
+       header.ph_len = known_size + needed;
        debug_buf = (char *)page_address(tage->page) + tage->used;
 
-        if (libcfs_debug_binary) {
-                memcpy(debug_buf, &header, sizeof(header));
-                tage->used += sizeof(header);
-                debug_buf += sizeof(header);
-        }
+       if (libcfs_debug_binary) {
+               memcpy(debug_buf, &header, sizeof(header));
+               tage->used += sizeof(header);
+               debug_buf += sizeof(header);
+       }
 
-        strcpy(debug_buf, file);
-        tage->used += strlen(file) + 1;
-        debug_buf += strlen(file) + 1;
+       strlcpy(debug_buf, file, PAGE_SIZE - tage->used);
+       tage->used += strlen(file) + 1;
+       debug_buf += strlen(file) + 1;
 
-        if (msgdata->msg_fn) {
-                strcpy(debug_buf, msgdata->msg_fn);
-                tage->used += strlen(msgdata->msg_fn) + 1;
-                debug_buf += strlen(msgdata->msg_fn) + 1;
-        }
+       if (msgdata->msg_fn) {
+               strlcpy(debug_buf, msgdata->msg_fn, PAGE_SIZE - tage->used);
+               tage->used += strlen(msgdata->msg_fn) + 1;
+               debug_buf += strlen(msgdata->msg_fn) + 1;
+       }
 
-        __LASSERT(debug_buf == string_buf);
+       __LASSERT(debug_buf == string_buf);
 
-        tage->used += needed;
+       tage->used += needed;
        __LASSERT(tage->used <= PAGE_SIZE);
 
 console:
-        if ((mask & libcfs_printk) == 0) {
-                /* no console output requested */
-                if (tcd != NULL)
-                        cfs_trace_put_tcd(tcd);
-                return 1;
-        }
+       if ((mask & libcfs_printk) == 0) {
+               /* no console output requested */
+               if (tcd != NULL)
+                       cfs_trace_put_tcd(tcd);
+               return 1;
+       }
 
        if (cdls != NULL) {
                if (libcfs_console_ratelimit &&
@@ -404,45 +406,41 @@ console:
                cdls->cdls_next = (jiffies + cdls->cdls_delay) | 1;
        }
 
-        if (tcd != NULL) {
-                cfs_print_to_console(&header, mask, string_buf, needed, file,
-                                     msgdata->msg_fn);
-                cfs_trace_put_tcd(tcd);
-        } else {
-                string_buf = cfs_trace_get_console_buffer();
-
-                needed = 0;
-               remain = CFS_TRACE_CONSOLE_BUFFER_SIZE - needed;
-               if (remain > 0) {
-                       va_start(ap, format);
-                       needed += vsnprintf(string_buf+needed, remain,
-                                           format, ap);
-                       va_end(ap);
-                }
+       if (tcd) {
+               cfs_print_to_console(&header, mask, string_buf, needed, file,
+                                    msgdata->msg_fn);
+               cfs_trace_put_tcd(tcd);
+       } else {
+               string_buf = cfs_trace_get_console_buffer();
+
+               va_start(ap, format);
+               needed = vscnprintf(string_buf, CFS_TRACE_CONSOLE_BUFFER_SIZE,
+                                   format, ap);
+               va_end(ap);
 
-                cfs_print_to_console(&header, mask,
-                                     string_buf, needed, file, msgdata->msg_fn);
+               cfs_print_to_console(&header, mask,
+                                    string_buf, needed, file, msgdata->msg_fn);
 
                put_cpu();
-        }
+       }
 
-        if (cdls != NULL && cdls->cdls_count != 0) {
-                string_buf = cfs_trace_get_console_buffer();
+       if (cdls != NULL && cdls->cdls_count != 0) {
+               string_buf = cfs_trace_get_console_buffer();
 
-                needed = snprintf(string_buf, CFS_TRACE_CONSOLE_BUFFER_SIZE,
-                                  "Skipped %d previous similar message%s\n",
-                                  cdls->cdls_count,
-                                  (cdls->cdls_count > 1) ? "s" : "");
+               needed = scnprintf(string_buf, CFS_TRACE_CONSOLE_BUFFER_SIZE,
+                                  "Skipped %d previous similar message%s\n",
+                                  cdls->cdls_count,
+                                  (cdls->cdls_count > 1) ? "s" : "");
 
                /* Do not allow print this to TTY */
                cfs_print_to_console(&header, mask & ~D_TTY, string_buf,
                                     needed, file, msgdata->msg_fn);
 
                put_cpu();
-                cdls->cdls_count = 0;
-        }
+               cdls->cdls_count = 0;
+       }
 
-        return 0;
+       return 0;
 }
 EXPORT_SYMBOL(libcfs_debug_msg);
 
@@ -642,7 +640,7 @@ int cfs_tracefile_dump_all_pages(char *filename)
        char                    *buf;
        int rc;
 
-       cfs_tracefile_write_lock();
+       down_write(&cfs_tracefile_sem);
 
        filp = filp_open(filename, O_CREAT|O_EXCL|O_WRONLY|O_LARGEFILE, 0600);
        if (IS_ERR(filp)) {
@@ -686,7 +684,7 @@ int cfs_tracefile_dump_all_pages(char *filename)
 close:
        filp_close(filp, NULL);
 out:
-       cfs_tracefile_write_unlock();
+       up_write(&cfs_tracefile_sem);
        return rc;
 }
 
@@ -799,12 +797,12 @@ int cfs_trace_daemon_command(char *str)
 {
         int       rc = 0;
 
-        cfs_tracefile_write_lock();
+       down_write(&cfs_tracefile_sem);
 
         if (strcmp(str, "stop") == 0) {
-                cfs_tracefile_write_unlock();
+               up_write(&cfs_tracefile_sem);
                 cfs_trace_stop_thread();
-                cfs_tracefile_write_lock();
+               down_write(&cfs_tracefile_sem);
                 memset(cfs_tracefile, 0, sizeof(cfs_tracefile));
 
        } else if (strncmp(str, "size=", 5) == 0) {
@@ -830,7 +828,7 @@ int cfs_trace_daemon_command(char *str)
                cfs_trace_start_thread();
         }
 
-        cfs_tracefile_write_unlock();
+       up_write(&cfs_tracefile_sem);
         return rc;
 }
 
@@ -875,12 +873,12 @@ int cfs_trace_set_debug_mb(int mb)
        mb /= num_possible_cpus();
        pages = mb << (20 - PAGE_SHIFT);
 
-       cfs_tracefile_write_lock();
+       down_write(&cfs_tracefile_sem);
 
        cfs_tcd_for_each(tcd, i, j)
                tcd->tcd_max_pages = (pages * tcd->tcd_pages_factor) / 100;
 
-       cfs_tracefile_write_unlock();
+       up_write(&cfs_tracefile_sem);
 
        return 0;
 }
@@ -892,12 +890,12 @@ int cfs_trace_get_debug_mb(void)
         struct cfs_trace_cpu_data *tcd;
         int total_pages = 0;
 
-        cfs_tracefile_read_lock();
+       down_read(&cfs_tracefile_sem);
 
         cfs_tcd_for_each(tcd, i, j)
                 total_pages += tcd->tcd_max_pages;
 
-        cfs_tracefile_read_unlock();
+       up_read(&cfs_tracefile_sem);
 
        return (total_pages >> (20 - PAGE_SHIFT)) + 1;
 }
@@ -927,7 +925,7 @@ static int tracefiled(void *arg)
                         goto end_loop;
 
                 filp = NULL;
-                cfs_tracefile_read_lock();
+               down_read(&cfs_tracefile_sem);
                 if (cfs_tracefile[0] != 0) {
                        filp = filp_open(cfs_tracefile,
                                         O_CREAT | O_RDWR | O_LARGEFILE,
@@ -939,7 +937,7 @@ static int tracefiled(void *arg)
                                        cfs_tracefile, rc);
                        }
                }
-                cfs_tracefile_read_unlock();
+               up_read(&cfs_tracefile_sem);
                 if (filp == NULL) {
                         put_pages_on_daemon_list(&pc);
                        __LASSERT(list_empty(&pc.pc_pages));