| To: | xfs-dev <xfs-dev@xxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx> |
|---|---|
| Subject: | [REVIEW] Move memory allocations for log tracing out of the critical path |
| From: | Lachlan McIlroy <lachlan@xxxxxxx> |
| Date: | Tue, 12 Aug 2008 15:27:40 +1000 |
| Reply-to: | lachlan@xxxxxxx |
| Sender: | xfs-bounce@xxxxxxxxxxx |
| User-agent: | Thunderbird 2.0.0.16 (X11/20080707) |
Memory allocations for log->l_grant_trace and iclog->ic_trace are done on demand when the first event is logged. In xlog_state_get_iclog_space() we call xlog_trace_iclog() under a spinlock and allocating memory here can cause us to sleep with a spinlock held and deadlock the system. For the log grant tracing we use KM_NOSLEEP but that means we can lose trace entries. Since there is no locking to serialize the log grant tracing we could race and have multiple allocations and leak memory. So move the allocations to where we initialize the log/iclog structures. Use KM_NOFS to avoid recursing into the filesystem and drop log->l_trace since it's not even used. Index: 2.6.x-clean/fs/xfs/xfs_log.c =================================================================== --- 2.6.x-clean.orig/fs/xfs/xfs_log.c +++ 2.6.x-clean/fs/xfs/xfs_log.c @@ -124,16 +124,27 @@ STATIC void xlog_verify_tail_lsn(xlog_t STATIC int xlog_iclogs_empty(xlog_t *log); #if defined(XFS_LOG_TRACE)
+
+#define XLOG_TRACE_LOGGRANT_SIZE 2048
+#define XLOG_TRACE_ICLOG_SIZE 256
+
+void
+xlog_trace_loggrant_alloc(xlog_t *log)
+{
+ log->l_grant_trace = ktrace_alloc(XLOG_TRACE_LOGGRANT_SIZE, KM_NOFS);
+}
+
+void
+xlog_trace_loggrant_dealloc(xlog_t *log)
+{
+ ktrace_free(log->l_grant_trace);
+}
+
void
xlog_trace_loggrant(xlog_t *log, xlog_ticket_t *tic, xfs_caddr_t string)
{
unsigned long cnts;- if (!log->l_grant_trace) {
- log->l_grant_trace = ktrace_alloc(2048, KM_NOSLEEP);
- if (!log->l_grant_trace)
- return;
- }
/* ticket counts are 1 byte each */
cnts = ((unsigned long)tic->t_ocnt) | ((unsigned long)tic->t_cnt) << 8;@@ -157,10 +168,20 @@ xlog_trace_loggrant(xlog_t *log, xlog_ti } void +xlog_trace_iclog_alloc(xlog_in_core_t *iclog) +{ + iclog->ic_trace = ktrace_alloc(XLOG_TRACE_ICLOG_SIZE, KM_NOFS); +} + +void +xlog_trace_iclog_dealloc(xlog_in_core_t *iclog) +{ + ktrace_free(iclog->ic_trace); +} + +void xlog_trace_iclog(xlog_in_core_t *iclog, uint state) { - if (!iclog->ic_trace) - iclog->ic_trace = ktrace_alloc(256, KM_NOFS); ktrace_enter(iclog->ic_trace, (void *)((unsigned long)state), (void *)((unsigned long)current_pid()), @@ -170,8 +191,15 @@ xlog_trace_iclog(xlog_in_core_t *iclog, (void *)NULL, (void *)NULL); } #else + +#define xlog_trace_loggrant_alloc(log) +#define xlog_trace_loggrant_dealloc(log) #define xlog_trace_loggrant(log,tic,string) + +#define xlog_trace_iclog_alloc(iclog) +#define xlog_trace_iclog_dealloc(iclog) #define xlog_trace_iclog(iclog,state) + #endif /* XFS_LOG_TRACE */
+ xlog_trace_loggrant_alloc(log);
/* log record size must be multiple of BBSIZE; see xlog_rec_header_t */
ASSERT((XFS_BUF_SIZE(bp) & BBMASK) == 0);@@ -1285,6 +1314,8 @@ xlog_alloc_log(xfs_mount_t *mp,
sv_init(&iclog->ic_force_wait, SV_DEFAULT, "iclog-force");
sv_init(&iclog->ic_write_wait, SV_DEFAULT, "iclog-write");+ xlog_trace_iclog_alloc(iclog);
+
iclogp = &iclog->ic_next;
}
*iclogp = log->l_iclog; /* complete ring */
@@ -1565,11 +1596,7 @@ xlog_dealloc_log(xlog_t *log)
sv_destroy(&iclog->ic_force_wait);
sv_destroy(&iclog->ic_write_wait);
xfs_buf_free(iclog->ic_bp);
-#ifdef XFS_LOG_TRACE
- if (iclog->ic_trace != NULL) {
- ktrace_free(iclog->ic_trace);
- }
-#endif
+ xlog_trace_iclog_dealloc(iclog);
next_iclog = iclog->ic_next;
kmem_free(iclog);
iclog = next_iclog;
@@ -1578,14 +1605,7 @@ xlog_dealloc_log(xlog_t *log)
spinlock_destroy(&log->l_grant_lock); xfs_buf_free(log->l_xbuf);
-#ifdef XFS_LOG_TRACE
- if (log->l_trace != NULL) {
- ktrace_free(log->l_trace);
- }
- if (log->l_grant_trace != NULL) {
- ktrace_free(log->l_grant_trace);
- }
-#endif
+ xlog_trace_loggrant_dealloc(log);
log->l_mp->m_log = NULL;
kmem_free(log);
} /* xlog_dealloc_log */
Index: 2.6.x-clean/fs/xfs/xfs_log_priv.h
===================================================================
--- 2.6.x-clean.orig/fs/xfs/xfs_log_priv.h
+++ 2.6.x-clean/fs/xfs/xfs_log_priv.h
@@ -448,7 +448,6 @@ typedef struct log {
int l_grant_write_bytes;#ifdef XFS_LOG_TRACE
- struct ktrace *l_trace;
struct ktrace *l_grant_trace;
#endifIndex: 2.6.x-clean/fs/xfs/xfsidbg.c
===================================================================
--- 2.6.x-clean.orig/fs/xfs/xfsidbg.c
+++ 2.6.x-clean/fs/xfs/xfsidbg.c
@@ -5864,7 +5864,7 @@ xfsidbg_xlog(xlog_t *log)
(int)BTOBBT(log->l_grant_write_bytes),
log->l_grant_write_bytes % BBSIZE);
#ifdef XFS_LOG_TRACE
- qprintf("trace: 0x%p grant_trace: use xlog value\n", log->l_trace);
+ qprintf("grant_trace: use xlog value\n");
#endif
} /* xfsidbg_xlog */ |
| <Prev in Thread] | Current Thread | [Next in Thread> |
|---|---|---|
| ||
| Previous by Date: | TAKE 985525 - Fix use after free in xfs_log_done()., Lachlan McIlroy |
|---|---|
| Next by Date: | Re: commit messages wrong in XFS git -master tree...., Lachlan McIlroy |
| Previous by Thread: | TAKE 985525 - Fix use after free in xfs_log_done()., Lachlan McIlroy |
| Next by Thread: | Re: [REVIEW] Move memory allocations for log tracing out of the critical path, Christoph Hellwig |
| Indexes: | [Date] [Thread] [Top] [All Lists] |