lkml.org 
[lkml]   [2007]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2] xfs: use xfs_get_buf_noaddr for iclogs
Does not look to me either.  Looks logical as well because these buffers 
are used only in log syncing and only one thread can be ever flushing
one ICLOG and, hence, no need for protection.
Even split buffer (log->l_xbuf) is used by only ICLOG at a time,
should not matter. I don't see protection for this even today as no
locking is done in split sync path.

-shailendra

Christoph Hellwig wrote:
> Currently xlog_alloc allocates memory for the iclogs first, then
> allocates a buffer using xfs_buf_get_empty and finally assigns
> the memory to the buffer. We don't really want to do this, but
> rather allocate a buffer with memory attached to it using
> xfs_buf_get_noaddr. There's a subtile change because
> xfs_buf_get_empty returns the buffer locked, but xfs_buf_get_noaddr
> returns it unlocked. From my auditing and testing nothing in the
> log I/O code cares about this distincition, but I'd be happy if
> someone could try to prove this independently.
>
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> Index: linux-2.6/fs/xfs/xfs_log.c
> ===================================================================
> --- linux-2.6.orig/fs/xfs/xfs_log.c 2007-03-06 17:26:40.000000000 +0100
> +++ linux-2.6/fs/xfs/xfs_log.c 2007-03-06 17:28:03.000000000 +0100
> @@ -1199,11 +1199,16 @@
> *iclogp = (xlog_in_core_t *)
> kmem_zalloc(sizeof(xlog_in_core_t), KM_SLEEP);
> iclog = *iclogp;
> - iclog->hic_data = (xlog_in_core_2_t *)
> - kmem_zalloc(iclogsize, KM_SLEEP | KM_LARGE);
> -
> iclog->ic_prev = prev_iclog;
> prev_iclog = iclog;
> +
> + bp = xfs_buf_get_noaddr(log->l_iclog_size, mp->m_logdev_targp);
> + XFS_BUF_SET_IODONE_FUNC(bp, xlog_iodone);
> + XFS_BUF_SET_BDSTRAT_FUNC(bp, xlog_bdstrat_cb);
> + XFS_BUF_SET_FSPRIVATE2(bp, (unsigned long)1);
> + iclog->ic_bp = bp;
> + iclog->hic_data = bp->b_addr;
> +
> log->l_iclog_bak[i] = (xfs_caddr_t)&(iclog->ic_header);
>
> head = &iclog->ic_header;
> @@ -1216,11 +1221,6 @@
> INT_SET(head->h_fmt, ARCH_CONVERT, XLOG_FMT);
> memcpy(&head->h_fs_uuid, &mp->m_sb.sb_uuid, sizeof(uuid_t));
>
> - bp = xfs_buf_get_empty(log->l_iclog_size, mp->m_logdev_targp);
> - XFS_BUF_SET_IODONE_FUNC(bp, xlog_iodone);
> - XFS_BUF_SET_BDSTRAT_FUNC(bp, xlog_bdstrat_cb);
> - XFS_BUF_SET_FSPRIVATE2(bp, (unsigned long)1);
> - iclog->ic_bp = bp;
>
> iclog->ic_size = XFS_BUF_SIZE(bp) - log->l_iclog_hsize;
> iclog->ic_state = XLOG_STATE_ACTIVE;
> @@ -1229,7 +1229,6 @@
> iclog->ic_datap = (char *)iclog->hic_data + log->l_iclog_hsize;
>
> ASSERT(XFS_BUF_ISBUSY(iclog->ic_bp));
> - ASSERT(XFS_BUF_VALUSEMA(iclog->ic_bp) <= 0);
> sv_init(&iclog->ic_forcesema, SV_DEFAULT, "iclog-force");
> sv_init(&iclog->ic_writesema, SV_DEFAULT, "iclog-write");
>
> @@ -1528,7 +1527,6 @@
> }
> #endif
> next_iclog = iclog->ic_next;
> - kmem_free(iclog->hic_data, log->l_iclog_size);
> kmem_free(iclog, sizeof(xlog_in_core_t));
> iclog = next_iclog;
> }
>
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2007-03-07 14:05    [W:0.084 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site