use bios directly in the log code v2
Hi all,
this series switches the log writing and log recovery code to use bios directly, and remove various special cases from the buffer cache code. Note that I have developed it on top of the previous series of log item related cleanups, so if you don't have that applied there is a small conflict. To make life easier I have pushed out a git branche here:
git://git.infradead.org/users/hch/xfs.git xfs-log-bio
Gitweb:
http://git.infradead.org/users/hch/xfs.git/shortlog/refs/heads/xfs-log-bio
Changes since v2:
- rename the 'flush' flag to 'need_flush'
- spelling fixes
- minor cleanups
Changes since v1:
- rebased to not required the log item series first
- split the bio-related code out of xfs_log_recover.c into a new file to ease using xfs_log_recover.c in xfsprogs
- use kmem_alloc_large instead of vmalloc to allocate the buffer
- additional minor cleanups
PATCH 1/20 xfs: remove the no-op spinlock_destroy stub
--- fs/xfs/xfs_linux.h | 2 -- fs/xfs/xfs_log.c | 2 -- 2 files changed, 4 deletions(-)
fs/xfs/xfs_linux.h +0 -2
@@ -110,8 +110,6 @@ typedef __u32 xfs_nlink_t;#define current_restore_flags_nested (sp, f) \ (current->flags = ((current->flags & ~(f)) | (*(sp) & (f)))) -#define spinlock_destroy (lock) -#define NBBY 8/* number of bits per byte */ /*
fs/xfs/xfs_log.c +0 -2
@@ -1580,7 +1580,6 @@ xlog_alloc_log(xfs_buf_free (iclog->ic_bp);kmem_free (iclog); } -spinlock_destroy (&log->l_icloglock);xfs_buf_free (log->l_xbuf); out_free_log:kmem_free (log);
@@ -2027,7 +2026,6 @@ xlog_dealloc_log(kmem_free (iclog); iclog = next_iclog; } -spinlock_destroy (&log->l_icloglock); log->l_mp->m_log =NULL ;kmem_free (log);
Signature · 1 line
2.20.1
PATCH 2/20 xfs: remove the never used _XBF_COMPOUND flag
In reply to Christoph Hellwig--- fs/xfs/xfs_buf.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
fs/xfs/xfs_buf.h +1 -3
@@ -49,7 +49,6 @@ typedef enum {#define _XBF_PAGES (1 << 20)/* backed by refcounted pages */ #define _XBF_KMEM (1 << 21)/* backed by heap memory */ #define _XBF_DELWRI_Q (1 << 22)/* buffer on a delwri queue */ -#define _XBF_COMPOUND (1 << 23)/* compound buffer */ typedef unsigned int xfs_buf_flags_t ;
@@ -69,8 +68,7 @@ typedef unsigned int xfs_buf_flags_t; { XBF_UNMAPPED, "UNMAPPED" }, /* ditto */\ { _XBF_PAGES, "PAGES" }, \ { _XBF_KMEM, "KMEM" }, \ - { _XBF_DELWRI_Q, "DELWRI_Q" }, \ - { _XBF_COMPOUND, "COMPOUND" } + { _XBF_DELWRI_Q, "DELWRI_Q" }/*
Signature · 1 line
2.20.1
PATCH 3/20 xfs: renumber XBF_WRITE_FAIL
In reply to Christoph HellwigAssining a numerical value that is not close to the flags defined near by is just asking for conflicts later on.
--- fs/xfs/xfs_buf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/xfs/xfs_buf.h +1 -1
@@ -34,7 +34,7 @@ typedef enum {#define XBF_ASYNC (1 << 4)/* initiator will not wait for completion */ #define XBF_DONE (1 << 5)/* all pages in the buffer uptodate */ #define XBF_STALE (1 << 6)/* buffer has been staled, do not find it */ -#define XBF_WRITE_FAIL (1 << 24)/* async writes have failed on this buffer */ +#define XBF_WRITE_FAIL (1 << 7)/* async writes have failed on this buffer */ /* I/O hints for the BIO layer */ #define XBF_SYNCIO (1 << 10)/* treat this buffer as synchronous I/O */
Signature · 1 line
2.20.1
PATCH 4/20 xfs: make mem_to_page available outside of xfs_buf.c
In reply to Christoph HellwigRename the function to kmem_to_page and move it to kmem.h together with our kmem_large allocator that may either return kmalloced or vmalloc pages.
--- fs/xfs/kmem.h | 8 ++++++++ fs/xfs/xfs_buf.c | 13 +------------ 2 files changed, 9 insertions(+), 12 deletions(-)
fs/xfs/kmem.h +8 -0
@@ -124,4 +124,12 @@ kmem_zone_zalloc(kmem_zone_t *zone, xfs_km_flags_t flags) return kmem_zone_alloc(zone, flags | KM_ZERO); } +static inline struct page * +kmem_to_page (void *addr) +{ +if (is_vmalloc_addr (addr)) +return vmalloc_to_page (addr); +return virt_to_page (addr); +} +#endif /* __XFS_SUPPORT_KMEM_H__ */
fs/xfs/xfs_buf.c +1 -12
@@ -933,17 +933,6 @@ xfs_buf_set_empty( bp->b_maps[0 ].bm_len = bp->b_length; } -static inline struct page * -mem_to_page ( -void *addr) -{ -if ((!is_vmalloc_addr (addr))) { -return virt_to_page (addr); - }else { -return vmalloc_to_page (addr); - } -} - int xfs_buf_associate_memory( xfs_buf_t *bp,
@@ -976,7 +965,7 @@ xfs_buf_associate_memory( bp->b_offset = offset;for (i =0 ; i < bp->b_page_count; i++) { - bp->b_pages[i] =mem_to_page ((void *)pageaddr); + bp->b_pages[i] =kmem_to_page ((void *)pageaddr); pageaddr +=PAGE_SIZE ; }
Signature · 1 line
2.20.1
PATCH 5/20 xfs: reformat xlog_get_lowest_lsn
In reply to Christoph HellwigReformat xlog_get_lowest_lsn to our usual style.
--- fs/xfs/xfs_log.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-)
fs/xfs/xfs_log.c +10 -14
@@ -2671,27 +2671,23 @@ xlog_state_clean_log( STATIC xfs_lsn_txlog_get_lowest_lsn ( -struct xlog *log) +struct xlog *log) { -xlog_in_core_t *lsn_log; -xfs_lsn_t lowest_lsn, lsn; +struct xlog_in_core *iclog = log->l_iclog; +xfs_lsn_t lowest_lsn =0 , lsn; - lsn_log = log->l_iclog; - lowest_lsn =0 ;do { -if (!(lsn_log->ic_state & (XLOG_STATE_ACTIVE |XLOG_STATE_DIRTY ))) { - lsn =be64_to_cpu (lsn_log->ic_header.h_lsn); -if ((lsn && !lowest_lsn) || - (XFS_LSN_CMP (lsn, lowest_lsn) <0 )) { +if (iclog->ic_state & (XLOG_STATE_ACTIVE |XLOG_STATE_DIRTY )) +continue ; + + lsn =be64_to_cpu (iclog->ic_header.h_lsn); +if ((lsn && !lowest_lsn) ||XFS_LSN_CMP (lsn, lowest_lsn) <0 ) lowest_lsn = lsn; - } - } - lsn_log = lsn_log->ic_next; - }while (lsn_log != log->l_iclog); + }while ((iclog = iclog->ic_next) != log->l_iclog); +return lowest_lsn; } - STATIC void xlog_state_do_callback( struct xlog *log,
Signature · 1 line
2.20.1
PATCH 6/20 xfs: don't use REQ_PREFLUSH for split log writes
In reply to Christoph HellwigIf we have to split a log write because it wraps the end of the log we can't just use REQ_PREFLUSH to flush before the first log write, as the writes might get reordered somewhere in the I/O stack. Issue a manual flush in that case so that the ordering of the two log I/Os doesn't matter.
--- fs/xfs/xfs_log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/xfs/xfs_log.c +1 -1
@@ -1941,7 +1941,7 @@ xlog_sync(* synchronously here; for an internal log we can simply use the block * layer state machine for preflushes. */ -if (log->l_mp->m_logdev_targp != log->l_mp->m_ddev_targp) +if (log->l_mp->m_logdev_targp != log->l_mp->m_ddev_targp || split)xfs_blkdev_issue_flush (log->l_mp->m_ddev_targp);else bp->b_flags |=XBF_FLUSH ;
Signature · 1 line
2.20.1
Re: [PATCH 06/20] xfs: don't use REQ_PREFLUSH for split log writes
Quoted 17 lines · Christoph Hellwig
If we have to split a log write because it wraps the end of the log we can't just use
REQ_PREFLUSHto flush before the first log write, as the writes might get reordered somewhere in the I/O stack. Issue a manual flush in that case so that the ordering of the two log I/Os doesn't matter.Signed-off-by Christoph Hellwig <[email protected]>Reviewed-by Dave Chinner <[email protected]>--- fs/xfs/xfs_log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)fs/xfs/xfs_log.c +0
-0
fs/xfs/xfs_log.c xlog_sync( · line 1945
Quoted 6 lines
fs/xfs/xfs_log.c +1
-1@@ -1941,7 +1941,7 @@ xlog_sync(* synchronously here; for an internal log we can simply use the block * layer state machine for preflushes. */ -if (log->l_mp->m_logdev_targp != log->l_mp->m_ddev_targp) +if (log->l_mp->m_logdev_targp != log->l_mp->m_ddev_targp || split)xfs_blkdev_issue_flush (log->l_mp->m_ddev_targp);
I'm curious if this is really necessary. The log record isn't recoverable until it's complete on disk (and thus the tail LSN stamped in the record header not relevant). As long as the cache flushes before the record is completely written, what difference does it make if it was made up of two out of order I/Os?
Granted log wrapping is not a frequent operation, but the explicit flush is a synchronous operation in the log force path whereas the flush flag isn't.
Brian
Quoted 5 lines
fs/xfs/xfs_log.c +0
-0else bp->b_flags |=XBF_FLUSH ;Signature · 1 line
2.20.1
Re: [PATCH 06/20] xfs: don't use REQ_PREFLUSH for split log writes
Quoted 22 lines · Brian Foster
Quoted 21 lines · Christoph Hellwig
If we have to split a log write because it wraps the end of the log we can't just use
REQ_PREFLUSHto flush before the first log write, as the writes might get reordered somewhere in the I/O stack. Issue a manual flush in that case so that the ordering of the two log I/Os doesn't matter.Signed-off-by Christoph Hellwig <[email protected]>Reviewed-by Dave Chinner <[email protected]>--- fs/xfs/xfs_log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)fs/xfs/xfs_log.c +0
-1@@ -1941,7 +1941,7 @@ xlog_sync(* synchronously here; for an internal log we can simply use the block * layer state machine for preflushes. */ -if (log->l_mp->m_logdev_targp != log->l_mp->m_ddev_targp)
Quoted 8 lines
fs/xfs/xfs_log.c xlog_sync( · line 1945
Quoted 2 lines
fs/xfs/xfs_log.c +1
-0+if (log->l_mp->m_logdev_targp != log->l_mp->m_ddev_targp || split)xfs_blkdev_issue_flush (log->l_mp->m_ddev_targp);I'm curious if this is really necessary. The log record isn't recoverable until it's complete on disk (and thus the tail LSN stamped in the record header not relevant). As long as the cache flushes before the record is completely written, what difference does it make if it was made up of two out of order I/Os?
The problem is not whether the log write is recoverable, it's whether what it overwrites is already on stable storage. i.e. the tail of the log can be overwritten by the split write to the start of the log before the cache flush in the first iclog IO makes the metadata it is overwriting stable. i.e:
metadata write -> volatile disk cache move log tail forwards -> tail wraps back to start
<...>
log write wrapping tail
iclog split
iclog write to end /w PREFLUSH + FUA
-> queued in request queue
iclog write to start /w FUA
-> queued in request queue
<....>
request queue gets processed
dispatches write to start w/ FUA
-> overwrites tail of log
<....>
dispatches write to end w/ PREFLUSH + FUA -> flushes metadata @ tail of log
If we have a power loss incident after the first FUA write to the start of the log but before the second write issues/completes the PREFLUSH, we have a situation on disk where the log tail has been overwritten but the metadata that it overwrote had not yet been committed to stable storage. That will result in either a corrupt log (can't find tail) or a corrupt fielsystem because metadata in some structure was not recovered.
Quoted 3 lines
Granted log wrapping is not a frequent operation, but the explicit flush is a synchronous operation in the log force path whereas the flush flag isn't.
We have the options of:
1) issuing a synchronous flush before both writes and then doing them w/ FUA only; or
2) issuing both log writes with PREFLUSH+FUA.
In the first case, the fact the cache flush is done synchronously really doesn't affect anything - it's done in the CIL push kworker context, so blocking here doesn't really add any extra latency to anything except synchronous log force waiters. Hence, typically, there is nothing waiting on the log being flushed so it what extra latency there is mostly won't matter.
In the second case, one of the cache flushes is superfluous and for busy filesytems with small logs where we frequently hit the wrap case this may add up to quite a bit of avoidable IO overhead....
Either way works, it's not clear to me that one is always superior to the other, so we just have to chose one....
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
Re: [PATCH 06/20] xfs: don't use REQ_PREFLUSH for split log writes
Quoted 32 lines · Dave Chinner
Quoted 22 lines · Brian Foster
Quoted 21 lines · Christoph Hellwig
If we have to split a log write because it wraps the end of the log we can't just use
REQ_PREFLUSHto flush before the first log write, as the writes might get reordered somewhere in the I/O stack. Issue a manual flush in that case so that the ordering of the two log I/Os doesn't matter.Signed-off-by Christoph Hellwig <[email protected]>Reviewed-by Dave Chinner <[email protected]>--- fs/xfs/xfs_log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)fs/xfs/xfs_log.c +0
-1@@ -1941,7 +1941,7 @@ xlog_sync(* synchronously here; for an internal log we can simply use the block * layer state machine for preflushes. */ -if (log->l_mp->m_logdev_targp != log->l_mp->m_ddev_targp)Quoted 8 lines
fs/xfs/xfs_log.c xlog_sync( · line 1945
Quoted 2 lines
fs/xfs/xfs_log.c +1
-0+if (log->l_mp->m_logdev_targp != log->l_mp->m_ddev_targp || split)xfs_blkdev_issue_flush (log->l_mp->m_ddev_targp);I'm curious if this is really necessary. The log record isn't recoverable until it's complete on disk (and thus the tail LSN stamped in the record header not relevant). As long as the cache flushes before the record is completely written, what difference does it make if it was made up of two out of order I/Os?
Quoted 6 lines
The problem is not whether the log write is recoverable, it's whether what it overwrites is already on stable storage. i.e. the tail of the log can be overwritten by the split write to the start of the log before the cache flush in the first iclog IO makes the metadata it is overwriting stable. i.e:
Ah, I see. If the log is full, the head pushes behind the tail and associated head record refers to the current tail. The AIL pushes the tail forward some and a log record writeback occurs with a new tail, overwriting the previous tail. If the metadata writeback has not persisted by the time we start that tail overwrite and we crash before the record completes in the log, then we still need to recover from the previous tail record (we just partially overwrote) to the previous head on the next mount, which would be troublesome. Makes sense, thanks.
Quoted 36 lines
metadata write -> volatile disk cache move log tail forwards -> tail wraps back to start<...>
log write wrapping tail
iclog split
iclog write to end /w PREFLUSH + FUA
-> queued in request queue
iclog write to start /w FUA
-> queued in request queue
<....>
request queue gets processed
dispatches write to start w/ FUA
-> overwrites tail of log
<....>dispatches write to end w/ PREFLUSH + FUA -> flushes metadata @ tail of logIf we have a power loss incident after the first FUA write to the start of the log but before the second write issues/completes the PREFLUSH, we have a situation on disk where the log tail has been overwritten but the metadata that it overwrote had not yet been committed to stable storage. That will result in either a corrupt log (can't find tail) or a corrupt fielsystem because metadata in some structure was not recovered.
Quoted 3 lines
Granted log wrapping is not a frequent operation, but the explicit flush is a synchronous operation in the log force path whereas the flush flag isn't.
We have the options of:
1) issuing a synchronous flush before both writes and then doing them w/ FUA only; or
2) issuing both log writes with PREFLUSH+FUA.
Quoted 7 lines
In the first case, the fact the cache flush is done synchronously really doesn't affect anything - it's done in the CIL push kworker context, so blocking here doesn't really add any extra latency to anything except synchronous log force waiters. Hence, typically, there is nothing waiting on the log being flushed so it what extra latency there is mostly won't matter.
It was more just a question of whether it was necessary than a major performance concern. But I was considering the integrity of the current tail update (i.e., from in-core AIL state) and didn't consider the integrity of the current tail itself if it lies just beyond the head (i.e., active log range).
Quoted 7 lines
In the second case, one of the cache flushes is superfluous and for busy filesytems with small logs where we frequently hit the wrap case this may add up to quite a bit of avoidable IO overhead....
Either way works, it's not clear to me that one is always superior to the other, so we just have to chose one....
Yep. Another thing that briefly crossed my mind is whether we could in theory optimize flushes out if the tail hasn't moved since the last flush. We'd still have to FUA the log records, but we haven't introduced any such integrity/ordering requirements if the tail hasn't changed, right?
It's debatable whether that would provide any value, but it might at least apply to certain scenarios like if the tail happens to be pinned by a single object across several iclog writes, or if fsyncs or some other pattern result in smaller/frequent checkpoints relative to metadata writeback, etc.
Brian
Quoted 6 lines
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
Re: [PATCH 06/20] xfs: don't use REQ_PREFLUSH for split log writes
Quoted 11 lines · Brian Foster
Yep. Another thing that briefly crossed my mind is whether we could in theory optimize flushes out if the tail hasn't moved since the last flush. We'd still have to FUA the log records, but we haven't introduced any such integrity/ordering requirements if the tail hasn't changed, right?
It's debatable whether that would provide any value, but it might at least apply to certain scenarios like if the tail happens to be pinned by a single object across several iclog writes, or if fsyncs or some other pattern result in smaller/frequent checkpoints relative to metadata writeback, etc.
Well, for the fsync we require the data device to be flushed for the fsync semantics anyway. So we'd now need to treat that special, simiar to that old log_flushed return parameter hack.
Re: [PATCH 06/20] xfs: don't use REQ_PREFLUSH for split log writes
Quoted 13 lines · Christoph Hellwig
Quoted 11 lines · Brian Foster
Yep. Another thing that briefly crossed my mind is whether we could in theory optimize flushes out if the tail hasn't moved since the last flush. We'd still have to FUA the log records, but we haven't introduced any such integrity/ordering requirements if the tail hasn't changed, right?
It's debatable whether that would provide any value, but it might at least apply to certain scenarios like if the tail happens to be pinned by a single object across several iclog writes, or if fsyncs or some other pattern result in smaller/frequent checkpoints relative to metadata writeback, etc.
Quoted 3 lines
Well, for the fsync we require the data device to be flushed for the fsync semantics anyway. So we'd now need to treat that special, simiar to that old
log_flushedreturn parameter hack.
Yeah, good point. It looks like the current fsync code already relies on the flush associated with the log force if the log and data devices are the same (otherwise we explicitly flush the data dev between data writeback and the log force). Any optimization there probably wouldn't buy anything for fsync callers that have to write file data. It would just shift responsibility of the flush to the fsync and perhaps complicate that current log_flushed thing since it also kind of assumes an unconditional flush based on iclog state.
Brian
PATCH 7/20 xfs: factor out log buffer writing from xlog_sync
In reply to Christoph HellwigReplace the not very useful xlog_bdstrat wrapper with a new version that that takes care of all the common logic for writing log buffers. Use the opportunity to avoid overloading the buffer address with the log relative address, and to shed the unused return value.
--- fs/xfs/xfs_log.c | 125 +++++++++++++++++------------------------------ 1 file changed, 45 insertions(+), 80 deletions(-)
fs/xfs/xfs_log.c +45 -80
@@ -45,10 +45,6 @@ STATIC intxlog_space_left ( struct xlog *log, atomic64_t *head); -STATIC int -xlog_sync ( -struct xlog *log, -struct xlog_in_core *iclog); STATIC voidxlog_dealloc_log (struct xlog *log);
@@ -1765,28 +1761,34 @@ xlog_cksum(return xfs_end_cksum (crc); } -/* -* The bdstrat callback function for log bufs. This gives us a central -* place to trap bufs in case we get hit by a log I/O error and need to -* shutdown. Actually, in practice, even when we didn't get a log error, -* we transition the iclogs to IOERROR state *after* flushing all existing -* iclogs to disk. This is because we don't want anymore new transactions to be -* started or completed afterwards. -* -* We lock the iclogbufs here so that we can serialise against IO completion -* during unmount. We might be processing a shutdown triggered during unmount, -* and that can occur asynchronously to the unmount thread, and hence we need to -* ensure that completes before tearing down the iclogbufs. Hence we need to -* hold the buffer lock across the log IO to acheive that. -*/ -STATIC int -xlog_bdstrat ( -struct xfs_buf *bp) +STATIC void +xlog_write_iclog ( +struct xlog *log, +struct xlog_in_core *iclog, +struct xfs_buf *bp, +uint64_t bno, +bool need_flush) { -struct xlog_in_core *iclog = bp->b_log_item; +ASSERT (bno < log->l_logBBsize); +ASSERT (bno + bp->b_io_length <= log->l_logBBsize); + bp->b_maps[0 ].bm_bn = log->l_logBBstart + bno; + bp->b_log_item = iclog; + bp->b_flags &= ~XBF_FLUSH ; + bp->b_flags |= (XBF_ASYNC |XBF_SYNCIO |XBF_WRITE |XBF_FUA ); +if (need_flush) + bp->b_flags |=XBF_FLUSH ; + +/* +* We lock the iclogbufs here so that we can serialise against I/O +* completion during unmount. We might be processing a shutdown +* triggered during unmount, and that can occur asynchronously to the +* unmount thread, and hence we need to ensure that completes before +* tearing down the iclogbufs. Hence we need to hold the buffer lock +* across the log IO to archieve that. +*/ xfs_buf_lock (bp); -if (iclog->ic_state &XLOG_STATE_IOERROR ) { +if (unlikely (iclog->ic_state &XLOG_STATE_IOERROR )) {xfs_buf_ioerror (bp, -EIO );xfs_buf_stale (bp);xfs_buf_ioend (bp);
@@ -1796,11 +1798,10 @@ xlog_bdstrat(* doing it here. Similarly, IO completion will unlock the * buffer, so we don't do it here. */ -return 0 ; +return ; }xfs_buf_submit (bp); -return 0 ; }/*
@@ -1823,25 +1824,23 @@ xlog_bdstrat(* log will require grabbing the lock though. * * The entire log manager uses a logical block numbering scheme. Only -* log_sync (and then only bwrite()) know about the fact that the log may -* not start with block zero on a given device. The log block start offset -* is added immediately before calling bwrite(). +* xlog_write_iclog knows about the fact that the log may not start with +* block zero on a given device. */ - -STATIC int +STATIC voidxlog_sync (struct xlog *log,struct xlog_in_core *iclog) { -xfs_buf_t *bp;int i;uint count;/* byte count of bwrite */ uint count_init;/* initial count before roundup */ int roundoff;/* roundoff to BB or stripe */ int split =0 ;/* split write into two regions */ -int error;int v2 =xfs_sb_version_haslogv2 (&log->l_mp->m_sb); +uint64_t bno;int size; +bool need_flush = true;XFS_STATS_INC (log->l_mp, xs_log_writes);ASSERT (atomic_read (&iclog->ic_refcnt) ==0 );
@@ -1877,17 +1876,16 @@ xlog_sync( size += roundoff; iclog->ic_header.h_len =cpu_to_be32 (size); - bp = iclog->ic_bp; -XFS_BUF_SET_ADDR (bp,BLOCK_LSN (be64_to_cpu (iclog->ic_header.h_lsn))); -XFS_STATS_ADD (log->l_mp, xs_log_blocks,BTOBB (count)); + bno =BLOCK_LSN (be64_to_cpu (iclog->ic_header.h_lsn)); +/* Do we need to split this write into 2 parts? */ -if (XFS_BUF_ADDR (bp) +BTOBB (count) > log->l_logBBsize) { +if (bno +BTOBB (count) > log->l_logBBsize) {char *dptr; - split = count - (BBTOB (log->l_logBBsize -XFS_BUF_ADDR (bp))); - count =BBTOB (log->l_logBBsize -XFS_BUF_ADDR (bp)); + split = count - (BBTOB (log->l_logBBsize - bno)); + count =BBTOB (log->l_logBBsize - bno); iclog->ic_bwritecnt =2 ;/*
@@ -1928,11 +1926,6 @@ xlog_sync( be64_to_cpu(iclog->ic_header.h_lsn)); } - bp->b_io_length =BTOBB (count); - bp->b_log_item = iclog; - bp->b_flags &= ~XBF_FLUSH ; - bp->b_flags |= (XBF_ASYNC |XBF_SYNCIO |XBF_WRITE |XBF_FUA ); -/* * Flush the data device before flushing the log to make sure all meta * data written back from the AIL actually made it to disk before
@@ -1941,50 +1934,22 @@ xlog_sync(* synchronously here; for an internal log we can simply use the block * layer state machine for preflushes. */ -if (log->l_mp->m_logdev_targp != log->l_mp->m_ddev_targp || split) +if (log->l_mp->m_logdev_targp != log->l_mp->m_ddev_targp || split) {xfs_blkdev_issue_flush (log->l_mp->m_ddev_targp); -else - bp->b_flags |=XBF_FLUSH ; + need_flush = false; + } -ASSERT (XFS_BUF_ADDR (bp) <= log->l_logBBsize-1 ); -ASSERT (XFS_BUF_ADDR (bp) +BTOBB (count) <= log->l_logBBsize); + iclog->ic_bp->b_io_length =BTOBB (count);xlog_verify_iclog (log, iclog, count, true); +xlog_write_iclog (log, iclog, iclog->ic_bp, bno, need_flush); -/* account for log which doesn't start at block #0 */ -XFS_BUF_SET_ADDR (bp,XFS_BUF_ADDR (bp) + log->l_logBBstart); - -/* -* Don't call xfs_bwrite here. We do log-syncs even when the filesystem -* is shutting down. -*/ - error =xlog_bdstrat (bp); -if (error) { -xfs_buf_ioerror_alert (bp,"xlog_sync" ); -return error; - }if (split) { - bp = iclog->ic_log->l_xbuf; -XFS_BUF_SET_ADDR (bp,0 );/* logical 0 */ -xfs_buf_associate_memory (bp, +xfs_buf_associate_memory (iclog->ic_log->l_xbuf, (char *)&iclog->ic_header + count, split); - bp->b_log_item = iclog; - bp->b_flags &= ~XBF_FLUSH ; - bp->b_flags |= (XBF_ASYNC |XBF_SYNCIO |XBF_WRITE |XBF_FUA ); - -ASSERT (XFS_BUF_ADDR (bp) <= log->l_logBBsize-1 ); -ASSERT (XFS_BUF_ADDR (bp) +BTOBB (count) <= log->l_logBBsize); - -/* account for internal log which doesn't start at block #0 */ -XFS_BUF_SET_ADDR (bp,XFS_BUF_ADDR (bp) + log->l_logBBstart); - error =xlog_bdstrat (bp); -if (error) { -xfs_buf_ioerror_alert (bp,"xlog_sync (split)" ); -return error; - } +xlog_write_iclog (log, iclog, iclog->ic_log->l_xbuf,0 , false); } -return 0 ; -}/* xlog_sync */ +}/* * Deallocate a log structure
@@ -3224,7 +3189,7 @@ xlog_state_release_iclog(* flags after this point. */ if (sync) -return xlog_sync (log, iclog); +xlog_sync (log, iclog);return 0 ; }/* xlog_state_release_iclog */
Signature · 1 line
2.20.1
Re: [PATCH 07/20] xfs: factor out log buffer writing from xlog_sync
Quoted 6 lines · Christoph Hellwig
Replace the not very useful
xlog_bdstratwrapper with a new version that that takes care of all the common logic for writing log buffers. Use the opportunity to avoid overloading the buffer address with the log relative address, and to shed the unused return value.Signed-off-by Christoph Hellwig <[email protected]>
LGTM.
Signature · 2 lines
Dave Chinner [email protected]
PATCH 8/20 xfs: factor out splitting of an iclog from xlog_sync
In reply to Christoph HellwigSplit out a self-contained chunk of code from xlog_sync that calculates the split offset for an iclog that wraps the log end and bumps the cycles for the second half.
Use the chance to bring some sanity to the variables used to track the split in xlog_sync by not changing the count variable, and instead use split as the offset for the split and use those to calculate the sizes and offsets for the two write buffers.
--- fs/xfs/xfs_log.c | 63 +++++++++++++++++++++++++----------------------- 1 file changed, 33 insertions(+), 30 deletions(-)
fs/xfs/xfs_log.c +33 -30
@@ -1804,6 +1804,32 @@ xlog_write_iclog(xfs_buf_submit (bp); } +/* +* We need to bump cycle number for the part of the iclog that is +* written to the start of the log. Watch out for the header magic +* number case, though. +*/ +static unsigned int +xlog_split_iclog ( +struct xlog *log, +void *data, +uint64_t bno, +unsigned int count) +{ +unsigned int split_offset =BBTOB (log->l_logBBsize - bno); +unsigned int i; + +for (i = split_offset; i < count; i +=BBSIZE ) { +uint32_t cycle =get_unaligned_be32 (data + i); + +if (++cycle ==XLOG_HEADER_MAGIC_NUM ) + cycle++; +put_unaligned_be32 (cycle, data + i); + } + +return split_offset; +} +/* * Flush out the in-core log (iclog) to the on-disk log in an asynchronous * fashion. Previously, we should have moved the current iclog
@@ -1832,13 +1858,12 @@ xlog_sync(struct xlog *log, struct xlog_in_core *iclog) { - int i; uint count; /* byte count of bwrite */uint count_init;/* initial count before roundup */ int roundoff;/* roundoff to BB or stripe */ -int split =0 ;/* split write into two regions */ int v2 =xfs_sb_version_haslogv2 (&log->l_mp->m_sb);uint64_t bno; +unsigned int split =0 ;int size;bool need_flush = true;
@@ -1881,32 +1906,8 @@ xlog_sync( bno =BLOCK_LSN (be64_to_cpu (iclog->ic_header.h_lsn));/* Do we need to split this write into 2 parts? */ -if (bno +BTOBB (count) > log->l_logBBsize) { -char *dptr; - - split = count - (BBTOB (log->l_logBBsize - bno)); - count =BBTOB (log->l_logBBsize - bno); - iclog->ic_bwritecnt =2 ; - -/* -* Bump the cycle numbers at the start of each block in the -* part of the iclog that ends up in the buffer that gets -* written to the start of the log. -* -* Watch out for the header magic number case, though. -*/ - dptr = (char *)&iclog->ic_header + count; -for (i =0 ; i < split; i +=BBSIZE ) { -uint32_t cycle =be32_to_cpu (*(__be32 *)dptr); -if (++cycle ==XLOG_HEADER_MAGIC_NUM ) - cycle++; - *(__be32 *)dptr =cpu_to_be32 (cycle); - - dptr +=BBSIZE ; - } - }else { - iclog->ic_bwritecnt =1 ; - } +if (bno +BTOBB (count) > log->l_logBBsize) + split =xlog_split_iclog (log, &iclog->ic_header, bno, count);/* calculcate the checksum */ iclog->ic_header.h_crc = xlog_cksum(log, &iclog->ic_header,
@@ -1939,14 +1940,16 @@ xlog_sync( need_flush = false; } - iclog->ic_bp->b_io_length =BTOBB (count); + iclog->ic_bp->b_io_length =BTOBB (split ? split : count); + iclog->ic_bwritecnt = split ?2 :1 ;xlog_verify_iclog (log, iclog, count, true);xlog_write_iclog (log, iclog, iclog->ic_bp, bno, need_flush);if (split) {xfs_buf_associate_memory (iclog->ic_log->l_xbuf, - (char *)&iclog->ic_header + count, split); + (char *)&iclog->ic_header + split, + count - split);xlog_write_iclog (log, iclog, iclog->ic_log->l_xbuf,0 , false); } }
Signature · 1 line
2.20.1
Re: [PATCH 08/20] xfs: factor out splitting of an iclog from xlog_sync
Quoted 10 lines · Christoph Hellwig
Split out a self-contained chunk of code from
xlog_syncthat calculates the split offset for an iclog that wraps the log end and bumps the cycles for the second half.Use the chance to bring some sanity to the variables used to track the split in
xlog_syncby not changing the count variable, and instead use split as the offset for the split and use those to calculate the sizes and offsets for the two write buffers.Signed-off-by Christoph Hellwig <[email protected]>
Looks good.
Signature · 2 lines
Dave Chinner [email protected]
PATCH 9/20 xfs: factor out iclog size calculation from xlog_sync
In reply to Christoph HellwigSplit out another self-contained bit of code from xlog_sync.
--- fs/xfs/xfs_log.c | 67 +++++++++++++++++++++++++++++------------------- 1 file changed, 41 insertions(+), 26 deletions(-)
fs/xfs/xfs_log.c +41 -26
@@ -1830,6 +1830,39 @@ xlog_split_iclog(return split_offset; } +static int +xlog_calc_iclog_size ( +struct xlog *log, +struct xlog_in_core *iclog, +uint32_t *roundoff) +{ +uint32_t count_init, count; +bool use_lsunit; + + use_lsunit =xfs_sb_version_haslogv2 (&log->l_mp->m_sb) && + log->l_mp->m_sb.sb_logsunit >1 ; + +/* Add for LR header */ + count_init = log->l_iclog_hsize + iclog->ic_offset; + +/* Round out the log write size */ +if (use_lsunit) { +/* we have a v2 stripe unit to use */ + count =XLOG_LSUNITTOB (log,XLOG_BTOLSUNIT (log, count_init)); + }else { + count =BBTOB (BTOBB (count_init)); + } + +ASSERT (count >= count_init); + *roundoff = count - count_init; + +if (use_lsunit) +ASSERT (*roundoff < log->l_mp->m_sb.sb_logsunit); +else +ASSERT (*roundoff <BBTOB (1 )); +return count; +} +/* * Flush out the in-core log (iclog) to the on-disk log in an asynchronous * fashion. Previously, we should have moved the current iclog
@@ -1858,35 +1891,17 @@ xlog_sync(struct xlog *log, struct xlog_in_core *iclog) { - uint count; /* byte count of bwrite */ -uint count_init;/* initial count before roundup */ -int roundoff;/* roundoff to BB or stripe */ -int v2 =xfs_sb_version_haslogv2 (&log->l_mp->m_sb); -uint64_t bno; -unsigned int split =0 ; -int size; -bool need_flush = true; + unsigned int count; /* byte count of bwrite */ +unsigned int roundoff;/* roundoff to BB or stripe */ +uint64_t bno; +unsigned int split =0 ; +unsigned int size; +bool need_flush = true;XFS_STATS_INC (log->l_mp, xs_log_writes);ASSERT (atomic_read (&iclog->ic_refcnt) ==0 ); -/* Add for LR header */ - count_init = log->l_iclog_hsize + iclog->ic_offset; - -/* Round out the log write size */ -if (v2 && log->l_mp->m_sb.sb_logsunit >1 ) { -/* we have a v2 stripe unit to use */ - count =XLOG_LSUNITTOB (log,XLOG_BTOLSUNIT (log, count_init)); - }else { - count =BBTOB (BTOBB (count_init)); - } - roundoff = count - count_init; -ASSERT (roundoff >=0 ); -ASSERT ((v2 && log->l_mp->m_sb.sb_logsunit >1 && - roundoff < log->l_mp->m_sb.sb_logsunit) - || - (log->l_mp->m_sb.sb_logsunit <=1 && - roundoff <BBTOB (1 ))); + count =xlog_calc_iclog_size (log, iclog, &roundoff);/* move grant heads by roundoff in sync */ xlog_grant_add_space (log, &log->l_reserve_head.grant, roundoff);
@@ -1897,7 +1912,7 @@ xlog_sync(/* real byte length */ size = iclog->ic_offset; -if (v2) +if (xfs_sb_version_haslogv2 (&log->l_mp->m_sb)) size += roundoff; iclog->ic_header.h_len =cpu_to_be32 (size);
Signature · 1 line
2.20.1
Re: [PATCH 09/20] xfs: factor out iclog size calculation from xlog_sync
Quoted 3 lines · Christoph Hellwig
Split out another self-contained bit of code from
xlog_sync.Signed-off-by Christoph Hellwig <[email protected]>
Much easier on the eyes. :)
Signature · 2 lines
Dave Chinner [email protected]
PATCH 10/20 xfs: update both stat counters together in xlog_sync
In reply to Christoph HellwigJust a small bit of code tidying up.
--- fs/xfs/xfs_log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/xfs/xfs_log.c +1 -1
@@ -1898,7 +1898,6 @@ xlog_sync(unsigned int size;bool need_flush = true; -XFS_STATS_INC (log->l_mp, xs_log_writes);ASSERT (atomic_read (&iclog->ic_refcnt) ==0 ); count =xlog_calc_iclog_size (log, iclog, &roundoff);
@@ -1916,6 +1915,7 @@ xlog_sync( size += roundoff; iclog->ic_header.h_len =cpu_to_be32 (size); +XFS_STATS_INC (log->l_mp, xs_log_writes);XFS_STATS_ADD (log->l_mp, xs_log_blocks,BTOBB (count)); bno =BLOCK_LSN (be64_to_cpu (iclog->ic_header.h_lsn));
Signature · 1 line
2.20.1
PATCH 11/20 xfs: remove the syncing argument from xlog_verify_iclog
In reply to Christoph HellwigThe only caller unconditionally passes true here.
--- fs/xfs/xfs_log.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
fs/xfs/xfs_log.c +6 -8
@@ -103,8 +103,7 @@ STATIC voidxlog_verify_iclog ( struct xlog *log, struct xlog_in_core *iclog, - int count, - bool syncing); + int count); STATIC void xlog_verify_tail_lsn( struct xlog *log,
@@ -113,7 +112,7 @@ xlog_verify_tail_lsn(#else #define xlog_verify_dest_ptr (a,b)#define xlog_verify_grant_tail (a) -#define xlog_verify_iclog (a,b,c,d) +#define xlog_verify_iclog (a,b,c)#define xlog_verify_tail_lsn (a,b,c)#endif
@@ -1958,7 +1957,7 @@ xlog_sync( iclog->ic_bp->b_io_length =BTOBB (split ? split : count); iclog->ic_bwritecnt = split ?2 :1 ; -xlog_verify_iclog (log, iclog, count, true); +xlog_verify_iclog (log, iclog, count);xlog_write_iclog (log, iclog, iclog->ic_bp, bno, need_flush);if (split) {
@@ -3798,8 +3797,7 @@ STATIC voidxlog_verify_iclog ( struct xlog *log, struct xlog_in_core *iclog, - int count, - bool syncing) + int count) { xlog_op_header_t *ophead;xlog_in_core_t *icptr;
@@ -3843,7 +3841,7 @@ xlog_verify_iclog(/* clientid is only 1 byte */ p = &ophead->oh_clientid; field_offset = p - base_ptr; -if (!syncing || (field_offset &0x1ff )) { +if (field_offset &0x1ff ) { clientid = ophead->oh_clientid; }else { idx =BTOBBT ((char *)&ophead->oh_clientid - iclog->ic_datap);
@@ -3866,7 +3864,7 @@ xlog_verify_iclog(/* check length */ p = &ophead->oh_len; field_offset = p - base_ptr; -if (!syncing || (field_offset &0x1ff )) { +if (field_offset &0x1ff ) { op_len =be32_to_cpu (ophead->oh_len); }else { idx = BTOBBT((uintptr_t)&ophead->oh_len -
Signature · 1 line
2.20.1
PATCH 12/20 xfs: make use of the l_targ field in struct xlog
In reply to Christoph HellwigUse the slightly shorter way to get at the buftarg for the log device wherever we can in the log and log recovery code.
--- fs/xfs/xfs_log.c | 6 +++--- fs/xfs/xfs_log_recover.c | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-)
fs/xfs/xfs_log.c +3 -3
@@ -927,7 +927,7 @@ xfs_log_unmount_write(xfs_mount_t *mp)* Or, if we are doing a forced umount (typically because of IO errors). */ if (mp->m_flags &XFS_MOUNT_NORECOVERY || -xfs_readonly_buftarg (log->l_mp->m_logdev_targp)) { +xfs_readonly_buftarg (log->l_targ)) {ASSERT (mp->m_flags &XFS_MOUNT_RDONLY );return 0 ; }
@@ -1481,7 +1481,7 @@ xlog_alloc_log(* having set it up properly. */ error = -ENOMEM ; - bp =xfs_buf_alloc (mp->m_logdev_targp,XFS_BUF_DADDR_NULL , + bp =xfs_buf_alloc (log->l_targ,XFS_BUF_DADDR_NULL ,BTOBB (log->l_iclog_size),XBF_NO_IOACCT );if (!bp) goto out_free_log;
@@ -1949,7 +1949,7 @@ xlog_sync(* synchronously here; for an internal log we can simply use the block * layer state machine for preflushes. */ -if (log->l_mp->m_logdev_targp != log->l_mp->m_ddev_targp || split) { +if (log->l_targ != log->l_mp->m_ddev_targp || split) {xfs_blkdev_issue_flush (log->l_mp->m_ddev_targp); need_flush = false; }
fs/xfs/xfs_log_recover.c +2 -2
@@ -134,7 +134,7 @@ xlog_get_bp( nbblks += log->l_sectBBsize; nbblks =round_up (nbblks, log->l_sectBBsize); - bp =xfs_buf_get_uncached (log->l_mp->m_logdev_targp, nbblks,0 ); + bp =xfs_buf_get_uncached (log->l_targ, nbblks,0 );if (bp)xfs_buf_unlock (bp);return bp;
@@ -1505,7 +1505,7 @@ xlog_find_tail(* But... if the -device- itself is readonly, just skip this. * We can't recover this device anyway, so it won't matter. */ -if (!xfs_readonly_buftarg (log->l_mp->m_logdev_targp)) +if (!xfs_readonly_buftarg (log->l_targ)) error =xlog_clear_stale_blocks (log, tail_lsn); done:
Signature · 1 line
2.20.1
PATCH 13/20 xfs: use bios directly to write log buffers
In reply to Christoph HellwigCurrently the XFS logging code uses the xfs_buf structure and associated APIs to write the log buffers to disk. This requires various special cases in the log code and is generally not very optimal.
Instead of using a buffer just allocate a kmem_alloc_larger region for each log buffer, and use a bio and bio_vec array embedded in the iclog structure to write the buffer to disk. This also allows for using the bio split and chaining case to deal with the case of a log buffer wrapping around the end of the log.
--- fs/xfs/xfs_log.c | 220 ++++++++++++++++++++---------------------- fs/xfs/xfs_log_priv.h | 15 +-- 2 files changed, 112 insertions(+), 123 deletions(-)
fs/xfs/xfs_log.c +104 -116
@@ -1239,32 +1239,32 @@ xlog_space_left( } -/* -* Log function which is called when an io completes. -* -* The log manager needs its own routine, in order to control what -* happens with the buffer after the write completes. -*/ static void -xlog_iodone (xfs_buf_t *bp) +xlog_ioend_work ( +struct work_struct *work) { -struct xlog_in_core *iclog = bp->b_log_item; -struct xlog *l = iclog->ic_log; +struct xlog_in_core *iclog = + container_of(work, struct xlog_in_core, ic_end_io_work); +struct xlog *log = iclog->ic_log;int aborted =0 ; +int error; + +if (is_vmalloc_addr (iclog->ic_data)) +invalidate_kernel_vmap_range (iclog->ic_data, iclog->ic_io_size);/* * Race to shutdown the filesystem if we see an error or the iclog is in * IOABORT state. The IOABORT state is only set in DEBUG mode to inject * CRC errors into log recovery. */ -if (XFS_TEST_ERROR (bp->b_error, l->l_mp,XFS_ERRTAG_IODONE_IOERR ) || + error =blk_status_to_errno (iclog->ic_bio.bi_status); +if (XFS_TEST_ERROR (error, log->l_mp,XFS_ERRTAG_IODONE_IOERR ) || iclog->ic_state &XLOG_STATE_IOABORT ) {if (iclog->ic_state &XLOG_STATE_IOABORT ) iclog->ic_state &= ~XLOG_STATE_IOABORT ; -xfs_buf_ioerror_alert (bp, __func__); -xfs_buf_stale (bp); -xfs_force_shutdown (l->l_mp,SHUTDOWN_LOG_IO_ERROR ); +xfs_alert (log->l_mp,"log I/O error %d" , error); +xfs_force_shutdown (log->l_mp,SHUTDOWN_LOG_IO_ERROR );/* * This flag will be propagated to the trans-committed * callback routines to let them know that the log-commit
@@ -1275,17 +1275,16 @@ xlog_iodone(xfs_buf_t *bp) aborted =XFS_LI_ABORTED ; } -/* log I/O is always issued ASYNC */ -ASSERT (bp->b_flags &XBF_ASYNC );xlog_state_done_syncing (iclog, aborted); +bio_uninit (&iclog->ic_bio);/* -* drop the buffer lock now that we are done. Nothing references -* the buffer after this, so an unmount waiting on this lock can now -* tear it down safely. As such, it is unsafe to reference the buffer -* (bp) after the unlock as we could race with it being freed. +* Drop the lock to signal that we are done. Nothing references the +* iclog after this, so an unmount waiting on this lock can now tear it +* down safely. As such, it is unsafe to reference the iclog after the +* unlock as we could race with it being freed. */ -xfs_buf_unlock (bp); +up (&iclog->ic_sema); }/*
@@ -1417,7 +1416,6 @@ xlog_alloc_log(xlog_rec_header_t *head;xlog_in_core_t **iclogp;xlog_in_core_t *iclog, *prev_iclog=NULL ; -xfs_buf_t *bp;int i;int error = -ENOMEM ;uint log2_size =0 ;
@@ -1475,30 +1473,6 @@ xlog_alloc_log(xlog_get_iclog_buffer_size (mp, log); -/* -* Use a NULL block for the extra log buffer used during splits so that -* it will trigger errors if we ever try to do IO on it without first -* having set it up properly. -*/ - error = -ENOMEM ; - bp =xfs_buf_alloc (log->l_targ,XFS_BUF_DADDR_NULL , -BTOBB (log->l_iclog_size),XBF_NO_IOACCT ); -if (!bp) - goto out_free_log; - -/* -* The iclogbuf buffer locks are held over IO but we are not going to do -* IO yet. Hence unlock the buffer so that the log IO path can grab it -* when appropriately. -*/ -ASSERT (xfs_buf_islocked (bp)); -xfs_buf_unlock (bp); - -/* use high priority wq for log I/O completion */ - bp->b_ioend_wq = mp->m_log_workqueue; - bp->b_iodone = xlog_iodone; - log->l_xbuf = bp; -spin_lock_init (&log->l_icloglock);init_waitqueue_head (&log->l_flush_wait);
@@ -1512,7 +1486,9 @@ xlog_alloc_log(*/ ASSERT (log->l_iclog_size >=4096 );for (i=0 ; i < log->l_iclog_bufs; i++) { - *iclogp =kmem_zalloc (sizeof (xlog_in_core_t),KM_MAYFAIL ); + *iclogp =kmem_zalloc (struct_size (*iclogp, ic_bvec, +howmany (log->l_iclog_size,PAGE_SIZE )), +KM_MAYFAIL );if (!*iclogp) goto out_free_iclog;
@@ -1520,20 +1496,10 @@ xlog_alloc_log( iclog->ic_prev = prev_iclog; prev_iclog = iclog; - bp =xfs_buf_get_uncached (mp->m_logdev_targp, -BTOBB (log->l_iclog_size), -XBF_NO_IOACCT ); -if (!bp) + iclog->ic_data =kmem_alloc_large (log->l_iclog_size, +KM_MAYFAIL ); +if (!iclog->ic_data) goto out_free_iclog; - -ASSERT (xfs_buf_islocked (bp)); -xfs_buf_unlock (bp); - -/* use high priority wq for log I/O completion */ - bp->b_ioend_wq = mp->m_log_workqueue; - bp->b_iodone = xlog_iodone; - iclog->ic_bp = bp; - iclog->ic_data = bp->b_addr;#ifdef DEBUG log->l_iclog_bak[i] = &iclog->ic_header;#endif
@@ -1547,7 +1513,7 @@ xlog_alloc_log( head->h_fmt =cpu_to_be32 (XLOG_FMT );memcpy (&head->h_fs_uuid, &mp->m_sb.sb_uuid,sizeof (uuid_t)); - iclog->ic_size =BBTOB (bp->b_length) - log->l_iclog_hsize; + iclog->ic_size = log->l_iclog_size - log->l_iclog_hsize; iclog->ic_state =XLOG_STATE_ACTIVE ; iclog->ic_log = log;atomic_set (&iclog->ic_refcnt,0 );
@@ -1557,6 +1523,8 @@ xlog_alloc_log(init_waitqueue_head (&iclog->ic_force_wait);init_waitqueue_head (&iclog->ic_write_wait); +INIT_WORK (&iclog->ic_end_io_work, xlog_ioend_work); +sema_init (&iclog->ic_sema,1 ); iclogp = &iclog->ic_next; }
@@ -1571,11 +1539,9 @@ xlog_alloc_log( out_free_iclog:for (iclog = log->l_iclog; iclog; iclog = prev_iclog) { prev_iclog = iclog->ic_next; -if (iclog->ic_bp) -xfs_buf_free (iclog->ic_bp); +kmem_free (iclog->ic_data);kmem_free (iclog); } -xfs_buf_free (log->l_xbuf); out_free_log:kmem_free (log); out:
@@ -1760,23 +1726,43 @@ xlog_cksum(return xfs_end_cksum (crc); } +static void +xlog_bio_end_io ( +struct bio *bio) +{ +struct xlog_in_core *iclog = bio->bi_private; + +queue_work (iclog->ic_log->l_mp->m_log_workqueue, + &iclog->ic_end_io_work); +} + +static void +xlog_map_iclog_data ( +struct bio *bio, +void *data, +size_t count) +{ +do { +struct page *page =kmem_to_page (data); +unsigned int off =offset_in_page (data); +size_t len =min_t (size_t, count,PAGE_SIZE - off); + +WARN_ON_ONCE (bio_add_page (bio, page, len, off) != len); + + data += len; + count -= len; + }while (count); +} + STATIC voidxlog_write_iclog (struct xlog *log,struct xlog_in_core *iclog, -struct xfs_buf *bp,uint64_t bno, +unsigned int count,bool need_flush) {ASSERT (bno < log->l_logBBsize); -ASSERT (bno + bp->b_io_length <= log->l_logBBsize); - - bp->b_maps[0 ].bm_bn = log->l_logBBstart + bno; - bp->b_log_item = iclog; - bp->b_flags &= ~XBF_FLUSH ; - bp->b_flags |= (XBF_ASYNC |XBF_SYNCIO |XBF_WRITE |XBF_FUA ); -if (need_flush) - bp->b_flags |=XBF_FLUSH ;/* * We lock the iclogbufs here so that we can serialise against I/O
@@ -1786,11 +1772,10 @@ xlog_write_iclog(* tearing down the iclogbufs. Hence we need to hold the buffer lock * across the log IO to archieve that. */ -xfs_buf_lock (bp); +down (&iclog->ic_sema);if (unlikely (iclog->ic_state &XLOG_STATE_IOERROR )) { -xfs_buf_ioerror (bp, -EIO ); -xfs_buf_stale (bp); -xfs_buf_ioend (bp); +xlog_state_done_syncing (iclog,XFS_LI_ABORTED ); +up (&iclog->ic_sema);/* * It would seem logical to return EIO here, but we rely on * the log state machine to propagate I/O errors instead of
@@ -1800,7 +1785,38 @@ xlog_write_iclog(return ; } -xfs_buf_submit (bp); + iclog->ic_io_size = count; + +bio_init (&iclog->ic_bio, iclog->ic_bvec,howmany (count,PAGE_SIZE )); +bio_set_dev (&iclog->ic_bio, log->l_targ->bt_bdev); + iclog->ic_bio.bi_iter.bi_sector = log->l_logBBstart + bno; + iclog->ic_bio.bi_end_io = xlog_bio_end_io; + iclog->ic_bio.bi_private = iclog; + iclog->ic_bio.bi_opf =REQ_OP_WRITE |REQ_META |REQ_SYNC |REQ_FUA ; +if (need_flush) + iclog->ic_bio.bi_opf |=REQ_PREFLUSH ; + +xlog_map_iclog_data (&iclog->ic_bio, iclog->ic_data, iclog->ic_io_size); +if (is_vmalloc_addr (iclog->ic_data)) +flush_kernel_vmap_range (iclog->ic_data, iclog->ic_io_size); + +/* +* If this log buffer would straddle the end of the log we will have +* to split it up into two bios, so that we can continue at the start. +*/ +if (bno +BTOBB (count) > log->l_logBBsize) { +struct bio *split; + + split =bio_split (&iclog->ic_bio, log->l_logBBsize - bno, +GFP_NOIO , &fs_bio_set); +bio_chain (split, &iclog->ic_bio); +submit_bio (split); + +/* restart at logical offset zero for the remainder */ + iclog->ic_bio.bi_iter.bi_sector = log->l_logBBstart; + } + +submit_bio (&iclog->ic_bio); }/*
@@ -1808,7 +1824,7 @@ xlog_write_iclog(* written to the start of the log. Watch out for the header magic * number case, though. */ -static unsigned int +static void xlog_split_iclog( struct xlog *log, void *data,
@@ -1825,8 +1841,6 @@ xlog_split_iclog( cycle++;put_unaligned_be32 (cycle, data + i); } - -return split_offset; } static int
@@ -1893,9 +1907,8 @@ xlog_sync(unsigned int count;/* byte count of bwrite */ unsigned int roundoff;/* roundoff to BB or stripe */ uint64_t bno; -unsigned int split =0 ;unsigned int size; -bool need_flush = true; +bool need_flush = true, split = false;ASSERT (atomic_read (&iclog->ic_refcnt) ==0 );
@@ -1920,8 +1933,10 @@ xlog_sync( bno =BLOCK_LSN (be64_to_cpu (iclog->ic_header.h_lsn));/* Do we need to split this write into 2 parts? */ -if (bno +BTOBB (count) > log->l_logBBsize) - split =xlog_split_iclog (log, &iclog->ic_header, bno, count); +if (bno +BTOBB (count) > log->l_logBBsize) { +xlog_split_iclog (log, &iclog->ic_header, bno, count); + split = true; + }/* calculcate the checksum */ iclog->ic_header.h_crc = xlog_cksum(log, &iclog->ic_header,
@@ -1954,18 +1969,8 @@ xlog_sync( need_flush = false; } - iclog->ic_bp->b_io_length =BTOBB (split ? split : count); - iclog->ic_bwritecnt = split ?2 :1 ; -xlog_verify_iclog (log, iclog, count); -xlog_write_iclog (log, iclog, iclog->ic_bp, bno, need_flush); - -if (split) { -xfs_buf_associate_memory (iclog->ic_log->l_xbuf, - (char *)&iclog->ic_header + split, - count - split); -xlog_write_iclog (log, iclog, iclog->ic_log->l_xbuf,0 , false); - } +xlog_write_iclog (log, iclog, bno, count, need_flush); }/*
@@ -1986,25 +1991,15 @@ xlog_dealloc_log(*/ iclog = log->l_iclog;for (i =0 ; i < log->l_iclog_bufs; i++) { -xfs_buf_lock (iclog->ic_bp); -xfs_buf_unlock (iclog->ic_bp); +down (&iclog->ic_sema); +up (&iclog->ic_sema); iclog = iclog->ic_next; } -/* -* Always need to ensure that the extra buffer does not point to memory -* owned by another log buffer before we free it. Also, cycle the lock -* first to ensure we've completed IO on it. -*/ -xfs_buf_lock (log->l_xbuf); -xfs_buf_unlock (log->l_xbuf); -xfs_buf_set_empty (log->l_xbuf,BTOBB (log->l_iclog_size)); -xfs_buf_free (log->l_xbuf); - iclog = log->l_iclog;for (i =0 ; i < log->l_iclog_bufs; i++) { -xfs_buf_free (iclog->ic_bp); next_iclog = iclog->ic_next; +kmem_free (iclog->ic_data);kmem_free (iclog); iclog = next_iclog; }
@@ -2922,8 +2917,6 @@ xlog_state_done_syncing(ASSERT (iclog->ic_state ==XLOG_STATE_SYNCING || iclog->ic_state ==XLOG_STATE_IOERROR );ASSERT (atomic_read (&iclog->ic_refcnt) ==0 ); -ASSERT (iclog->ic_bwritecnt ==1 || iclog->ic_bwritecnt ==2 ); -/* * If we got an error, either on the first buffer, or in the case of
@@ -2931,13 +2924,8 @@ xlog_state_done_syncing(* and none should ever be attempted to be written to disk * again. */ -if (iclog->ic_state !=XLOG_STATE_IOERROR ) { -if (--iclog->ic_bwritecnt ==1 ) { -spin_unlock (&log->l_icloglock); -return ; - } +if (iclog->ic_state !=XLOG_STATE_IOERROR ) iclog->ic_state =XLOG_STATE_DONE_SYNC ; - }/* * Someone could be sleeping prior to writing out the next
fs/xfs/xfs_log_priv.h +8 -7
@@ -179,7 +179,6 @@ typedef struct xlog_ticket {* the iclog. * - ic_forcewait is used to implement synchronous forcing of the iclog to disk. * - ic_next is the pointer to the next iclog in the ring. -* - ic_bp is a pointer to the buffer used to write this incore log to disk. * - ic_log is a pointer back to the global log structure. * - ic_callback is a linked list of callback function/argument pairs to be * called after an iclog finishes writing.
@@ -206,11 +205,10 @@ typedef struct xlog_in_core {wait_queue_head_t ic_write_wait;struct xlog_in_core *ic_next;struct xlog_in_core *ic_prev; -struct xfs_buf *ic_bp;struct xlog *ic_log; -int ic_size; -int ic_offset; -int ic_bwritecnt; +u32 ic_size; +u32 ic_io_size; +u32 ic_offset;unsigned short ic_state;char *ic_datap;/* pointer to iclog data */
@@ -223,6 +221,11 @@ typedef struct xlog_in_core {atomic_t ic_refcnt____cacheline_aligned_in_smp ;xlog_in_core_2_t *ic_data;#define ic_header ic_data->hic_header + +struct semaphore ic_sema; +struct work_struct ic_end_io_work; +struct bio ic_bio; +struct bio_vec ic_bvec[]; } xlog_in_core_t;/*
@@ -350,8 +353,6 @@ struct xlog {struct xfs_mount *l_mp;/* mount point */ struct xfs_ail *l_ailp;/* AIL log is working with */ struct xfs_cil *l_cilp;/* CIL log is working with */ -struct xfs_buf *l_xbuf;/* extra buffer for log -* wrapping */ struct xfs_buftarg *l_targ;/* buftarg of log */ struct delayed_work l_work;/* background flush work */ uint l_flags;
Signature · 1 line
2.20.1
Re: [PATCH 13/20] xfs: use bios directly to write log buffers
Quoted 5 lines · Christoph Hellwig
Currently the XFS logging code uses the
xfs_bufstructure and associated APIs to write the log buffers to disk. This requires various special cases in the log code and is generally not very optimal.
Quoted 7 lines
Instead of using a buffer just allocate a
kmem_alloc_largerregion for each log buffer, and use a bio andbio_vecarray embedded in the iclog structure to write the buffer to disk. This also allows for using the bio split and chaining case to deal with the case of a log buffer wrapping around the end of the log.Signed-off-by Christoph Hellwig <[email protected]>
Overall a good cleanup, comments inline.
FWIW, what does ic_sema protect? It looks to me like it just replaces the xfs_buf_lock(), and the only reason we were using that is to allow unmount to wait for iclogbuf IO completion. Can we just use a completion for this now?
Quoted 24 lines
--- fs/xfs/xfs_log.c | 220 ++++++++++++++++++++---------------------- fs/xfs/xfs_log_priv.h | 15 +-- 2 files changed, 112 insertions(+), 123 deletions(-)fs/xfs/xfs_log.c +2
-7@@ -1239,32 +1239,32 @@ xlog_space_left( } -/* -* Log function which is called when an io completes. -* -* The log manager needs its own routine, in order to control what -* happens with the buffer after the write completes. -*/ static void -xlog_iodone (xfs_buf_t *bp) +xlog_ioend_work ( +struct work_struct *work) {
fs/xfs/xfs_log.c xlog_space_left( · line 1253
Quoted 10 lines
fs/xfs/xfs_log.c +7
-2-struct xlog_in_core *iclog = bp->b_log_item; -struct xlog *l = iclog->ic_log; +struct xlog_in_core *iclog = + container_of(work, struct xlog_in_core, ic_end_io_work); +struct xlog *log = iclog->ic_log;int aborted =0 ; +int error; + +if (is_vmalloc_addr (iclog->ic_data)) +invalidate_kernel_vmap_range (iclog->ic_data, iclog->ic_io_size);
Do we need to invalidate here for write only operation? It's only when we are bringing new data into memory we have to invalidate the range, right? e.g. xfs_buf_bio_end_io() only does invalidation on read IO.
Quoted 32 lines
fs/xfs/xfs_log.c +0
-24@@ -1475,30 +1473,6 @@ xlog_alloc_log(xlog_get_iclog_buffer_size (mp, log); -/* -* Use a NULL block for the extra log buffer used during splits so that -* it will trigger errors if we ever try to do IO on it without first -* having set it up properly. -*/ - error = -ENOMEM ; - bp =xfs_buf_alloc (log->l_targ,XFS_BUF_DADDR_NULL , -BTOBB (log->l_iclog_size),XBF_NO_IOACCT ); -if (!bp) - goto out_free_log; - -/* -* The iclogbuf buffer locks are held over IO but we are not going to do -* IO yet. Hence unlock the buffer so that the log IO path can grab it -* when appropriately. -*/ -ASSERT (xfs_buf_islocked (bp)); -xfs_buf_unlock (bp); - -/* use high priority wq for log I/O completion */ - bp->b_ioend_wq = mp->m_log_workqueue; - bp->b_iodone = xlog_iodone; - log->l_xbuf = bp; -spin_lock_init (&log->l_icloglock);init_waitqueue_head (&log->l_flush_wait);
fs/xfs/xfs_log.c xlog_alloc_log( · line 1488
Quoted 3 lines
fs/xfs/xfs_log.c +0
-0@@ -1512,7 +1486,9 @@ xlog_alloc_log(*/ ASSERT (log->l_iclog_size >=4096 );for (i=0 ; i < log->l_iclog_bufs; i++) {
Fix the whitespace while you are touching this code?
fs/xfs/xfs_log.c xlog_alloc_log( · line 1491
Quoted 4 lines
fs/xfs/xfs_log.c +3
-1- *iclogp =kmem_zalloc (sizeof (xlog_in_core_t),KM_MAYFAIL ); + *iclogp =kmem_zalloc (struct_size (*iclogp, ic_bvec, +howmany (log->l_iclog_size,PAGE_SIZE )), +KM_MAYFAIL );
That's a bit of a mess - hard to read. It's times like this that I think generic helpers make the code worse rather than bettter. Perhaps some slightly different indenting to indicate that the howmany() function is actually a parameter of the struct_size() macro?
*iclogp = kmem_zalloc(struct_size(*iclogp, ic_bvec, howmany(log->l_iclog_size, PAGE_SIZE)), KM_MAYFAIL);
fs/xfs/xfs_log.c xlog_cksum( · line 1737
Quoted 9 lines
fs/xfs/xfs_log.c +9
-0+static void +xlog_bio_end_io ( +struct bio *bio) +{ +struct xlog_in_core *iclog = bio->bi_private; + +queue_work (iclog->ic_log->l_mp->m_log_workqueue, + &iclog->ic_end_io_work); +}
Can we just put a pointer to the wq in the iclog? It only needs to be set up at init time, then this only needs to be
queue_work(iclog->ic_wq, &iclog->ic_end_io_work);
Quoted 8 lines
fs/xfs/xfs_log.c +8
-0+ +static void +xlog_map_iclog_data ( +struct bio *bio, +void *data, +size_t count) +{ + do {
fs/xfs/xfs_log.c xlog_cksum( · line 1755
Quoted 10 lines
fs/xfs/xfs_log.c +10
-0+struct page *page =kmem_to_page (data); +unsigned int off =offset_in_page (data); +size_t len =min_t (size_t, count,PAGE_SIZE - off); + +WARN_ON_ONCE (bio_add_page (bio, page, len, off) != len); + + data += len; + count -= len; + }while (count); +}
Aren't we're always going to be mapping the same pages to the same bio at the same offsets. The only thing that changes is the length of the bio and the sector it is addressed to. It seems kind of odd to have an inline data buffer, bio and biovec all pre-allocated, but then have to map them into exactly the same state for every IO we do with them...
fs/xfs/xfs_log.c xlog_write_iclog( · line 1778
Quoted 12 lines
fs/xfs/xfs_log.c +3
-4@@ -1786,11 +1772,10 @@ xlog_write_iclog(* tearing down the iclogbufs. Hence we need to hold the buffer lock * across the log IO to archieve that. */ -xfs_buf_lock (bp); +down (&iclog->ic_sema);if (unlikely (iclog->ic_state &XLOG_STATE_IOERROR )) { -xfs_buf_ioerror (bp, -EIO ); -xfs_buf_stale (bp); -xfs_buf_ioend (bp); +xlog_state_done_syncing (iclog,XFS_LI_ABORTED ); +up (&iclog->ic_sema);
Hmmm - this open codes the end io error completion. Might be wroth a comment indicating that this needs to be kept in sync with the io completion processing?
Quoted 14 lines
fs/xfs/xfs_log_priv.h +0
-1@@ -179,7 +179,6 @@ typedef struct xlog_ticket {* the iclog. * - ic_forcewait is used to implement synchronous forcing of the iclog to disk. * - ic_next is the pointer to the next iclog in the ring. -* - ic_bp is a pointer to the buffer used to write this incore log to disk. * - ic_log is a pointer back to the global log structure. * - ic_callback is a linked list of callback function/argument pairs to be * called after an iclog finishes writing. @@ -206,11 +205,10 @@ typedef struct xlog_in_core {wait_queue_head_t ic_write_wait;
fs/xfs/xfs_log_priv.h typedef struct xlog_in_core { · line 211
Quoted 10 lines
fs/xfs/xfs_log_priv.h +3
-4struct xlog_in_core *ic_next;struct xlog_in_core *ic_prev; -struct xfs_buf *ic_bp;struct xlog *ic_log; -int ic_size; -int ic_offset; -int ic_bwritecnt; +u32 ic_size; +u32 ic_io_size; +u32 ic_offset;
Can we get a couple of comments here describing the difference between ic_size, ic_io_size and log->l_iclog_size so I don't have to go read all the code to find out what they are again in 6 months time?
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
Re: [PATCH 13/20] xfs: use bios directly to write log buffers
Quoted 4 lines · Dave Chinner
FWIW, what does
ic_semaprotect? It looks to me like it just replaces thexfs_buf_lock(), and the only reason we were using that is to allow unmount to wait for iclogbuf IO completion. Can we just use a completion for this now?
We could, I just didn't want to change it cosmetically as that whole pattern looks a little odd, and I'd like to spend some more time figuring out what we could do better at a higher level.
Quoted 7 lines
Quoted 7 lines
fs/xfs/xfs_log.c +4
-2-struct xlog_in_core *iclog = bp->b_log_item; -struct xlog *l = iclog->ic_log; +struct xlog_in_core *iclog = + container_of(work, struct xlog_in_core, ic_end_io_work); +struct xlog *log = iclog->ic_log;int aborted =0 ; +int error;
Quoted 8 lines
fs/xfs/xfs_log.c xlog_space_left( · line 1253
Quoted 3 lines
fs/xfs/xfs_log.c +3
-0+ +if (is_vmalloc_addr (iclog->ic_data)) +invalidate_kernel_vmap_range (iclog->ic_data, iclog->ic_io_size);Do we need to invalidate here for write only operation? It's only when we are bringing new data into memory we have to invalidate the range, right? e.g.
xfs_buf_bio_end_io()only does invalidation on read IO.
True, we shouldn't eed this one.
Quoted 3 lines
Quoted 1 line
for (i=0; i <
log->l_iclog_bufs; i++) {Fix the whitespace while you are touching this code?
Well, I usually do for everything I touch, but this line isn't touched. But I can do that anyway.
Quoted 11 lines
fs/xfs/xfs_log.c xlog_alloc_log( · line 1491
Quoted 4 lines
fs/xfs/xfs_log.c +3
-1- *iclogp =kmem_zalloc (sizeof (xlog_in_core_t),KM_MAYFAIL ); + *iclogp =kmem_zalloc (struct_size (*iclogp, ic_bvec, +howmany (log->l_iclog_size,PAGE_SIZE )), +KM_MAYFAIL );That's a bit of a mess - hard to read. It's times like this that I think generic helpers make the code worse rather than bettter. Perhaps some slightly different indenting to indicate that the
howmany()function is actually a parameter of thestruct_size()macro?
Quoted 3 lines
*iclogp = kmem_zalloc(struct_size(*iclogp, ic_bvec, howmany(log->l_iclog_size, PAGE_SIZE)), KM_MAYFAIL);
I don't really find this any better. Then again switching to make this line based on iclog and only assigning iclogp later might be nicer.
Quoted 13 lines
fs/xfs/xfs_log.c xlog_cksum( · line 1737
Quoted 9 lines
fs/xfs/xfs_log.c +9
-0+static void +xlog_bio_end_io ( +struct bio *bio) +{ +struct xlog_in_core *iclog = bio->bi_private; + +queue_work (iclog->ic_log->l_mp->m_log_workqueue, + &iclog->ic_end_io_work); +}Can we just put a pointer to the wq in the iclog? It only needs to be set up at init time, then this only needs to be
Quoted 1 line
queue_work(iclog->ic_wq, &iclog->ic_end_io_work);
The workqueue pointer is moving to the xlog later in the series. I don't really see any point to bloat every iclog with an extra pointer.
Quoted 6 lines
Aren't we're always going to be mapping the same pages to the same bio at the same offsets. The only thing that changes is the length of the bio and the sector it is addressed to. It seems kind of odd to have an inline data buffer, bio and biovec all pre-allocated, but then have to map them into exactly the same state for every IO we do with them...
We are, sort of. The length of the actual data is different each time, so we might not build up all bvecs, and the last one might not be filled entirely.
Quoted 6 lines
fs/xfs/xfs_log.c xlog_write_iclog( · line 1778
Quoted 2 lines
fs/xfs/xfs_log.c +2
-0+xlog_state_done_syncing (iclog,XFS_LI_ABORTED ); +up (&iclog->ic_sema);Hmmm - this open codes the end io error completion. Might be wroth a comment indicating that this needs to be kept in sync with the io completion processing?
Ok.
Quoted 8 lines
fs/xfs/xfs_log_priv.h typedef struct xlog_in_core { · line 211
Quoted 3 lines
fs/xfs/xfs_log_priv.h +3
-0+u32 ic_size; +u32 ic_io_size; +u32 ic_offset;Can we get a couple of comments here describing the difference between
ic_size,ic_io_sizeandlog->l_iclog_sizeso I don't have to go read all the code to find out what they are again in 6 months time?
Ok.
PATCH 14/20 xfs: move the log ioend workqueue to struct xlog
In reply to Christoph HellwigMove the workqueue used for log I/O completions from struct xfs_mount to struct xlog to keep it self contained in the log code.
--- fs/xfs/xfs_log.c | 13 +++++++++++-- fs/xfs/xfs_log_priv.h | 1 + fs/xfs/xfs_mount.h | 1 - fs/xfs/xfs_super.c | 11 +---------- 4 files changed, 13 insertions(+), 13 deletions(-)
fs/xfs/xfs_log.c +11 -2
@@ -1531,11 +1531,19 @@ xlog_alloc_log( *iclogp = log->l_iclog;/* complete ring */ log->l_iclog->ic_prev = prev_iclog;/* re-write 1st prev ptr */ + log->l_ioend_workqueue =alloc_workqueue ("xfs-log/%s" , +WQ_MEM_RECLAIM |WQ_FREEZABLE |WQ_HIGHPRI ,0 , + mp->m_fsname); +if (!log->l_ioend_workqueue) + goto out_free_iclog; + error =xlog_cil_init (log);if (error) - goto out_free_iclog; + goto out_destroy_workqueue;return log; +out_destroy_workqueue: +destroy_workqueue (log->l_ioend_workqueue); out_free_iclog:for (iclog = log->l_iclog; iclog; iclog = prev_iclog) { prev_iclog = iclog->ic_next;
@@ -1732,7 +1740,7 @@ xlog_bio_end_io( {struct xlog_in_core *iclog = bio->bi_private; -queue_work (iclog->ic_log->l_mp->m_log_workqueue, +queue_work (iclog->ic_log->l_ioend_workqueue, &iclog->ic_end_io_work); }
@@ -1984,6 +1992,7 @@ xlog_dealloc_log(int i;xlog_cil_destroy (log); +destroy_workqueue (log->l_ioend_workqueue);/* * Cycle all the iclogbuf locks to make sure all log IO completion
fs/xfs/xfs_log_priv.h +1 -0
@@ -354,6 +354,7 @@ struct xlog {struct xfs_ail *l_ailp;/* AIL log is working with */ struct xfs_cil *l_cilp;/* CIL log is working with */ struct xfs_buftarg *l_targ;/* buftarg of log */ +struct workqueue_struct *l_ioend_workqueue;/* for I/O completions */ struct delayed_work l_work;/* background flush work */ uint l_flags;uint l_quotaoffs_flag;/* XFS_DQ_*, for QUOTAOFFs */
fs/xfs/xfs_mount.h +0 -1
@@ -198,7 +198,6 @@ typedef struct xfs_mount {struct workqueue_struct *m_unwritten_workqueue;struct workqueue_struct *m_cil_workqueue;struct workqueue_struct *m_reclaim_workqueue; -struct workqueue_struct *m_log_workqueue;struct workqueue_struct *m_eofblocks_workqueue;struct workqueue_struct *m_sync_workqueue;
fs/xfs/xfs_super.c +1 -10
@@ -840,16 +840,10 @@ xfs_init_mount_workqueues(if (!mp->m_reclaim_workqueue) goto out_destroy_cil; - mp->m_log_workqueue =alloc_workqueue ("xfs-log/%s" , -WQ_MEM_RECLAIM |WQ_FREEZABLE |WQ_HIGHPRI ,0 , - mp->m_fsname); -if (!mp->m_log_workqueue) - goto out_destroy_reclaim; - mp->m_eofblocks_workqueue =alloc_workqueue ("xfs-eofblocks/%s" ,WQ_MEM_RECLAIM |WQ_FREEZABLE ,0 , mp->m_fsname);if (!mp->m_eofblocks_workqueue) - goto out_destroy_log; + goto out_destroy_reclaim; mp->m_sync_workqueue =alloc_workqueue ("xfs-sync/%s" ,WQ_FREEZABLE ,0 , mp->m_fsname);
@@ -860,8 +854,6 @@ xfs_init_mount_workqueues( out_destroy_eofb:destroy_workqueue (mp->m_eofblocks_workqueue); -out_destroy_log: -destroy_workqueue (mp->m_log_workqueue); out_destroy_reclaim:destroy_workqueue (mp->m_reclaim_workqueue); out_destroy_cil:
@@ -880,7 +872,6 @@ xfs_destroy_mount_workqueues( {destroy_workqueue (mp->m_sync_workqueue);destroy_workqueue (mp->m_eofblocks_workqueue); -destroy_workqueue (mp->m_log_workqueue);destroy_workqueue (mp->m_reclaim_workqueue);destroy_workqueue (mp->m_cil_workqueue);destroy_workqueue (mp->m_unwritten_workqueue);
Signature · 1 line
2.20.1
Re: [PATCH 14/20] xfs: move the log ioend workqueue to struct xlog
The build/test bot found an issue with this one leading to crashes at unmount, and I think this incremental patch should fix it:
fs/xfs/xfs_log.c +1 -1
@@ -1954,7 +1954,6 @@ xlog_dealloc_log(int i;xlog_cil_destroy (log); -destroy_workqueue (log->l_ioend_workqueue);/* * Cycle all the iclogbuf locks to make sure all log IO completion
@@ -1976,6 +1975,7 @@ xlog_dealloc_log( } log->l_mp->m_log =NULL ; +destroy_workqueue (log->l_ioend_workqueue);kmem_free (log); }/* xlog_dealloc_log */
Re: [PATCH 14/20] xfs: move the log ioend workqueue to struct xlog
Quoted 2 lines · Christoph Hellwig
The build/test bot found an issue with this one leading to crashes at unmount, and I think this incremental patch should fix it:
I have't see any crashes at unmount; would you mind sharing the report?
--D
Quoted 20 lines
fs/xfs/xfs_log.c +1
-1@@ -1954,7 +1954,6 @@ xlog_dealloc_log(int i;xlog_cil_destroy (log); -destroy_workqueue (log->l_ioend_workqueue);/* * Cycle all the iclogbuf locks to make sure all log IO completion @@ -1976,6 +1975,7 @@ xlog_dealloc_log( } log->l_mp->m_log =NULL ; +destroy_workqueue (log->l_ioend_workqueue);kmem_free (log); }/* xlog_dealloc_log */
Re: [PATCH 14/20] xfs: move the log ioend workqueue to struct xlog
Quoted 5 lines · Darrick J. Wong
Quoted 2 lines · Christoph Hellwig
The build/test bot found an issue with this one leading to crashes at unmount, and I think this incremental patch should fix it:
I have't see any crashes at unmount; would you mind sharing the report?
I'll forward you the report I got from the test bot.
PATCH 15/20 xfs: return an offset instead of a pointer from xlog_align
In reply to Christoph HellwigThis simplifies both the helper and the callers. We lost a bit of size sanity checking, but that is already covered by KASAN if needed.
--- fs/xfs/xfs_log_recover.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-)
fs/xfs/xfs_log_recover.c +5 -11
@@ -151,20 +151,14 @@ xlog_put_bp(* Return the address of the start of the given block number's data * in a log buffer. The buffer covers a log sector-aligned region. */ -STATIC char * +static inline unsigned int xlog_align (struct xlog *log, -xfs_daddr_t blk_no, -int nbblks, -struct xfs_buf *bp) +xfs_daddr_t blk_no) { -xfs_daddr_t offset = blk_no & ((xfs_daddr_t )log->l_sectBBsize -1 ); - -ASSERT (offset + nbblks <= bp->b_length); -return bp->b_addr +BBTOB (offset); +return BBTOB (blk_no & ((xfs_daddr_t )log->l_sectBBsize -1 )); } -/* * nbblks should be uint, but oh well. Just want to catch that 32-bit length. */
@@ -216,7 +210,7 @@ xlog_bread(if (error)return error; - *offset =xlog_align (log, blk_no, nbblks, bp); + *offset = bp->b_addr +xlog_align (log, blk_no);return 0 ; }
@@ -1713,7 +1707,7 @@ xlog_write_log_records( } - offset =xlog_align (log, start_block, endcount, bp); + offset = bp->b_addr +xlog_align (log, start_block);for (; j < endcount; j++) {xlog_add_record (log, offset, cycle, i+j, tail_cycle, tail_block);
Signature · 1 line
2.20.1
PATCH 16/20 xfs: use bios directly to read and write the log recovery buffers
In reply to Christoph HellwigThe xfs_buf structure is basically used as a glorified container for a memory allocation in the log recovery code. Replace it with a call to kmem_alloc_large and a simple abstraction to read into or write from it synchronously using chained bios.
--- fs/xfs/Makefile | 1 + fs/xfs/xfs_bio_io.c | 61 ++++++++++ fs/xfs/xfs_linux.h | 3 + fs/xfs/xfs_log_recover.c | 241 ++++++++++++++------------------------- 4 files changed, 149 insertions(+), 157 deletions(-) create mode 100644 fs/xfs/xfs_bio_io.c
fs/xfs/Makefile +1 -0
@@ -62,6 +62,7 @@ xfs-y += xfs_aops.o \ xfs_attr_inactive.o \ xfs_attr_list.o \ xfs_bmap_util.o \ + xfs_bio_io.o \ xfs_buf.o \ xfs_dir2_readdir.o \ xfs_discard.o \
fs/xfs/xfs_bio_io.c new file +61 -0
61 lines not on this page · raw message
fs/xfs/xfs_linux.h +3 -0
@@ -219,6 +219,9 @@ static inline uint64_t howmany_64(uint64_t x, uint32_t y)return x; } +int xfs_rw_bdev (struct block_device *bdev,sector_t sector,unsigned int count, +char *data,unsigned int op); +#define ASSERT_ALWAYS (expr) \ (likely(expr) ? (void)0 : assfail(#expr, __FILE__, __LINE__))
fs/xfs/xfs_log_recover.c +84 -157
503 lines not on this page · raw message
Signature · 1 line
2.20.1
Re: [PATCH 16/20] xfs: use bios directly to read and write the log recovery buffers
Quoted 6 lines · Christoph Hellwig
The
xfs_bufstructure is basically used as a glorified container for a memory allocation in the log recovery code. Replace it with a call tokmem_alloc_largeand a simple abstraction to read into or write from it synchronously using chained bios.Signed-off-by Christoph Hellwig <[email protected]>
....
Quoted 6 lines
fs/xfs/xfs_bio_io.c new file +6
-0+int +xfs_rw_bdev( + struct block_device *bdev, + sector_t sector, + unsigned int count, + char *data,
fs/xfs/xfs_bio_io.c line 27
Quoted 10 lines
fs/xfs/xfs_bio_io.c new file +10
-0+ unsigned int op) + +{ +unsigned int is_vmalloc =is_vmalloc_addr (data); +unsigned int left = count; +int error; +struct bio *bio; + +if (is_vmalloc && op ==REQ_OP_READ ) +flush_kernel_vmap_range (data, count);
That should be REQ_OP_WRITE. i.e. the data the CPU has written to the aliased vmap address has to be flushed back to the physical page before the DMA from the physical page to the storage device occurs.
Quoted 21 lines
fs/xfs/xfs_bio_io.c new file +21
-0+ bio =bio_alloc (GFP_KERNEL ,bio_max_vecs (left)); +bio_set_dev (bio, bdev); + bio->bi_iter.bi_sector = sector; + bio->bi_opf = op |REQ_META |REQ_SYNC ; + + do { + struct page *page = kmem_to_page(data); + unsigned int off = offset_in_page(data); + unsigned int len = min_t(unsigned, left, PAGE_SIZE - off); + + while (bio_add_page(bio, page, len, off) != len) { + struct bio *prev = bio; + + bio = bio_alloc(GFP_KERNEL, bio_max_vecs(left)); + bio_copy_dev(bio, prev); + bio->bi_iter.bi_sector = bio_end_sector(prev); + bio->bi_opf = prev->bi_opf; + bio_chain(bio, prev); + + submit_bio(prev); + }
fs/xfs/xfs_bio_io.c line 59
Quoted 10 lines
fs/xfs/xfs_bio_io.c new file +10
-0+ + data += len; + left -= len; + }while (left >0 ); + + error =submit_bio_wait (bio); +bio_put (bio); + +if (is_vmalloc && op ==REQ_OP_WRITE ) +invalidate_kernel_vmap_range (data, count);
And this should be REQ_OP_READ - to invalidate anything aliased in the CPU cache before the higher layers try to read the new data in the physical page...
Otherwise looks good.
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
Re: [PATCH 16/20] xfs: use bios directly to read and write the log recovery buffers
Quoted 7 lines
fs/xfs/xfs_bio_io.c line 27
Quoted 3 lines
fs/xfs/xfs_bio_io.c new file +3
-0+ +if (is_vmalloc && op ==REQ_OP_READ ) +flush_kernel_vmap_range (data, count);That should be
REQ_OP_WRITE. i.e. the data the CPU has written to the aliased vmap address has to be flushed back to the physical page before the DMA from the physical page to the storage device occurs.
Actually it probably should be unconditional, so that we have a clean cache to start with. That also matches what xfs_buf.c does.
Quoted 6 lines
fs/xfs/xfs_bio_io.c line 59
Quoted 2 lines
fs/xfs/xfs_bio_io.c new file +2
-0+if (is_vmalloc && op ==REQ_OP_WRITE ) +invalidate_kernel_vmap_range (data, count);And this should be
REQ_OP_READ- to invalidate anything aliased in the CPU cache before the higher layers try to read the new data in the physical page...
Fixed.
PATCH 17/20 xfs: stop using bp naming for log recovery buffers
In reply to Christoph HellwigNow that we don't use struct xfs_buf to hold log recovery buffer rename the related functions and variables to just talk of a buffer instead of using the bp name that we usually use for xfs_buf related functionality.
--- fs/xfs/xfs_log_recover.c | 201 ++++++++++++++++++++------------------- 1 file changed, 102 insertions(+), 99 deletions(-)
fs/xfs/xfs_log_recover.c +102 -99
@@ -79,7 +79,7 @@ struct xfs_buf_cancel {* are valid, false otherwise. */ static inline bool -xlog_verify_bp ( +xlog_verify_bno (struct xlog *log,xfs_daddr_t blk_no,int bbcount)
@@ -96,7 +96,7 @@ xlog_verify_bp(* a range of nbblks basic blocks at any valid offset within the log. */ static char * -xlog_get_bp ( +xlog_alloc_buffer (struct xlog *log,int nbblks) {
@@ -104,7 +104,7 @@ xlog_get_bp(* Pass log block 0 since we don't have an addr yet, buffer will be * verified on read. */ -if (!xlog_verify_bp (log,0 , nbblks)) { +if (!xlog_verify_bno (log,0 , nbblks)) {xfs_warn (log->l_mp,"Invalid block length (0x%x) for buffer" , nbblks);XFS_ERROR_REPORT (__func__,XFS_ERRLEVEL_HIGH , log->l_mp);
@@ -153,7 +153,7 @@ xlog_do_io( {int error; -if (!xlog_verify_bp (log, blk_no, nbblks)) { +if (!xlog_verify_bno (log, blk_no, nbblks)) {xfs_warn (log->l_mp,"Invalid log block/length (0x%llx, 0x%x) for buffer" , blk_no, nbblks);
@@ -327,7 +327,7 @@ xlog_recover_iodone( STATIC intxlog_find_cycle_start (struct xlog *log, -char *bp, +char *buffer,xfs_daddr_t first_blk,xfs_daddr_t *last_blk,uint cycle)
@@ -341,7 +341,7 @@ xlog_find_cycle_start( end_blk = *last_blk; mid_blk =BLK_AVG (first_blk, end_blk);while (mid_blk != first_blk && mid_blk != end_blk) { - error =xlog_bread (log, mid_blk,1 , bp, &offset); + error =xlog_bread (log, mid_blk,1 , buffer, &offset);if (error)return error; mid_cycle =xlog_get_cycle (offset);
@@ -377,7 +377,7 @@ xlog_find_verify_cycle( {xfs_daddr_t i, j;uint cycle; -char *bp; +char *buffer;xfs_daddr_t bufblks;char *buf =NULL ;int error =0 ;
@@ -391,7 +391,7 @@ xlog_find_verify_cycle( bufblks =1 <<ffs (nbblks);while (bufblks > log->l_logBBsize) bufblks >>=1 ; -while (!(bp =xlog_get_bp (log, bufblks))) { +while (!(buffer =xlog_alloc_buffer (log, bufblks))) { bufblks >>=1 ;if (bufblks < log->l_sectBBsize)return -ENOMEM ;
@@ -402,7 +402,7 @@ xlog_find_verify_cycle( bcount =min (bufblks, (start_blk + nbblks - i)); - error =xlog_bread (log, i, bcount, bp, &buf); + error =xlog_bread (log, i, bcount, buffer, &buf);if (error) goto out;
@@ -420,7 +420,7 @@ xlog_find_verify_cycle( *new_blk =-1 ; out: -kmem_free (bp); +kmem_free (buffer);return error; }
@@ -444,7 +444,7 @@ xlog_find_verify_log_record( int extra_bblks) {xfs_daddr_t i; -char *bp; +char *buffer;char *offset =NULL ;xlog_rec_header_t *head =NULL ;int error =0 ;
@@ -454,12 +454,14 @@ xlog_find_verify_log_record(ASSERT (start_blk !=0 || *last_blk != start_blk); -if (!(bp =xlog_get_bp (log, num_blks))) { -if (!(bp =xlog_get_bp (log,1 ))) + buffer =xlog_alloc_buffer (log, num_blks); +if (!buffer) { + buffer =xlog_alloc_buffer (log,1 ); +if (!buffer)return -ENOMEM ; smallmem =1 ; }else { - error =xlog_bread (log, start_blk, num_blks, bp, &offset); + error =xlog_bread (log, start_blk, num_blks, buffer, &offset);if (error) goto out; offset += ((num_blks -1 ) <<BBSHIFT );
@@ -476,7 +478,7 @@ xlog_find_verify_log_record( }if (smallmem) { - error =xlog_bread (log, i,1 , bp, &offset); + error =xlog_bread (log, i,1 , buffer, &offset);if (error) goto out; }
@@ -529,7 +531,7 @@ xlog_find_verify_log_record( *last_blk = i; out: -kmem_free (bp); +kmem_free (buffer);return error; }
@@ -551,7 +553,7 @@ xlog_find_head(struct xlog *log, xfs_daddr_t *return_head_blk) { - char *bp; + char *buffer;char *offset;xfs_daddr_t new_blk, first_blk, start_blk, last_blk, head_blk;int num_scan_bblks;
@@ -581,20 +583,20 @@ xlog_find_head( } first_blk =0 ;/* get cycle # of 1st block */ - bp =xlog_get_bp (log,1 ); -if (!bp) + buffer =xlog_alloc_buffer (log,1 ); +if (!buffer)return -ENOMEM ; - error =xlog_bread (log,0 ,1 , bp, &offset); + error =xlog_bread (log,0 ,1 , buffer, &offset);if (error) - goto bp_err; + goto out_free_buffer; first_half_cycle =xlog_get_cycle (offset); last_blk = head_blk = log_bbnum -1 ;/* get cycle # of last block */ - error =xlog_bread (log, last_blk,1 , bp, &offset); + error =xlog_bread (log, last_blk,1 , buffer, &offset);if (error) - goto bp_err; + goto out_free_buffer; last_half_cycle =xlog_get_cycle (offset);ASSERT (last_half_cycle !=0 );
@@ -662,9 +664,10 @@ xlog_find_head(* ^ we want to locate this spot */ stop_on_cycle = last_half_cycle; -if ((error =xlog_find_cycle_start (log, bp, first_blk, - &head_blk, last_half_cycle))) - goto bp_err; + error =xlog_find_cycle_start (log, buffer, first_blk, &head_blk, + last_half_cycle); +if (error) + goto out_free_buffer; }/*
@@ -684,7 +687,7 @@ xlog_find_head(if ((error =xlog_find_verify_cycle (log, start_blk, num_scan_bblks, stop_on_cycle, &new_blk))) - goto bp_err; + goto out_free_buffer;if (new_blk !=-1 ) head_blk = new_blk; }else {/* need to read 2 parts of log */
@@ -721,7 +724,7 @@ xlog_find_head(if ((error =xlog_find_verify_cycle (log, start_blk, num_scan_bblks - (int )head_blk, (stop_on_cycle -1 ), &new_blk))) - goto bp_err; + goto out_free_buffer;if (new_blk !=-1 ) { head_blk = new_blk; goto validate_head;
@@ -737,7 +740,7 @@ xlog_find_head(if ((error =xlog_find_verify_cycle (log, start_blk, (int )head_blk, stop_on_cycle, &new_blk))) - goto bp_err; + goto out_free_buffer;if (new_blk !=-1 ) head_blk = new_blk; }
@@ -756,13 +759,13 @@ xlog_find_head(if (error ==1 ) error = -EIO ;if (error) - goto bp_err; + goto out_free_buffer; }else { start_blk =0 ;ASSERT (head_blk <=INT_MAX ); error =xlog_find_verify_log_record (log, start_blk, &head_blk,0 );if (error <0 ) - goto bp_err; + goto out_free_buffer;if (error ==1 ) {/* We hit the beginning of the log during our search */ start_blk = log_bbnum - (num_scan_bblks - head_blk);
@@ -775,14 +778,14 @@ xlog_find_head(if (error ==1 ) error = -EIO ;if (error) - goto bp_err; + goto out_free_buffer;if (new_blk != log_bbnum) head_blk = new_blk; }else if (error) - goto bp_err; + goto out_free_buffer; } -kmem_free (bp); +kmem_free (buffer);if (head_blk == log_bbnum) *return_head_blk =0 ;else
@@ -795,9 +798,8 @@ xlog_find_head(*/ return 0 ; - bp_err: -kmem_free (bp); - +out_free_buffer: +kmem_free (buffer);if (error)xfs_warn (log->l_mp,"failed to find log head" );return error;
@@ -817,7 +819,7 @@ xlog_rseek_logrec_hdr(xfs_daddr_t head_blk, xfs_daddr_t tail_blk, int count, - char *bp, + char *buffer, xfs_daddr_t *rblk, struct xlog_rec_header **rhead, bool *wrapped)
@@ -836,7 +838,7 @@ xlog_rseek_logrec_hdr(*/ end_blk = head_blk > tail_blk ? tail_blk :0 ;for (i = (int ) head_blk -1 ; i >= end_blk; i--) { - error =xlog_bread (log, i,1 , bp, &offset); + error =xlog_bread (log, i,1 , buffer, &offset);if (error) goto out_error;
@@ -855,7 +857,7 @@ xlog_rseek_logrec_hdr(*/ if (tail_blk >= head_blk && found != count) {for (i = log->l_logBBsize -1 ; i >= (int ) tail_blk; i--) { - error =xlog_bread (log, i,1 , bp, &offset); + error =xlog_bread (log, i,1 , buffer, &offset);if (error) goto out_error;
@@ -891,7 +893,7 @@ xlog_seek_logrec_hdr(xfs_daddr_t head_blk, xfs_daddr_t tail_blk, int count, - char *bp, + char *buffer, xfs_daddr_t *rblk, struct xlog_rec_header **rhead, bool *wrapped)
@@ -910,7 +912,7 @@ xlog_seek_logrec_hdr(*/ end_blk = head_blk > tail_blk ? head_blk : log->l_logBBsize -1 ;for (i = (int ) tail_blk; i <= end_blk; i++) { - error =xlog_bread (log, i,1 , bp, &offset); + error =xlog_bread (log, i,1 , buffer, &offset);if (error) goto out_error;
@@ -928,7 +930,7 @@ xlog_seek_logrec_hdr(*/ if (tail_blk > head_blk && found != count) {for (i =0 ; i < (int ) head_blk; i++) { - error =xlog_bread (log, i,1 , bp, &offset); + error =xlog_bread (log, i,1 , buffer, &offset);if (error) goto out_error;
@@ -991,22 +993,22 @@ xlog_verify_tail( int hsize) {struct xlog_rec_header *thead; -char *bp; +char *buffer;xfs_daddr_t first_bad;int error =0 ;bool wrapped;xfs_daddr_t tmp_tail;xfs_daddr_t orig_tail = *tail_blk; - bp =xlog_get_bp (log,1 ); -if (!bp) + buffer =xlog_alloc_buffer (log,1 ); +if (!buffer)return -ENOMEM ;/* * Make sure the tail points to a record (returns positive count on * success). */ - error =xlog_seek_logrec_hdr (log, head_blk, *tail_blk,1 , bp, + error =xlog_seek_logrec_hdr (log, head_blk, *tail_blk,1 , buffer, &tmp_tail, &thead, &wrapped);if (error <0 ) goto out;
@@ -1035,8 +1037,8 @@ xlog_verify_tail(break ;/* skip to the next record; returns positive count on success */ - error =xlog_seek_logrec_hdr (log, head_blk, first_bad,2 , bp, - &tmp_tail, &thead, &wrapped); + error =xlog_seek_logrec_hdr (log, head_blk, first_bad,2 , + buffer, &tmp_tail, &thead, &wrapped);if (error <0 ) goto out;
@@ -1051,7 +1053,7 @@ xlog_verify_tail("Tail block (0x%llx) overwrite detected. Updated to 0x%llx" , orig_tail, *tail_blk); out: -kmem_free (bp); +kmem_free (buffer);return error; }
@@ -1073,13 +1075,13 @@ xlog_verify_head(struct xlog *log, xfs_daddr_t *head_blk, /* in/out: unverified head */ xfs_daddr_t *tail_blk, /* out: tail block */ - char *bp, + char *buffer, xfs_daddr_t *rhead_blk, /* start blk of last record */ struct xlog_rec_header **rhead, /* ptr to last record */ bool *wrapped) /* last rec. wraps phys. log */ { struct xlog_rec_header *tmp_rhead; -char *tmp_bp; +char *tmp_buffer;xfs_daddr_t first_bad;xfs_daddr_t tmp_rhead_blk;int found;
@@ -1090,15 +1092,15 @@ xlog_verify_head(* Check the head of the log for torn writes. Search backwards from the * head until we hit the tail or the maximum number of log record I/Os * that could have been in flight at one time. Use a temporary buffer so -* we don't trash the rhead/bp pointers from the caller. +* we don't trash the rhead/buffer pointers from the caller. */ - tmp_bp =xlog_get_bp (log,1 ); -if (!tmp_bp) + tmp_buffer =xlog_alloc_buffer (log,1 ); +if (!tmp_buffer)return -ENOMEM ; error =xlog_rseek_logrec_hdr (log, *head_blk, *tail_blk, -XLOG_MAX_ICLOGS , tmp_bp, &tmp_rhead_blk, - &tmp_rhead, &tmp_wrapped); -kmem_free (tmp_bp); +XLOG_MAX_ICLOGS , tmp_buffer, + &tmp_rhead_blk, &tmp_rhead, &tmp_wrapped); +kmem_free (tmp_buffer);if (error <0 )return error;
@@ -1127,8 +1129,8 @@ xlog_verify_head(* (i.e., the records with invalid CRC) if the cycle number * matches the the current cycle. */ - found =xlog_rseek_logrec_hdr (log, first_bad, *tail_blk,1 , bp, - rhead_blk, rhead, wrapped); + found =xlog_rseek_logrec_hdr (log, first_bad, *tail_blk,1 , + buffer, rhead_blk, rhead, wrapped);if (found <0 )return found;if (found ==0 )/* XXX: right thing to do here? */
@@ -1188,7 +1190,7 @@ xlog_check_unmount_rec(xfs_daddr_t *tail_blk, struct xlog_rec_header *rhead, xfs_daddr_t rhead_blk, - char *bp, + char *buffer, bool *clean) { struct xlog_op_header *op_head;
@@ -1231,7 +1233,7 @@ xlog_check_unmount_rec(if (*head_blk == after_umount_blk &&be32_to_cpu (rhead->h_num_logops) ==1 ) { umount_data_blk =xlog_wrap_logbno (log, rhead_blk + hblks); - error =xlog_bread (log, umount_data_blk,1 , bp, &offset); + error =xlog_bread (log, umount_data_blk,1 , buffer, &offset);if (error)return error;
@@ -1310,7 +1312,7 @@ xlog_find_tail( {xlog_rec_header_t *rhead;char *offset =NULL ; -char *bp; +char *buffer;int error;xfs_daddr_t rhead_blk;xfs_lsn_t tail_lsn;
@@ -1324,11 +1326,11 @@ xlog_find_tail(return error;ASSERT (*head_blk <INT_MAX ); - bp =xlog_get_bp (log,1 ); -if (!bp) + buffer =xlog_alloc_buffer (log,1 ); +if (!buffer)return -ENOMEM ;if (*head_blk ==0 ) {/* special case */ - error =xlog_bread (log,0 ,1 , bp, &offset); + error =xlog_bread (log,0 ,1 , buffer, &offset);if (error) goto done;
@@ -1344,7 +1346,7 @@ xlog_find_tail(* block. This wraps all the way back around to the head so something is * seriously wrong if we can't find it. */ - error =xlog_rseek_logrec_hdr (log, *head_blk, *head_blk,1 , bp, + error =xlog_rseek_logrec_hdr (log, *head_blk, *head_blk,1 , buffer, &rhead_blk, &rhead, &wrapped);if (error <0 )return error;
@@ -1365,7 +1367,7 @@ xlog_find_tail(* state to determine whether recovery is necessary. */ error =xlog_check_unmount_rec (log, head_blk, tail_blk, rhead, - rhead_blk, bp, &clean); + rhead_blk, buffer, &clean);if (error) goto done;
@@ -1382,7 +1384,7 @@ xlog_find_tail(if (!clean) {xfs_daddr_t orig_head = *head_blk; - error =xlog_verify_head (log, head_blk, tail_blk, bp, + error =xlog_verify_head (log, head_blk, tail_blk, buffer, &rhead_blk, &rhead, &wrapped);if (error) goto done;
@@ -1393,7 +1395,7 @@ xlog_find_tail( wrapped); tail_lsn =atomic64_read (&log->l_tail_lsn); error =xlog_check_unmount_rec (log, head_blk, tail_blk, - rhead, rhead_blk, bp, + rhead, rhead_blk, buffer, &clean);if (error) goto done;
@@ -1431,7 +1433,7 @@ xlog_find_tail( error =xlog_clear_stale_blocks (log, tail_lsn); done: -kmem_free (bp); +kmem_free (buffer);if (error)xfs_warn (log->l_mp,"failed to locate log tail" );
@@ -1459,7 +1461,7 @@ xlog_find_zeroed(struct xlog *log, xfs_daddr_t *blk_no) { - char *bp; + char *buffer;char *offset;uint first_cycle, last_cycle;xfs_daddr_t new_blk, last_blk, start_blk;
@@ -1469,35 +1471,36 @@ xlog_find_zeroed( *blk_no =0 ;/* check totally zeroed log */ - bp =xlog_get_bp (log,1 ); -if (!bp) + buffer =xlog_alloc_buffer (log,1 ); +if (!buffer)return -ENOMEM ; - error =xlog_bread (log,0 ,1 , bp, &offset); + error =xlog_bread (log,0 ,1 , buffer, &offset);if (error) - goto bp_err; + goto out_free_buffer; first_cycle =xlog_get_cycle (offset);if (first_cycle ==0 ) {/* completely zeroed log */ *blk_no =0 ; -kmem_free (bp); +kmem_free (buffer);return 1 ; }/* check partially zeroed log */ - error =xlog_bread (log, log_bbnum-1 ,1 , bp, &offset); + error =xlog_bread (log, log_bbnum-1 ,1 , buffer, &offset);if (error) - goto bp_err; + goto out_free_buffer; last_cycle =xlog_get_cycle (offset);if (last_cycle !=0 ) {/* log completely written to */ -kmem_free (bp); +kmem_free (buffer);return 0 ; }/* we have a partially zeroed log */ last_blk = log_bbnum-1 ; -if ((error =xlog_find_cycle_start (log, bp,0 , &last_blk,0 ))) - goto bp_err; + error =xlog_find_cycle_start (log, buffer,0 , &last_blk,0 ); +if (error) + goto out_free_buffer;/* * Validate the answer. Because there is no way to guarantee that
@@ -1520,7 +1523,7 @@ xlog_find_zeroed(*/ if ((error =xlog_find_verify_cycle (log, start_blk, (int )num_scan_bblks,0 , &new_blk))) - goto bp_err; + goto out_free_buffer;if (new_blk !=-1 ) last_blk = new_blk;
@@ -1532,11 +1535,11 @@ xlog_find_zeroed(if (error ==1 ) error = -EIO ;if (error) - goto bp_err; + goto out_free_buffer; *blk_no = last_blk; -bp_err: -kmem_free (bp); +out_free_buffer: +kmem_free (buffer);if (error)return error;return 1 ;
@@ -1579,7 +1582,7 @@ xlog_write_log_records( int tail_block) {char *offset; -char *bp; +char *buffer;int balign, ealign;int sectbb = log->l_sectBBsize;int end_block = start_block + blocks;
@@ -1596,7 +1599,7 @@ xlog_write_log_records( bufblks =1 <<ffs (blocks);while (bufblks > log->l_logBBsize) bufblks >>=1 ; -while (!(bp =xlog_get_bp (log, bufblks))) { +while (!(buffer =xlog_alloc_buffer (log, bufblks))) { bufblks >>=1 ;if (bufblks < sectbb)return -ENOMEM ;
@@ -1608,9 +1611,9 @@ xlog_write_log_records(*/ balign =round_down (start_block, sectbb);if (balign != start_block) { - error =xlog_bread_noalign (log, start_block,1 , bp); + error =xlog_bread_noalign (log, start_block,1 , buffer);if (error) - goto out_put_bp; + goto out_free_buffer; j = start_block - balign; }
@@ -1628,27 +1631,27 @@ xlog_write_log_records( ealign =round_down (end_block, sectbb);if (j ==0 && (start_block + endcount > ealign)) { error =xlog_bread_noalign (log, ealign, sectbb, - bp +BBTOB (ealign - start_block)); + buffer +BBTOB (ealign - start_block));if (error)break ; } - offset = bp +xlog_align (log, start_block); + offset = buffer +xlog_align (log, start_block);for (; j < endcount; j++) {xlog_add_record (log, offset, cycle, i+j, tail_cycle, tail_block); offset +=BBSIZE ; } - error =xlog_bwrite (log, start_block, endcount, bp); + error =xlog_bwrite (log, start_block, endcount, buffer);if (error)break ; start_block += endcount; j =0 ; } - out_put_bp: -kmem_free (bp); +out_free_buffer: +kmem_free (buffer);return error; }
@@ -5253,7 +5256,7 @@ xlog_do_recovery_pass(* iclog header and extract the header size from it. Get a * new hbp that is the correct size. */ - hbp =xlog_get_bp (log,1 ); + hbp =xlog_alloc_buffer (log,1 );if (!hbp)return -ENOMEM ;
@@ -5296,20 +5299,20 @@ xlog_do_recovery_pass(if (h_size %XLOG_HEADER_CYCLE_SIZE ) hblks++;kmem_free (hbp); - hbp =xlog_get_bp (log, hblks); + hbp =xlog_alloc_buffer (log, hblks); }else { hblks =1 ; } }else {ASSERT (log->l_sectBBsize ==1 ); hblks =1 ; - hbp =xlog_get_bp (log,1 ); + hbp =xlog_alloc_buffer (log,1 ); h_size =XLOG_BIG_RECORD_BSIZE ; }if (!hbp)return -ENOMEM ; - dbp =xlog_get_bp (log,BTOBB (h_size)); + dbp =xlog_alloc_buffer (log,BTOBB (h_size));if (!dbp) {kmem_free (hbp);return -ENOMEM ;
Signature · 1 line
2.20.1
Re: [PATCH 17/20] xfs: stop using bp naming for log recovery buffers
Quoted 5 lines · Christoph Hellwig
Now that we don't use struct
xfs_bufto hold log recovery buffer rename the related functions and variables to just talk of a buffer instead of using the bp name that we usually use forxfs_bufrelated functionality.Signed-off-by Christoph Hellwig <[email protected]>
Nice!
Signature · 2 lines
Dave Chinner [email protected]
PATCH 18/20 xfs: remove unused buffer cache APIs
In reply to Christoph HellwigNow that the log code uses bios directly we can drop various special cases in the buffer cache code.
--- fs/xfs/xfs_buf.c | 83 ++---------------------------------------------- fs/xfs/xfs_buf.h | 27 ---------------- 2 files changed, 2 insertions(+), 108 deletions(-)
fs/xfs/xfs_buf.c +2 -81
@@ -213,7 +213,7 @@ xfs_buf_free_maps( } } -struct xfs_buf * +static struct xfs_buf * _xfs_buf_alloc( struct xfs_buftarg *target, struct xfs_buf_map *map,
@@ -909,72 +909,6 @@ xfs_buf_read_uncached(return 0 ; } -/* -* Return a buffer allocated as an empty buffer and associated to external -* memory via xfs_buf_associate_memory() back to it's empty state. -*/ -void -xfs_buf_set_empty ( -struct xfs_buf *bp, -size_t numblks) -{ -if (bp->b_pages) -_xfs_buf_free_pages (bp); - - bp->b_pages =NULL ; - bp->b_page_count =0 ; - bp->b_addr =NULL ; - bp->b_length = numblks; - bp->b_io_length = numblks; - -ASSERT (bp->b_map_count ==1 ); - bp->b_bn =XFS_BUF_DADDR_NULL ; - bp->b_maps[0 ].bm_bn =XFS_BUF_DADDR_NULL ; - bp->b_maps[0 ].bm_len = bp->b_length; -} - -int -xfs_buf_associate_memory ( -xfs_buf_t *bp, -void *mem, -size_t len) -{ -int rval; -int i =0 ; -unsigned long pageaddr; -unsigned long offset; -size_t buflen; -int page_count; - - pageaddr = (unsigned long )mem &PAGE_MASK ; - offset = (unsigned long )mem - pageaddr; - buflen =PAGE_ALIGN (len + offset); - page_count = buflen >>PAGE_SHIFT ; - -/* Free any previous set of page pointers */ -if (bp->b_pages) -_xfs_buf_free_pages (bp); - - bp->b_pages =NULL ; - bp->b_addr = mem; - - rval =_xfs_buf_get_pages (bp, page_count); -if (rval) -return rval; - - bp->b_offset = offset; - -for (i =0 ; i < bp->b_page_count; i++) { - bp->b_pages[i] =kmem_to_page ((void *)pageaddr); - pageaddr +=PAGE_SIZE ; - } - - bp->b_io_length =BTOBB (len); - bp->b_length =BTOBB (buflen); - -return 0 ; -} -xfs_buf_t * xfs_buf_get_uncached( struct xfs_buftarg *target,
@@ -1258,7 +1192,7 @@ xfs_buf_ioend_async( struct xfs_buf *bp) {INIT_WORK (&bp->b_ioend_work, xfs_buf_ioend_work); -queue_work (bp->b_ioend_wq, &bp->b_ioend_work); +queue_work (bp->b_target->bt_mount->m_buf_workqueue, &bp->b_ioend_work); }void
@@ -1425,21 +1359,8 @@ _xfs_buf_ioapply(*/ bp->b_error =0 ; -/* -* Initialize the I/O completion workqueue if we haven't yet or the -* submitter has not opted to specify a custom one. -*/ -if (!bp->b_ioend_wq) - bp->b_ioend_wq = bp->b_target->bt_mount->m_buf_workqueue; -if (bp->b_flags &XBF_WRITE ) { op =REQ_OP_WRITE ; -if (bp->b_flags &XBF_SYNCIO ) - op_flags =REQ_SYNC ; -if (bp->b_flags &XBF_FUA ) - op_flags |=REQ_FUA ; -if (bp->b_flags &XBF_FLUSH ) - op_flags |=REQ_PREFLUSH ;/* * Run the write verifier callback function if it exists. If
fs/xfs/xfs_buf.h +0 -27
@@ -36,11 +36,6 @@ typedef enum {#define XBF_STALE (1 << 6)/* buffer has been staled, do not find it */ #define XBF_WRITE_FAIL (1 << 7)/* async writes have failed on this buffer */ -/* I/O hints for the BIO layer */ -#define XBF_SYNCIO (1 << 10)/* treat this buffer as synchronous I/O */ -#define XBF_FUA (1 << 11)/* force cache write through mode */ -#define XBF_FLUSH (1 << 12)/* flush the disk cache before a write */ -/* flags used only as arguments to access routines */ #define XBF_TRYLOCK (1 << 16)/* lock requested, but do not wait */ #define XBF_UNMAPPED (1 << 17)/* do not map the buffer */
@@ -61,9 +56,6 @@ typedef unsigned int xfs_buf_flags_t; { XBF_DONE, "DONE" }, \ { XBF_STALE, "STALE" }, \ { XBF_WRITE_FAIL, "WRITE_FAIL" }, \ - { XBF_SYNCIO, "SYNCIO" }, \ - { XBF_FUA, "FUA" }, \ - { XBF_FLUSH, "FLUSH" }, \ { XBF_TRYLOCK, "TRYLOCK" }, /* should never be set */\ { XBF_UNMAPPED, "UNMAPPED" }, /* ditto */\ { _XBF_PAGES, "PAGES" }, \
@@ -162,7 +154,6 @@ typedef struct xfs_buf {xfs_buftarg_t *b_target;/* buffer target (device) */ void *b_addr;/* virtual address of buffer */ struct work_struct b_ioend_work; -struct workqueue_struct *b_ioend_wq;/* I/O completion wq */ xfs_buf_iodone_t b_iodone;/* I/O completion function */ struct completion b_iowait;/* queue for I/O waiters */ void *b_log_item;
@@ -207,21 +198,6 @@ struct xfs_buf *xfs_buf_incore(struct xfs_buftarg *target, xfs_daddr_t blkno, size_t numblks, xfs_buf_flags_t flags); -struct xfs_buf *_xfs_buf_alloc (struct xfs_buftarg *target, -struct xfs_buf_map *map,int nmaps, -xfs_buf_flags_t flags); - -static inline struct xfs_buf * -xfs_buf_alloc ( -struct xfs_buftarg *target, -xfs_daddr_t blkno, -size_t numblks, -xfs_buf_flags_t flags) -{ -DEFINE_SINGLE_BUF_MAP (map, blkno, numblks); -return _xfs_buf_alloc (target, &map,1 , flags); -} -struct xfs_buf *xfs_buf_get_map (struct xfs_buftarg *target,struct xfs_buf_map *map,int nmaps,xfs_buf_flags_t flags);
@@ -267,9 +243,6 @@ xfs_buf_readahead(return xfs_buf_readahead_map (target, &map,1 , ops); } -void xfs_buf_set_empty (struct xfs_buf *bp,size_t numblks); -int xfs_buf_associate_memory (struct xfs_buf *bp,void *mem,size_t length); -struct xfs_buf *xfs_buf_get_uncached (struct xfs_buftarg *target,size_t numblks,int flags); int xfs_buf_read_uncached(struct xfs_buftarg *target, xfs_daddr_t daddr,
Signature · 1 line
2.20.1
Re: [PATCH 18/20] xfs: remove unused buffer cache APIs
Quoted 8 lines · Christoph Hellwig
Now that the log code uses bios directly we can drop various special cases in the buffer cache code.
Signed-off-by Christoph Hellwig <[email protected]>--- fs/xfs/xfs_buf.c | 83 ++---------------------------------------------- fs/xfs/xfs_buf.h | 27 ---------------- 2 files changed, 2 insertions(+), 108 deletions(-)
That's a nice result. One minor thing:
fs/xfs/xfs_buf.c xfs_buf_ioend_async( · line 1196
Quoted 7 lines
fs/xfs/xfs_buf.c +1
-1@@ -1258,7 +1192,7 @@ xfs_buf_ioend_async( struct xfs_buf *bp) {INIT_WORK (&bp->b_ioend_work, xfs_buf_ioend_work); -queue_work (bp->b_ioend_wq, &bp->b_ioend_work); +queue_work (bp->b_target->bt_mount->m_buf_workqueue, &bp->b_ioend_work); }
It'd be nice to keep bp->b_ioend_wq to avoid pointer chasing here. Perhaps we could set it up in _xfs_buf_alloc() where we are pretty much guaranteed to have the xfs_mount hot in cache, and then it's set for the life of the buffer. Just a thought, but either way:
Signature · 2 lines
Dave Chinner [email protected]
Re: [PATCH 18/20] xfs: remove unused buffer cache APIs
Quoted 4 lines · Dave Chinner
Quoted 4 lines
fs/xfs/xfs_buf.c +0
-0@@ -1258,7 +1192,7 @@ xfs_buf_ioend_async( struct xfs_buf *bp) {INIT_WORK (&bp->b_ioend_work, xfs_buf_ioend_work);
Quoted 8 lines
fs/xfs/xfs_buf.c xfs_buf_ioend_async( · line 1196
Quoted 3 lines
fs/xfs/xfs_buf.c +1
-1-queue_work (bp->b_ioend_wq, &bp->b_ioend_work); +queue_work (bp->b_target->bt_mount->m_buf_workqueue, &bp->b_ioend_work); }It'd be nice to keep
bp->b_ioend_wqto avoid pointer chasing here. Perhaps we could set it up in_xfs_buf_alloc()where we are pretty much guaranteed to have thexfs_mounthot in cache, and then it's set for the life of the buffer. Just a thought, but either way:
We'd need to benchmark this particular case, but the bloat to the xfs_buf structure probably cancels out at least one pointer dereference. That being said we probably could at least remove one indirection without bloating the buf, I'll take a look at that.
Re: [PATCH 18/20] xfs: remove unused buffer cache APIs
Quoted 14 lines · Christoph Hellwig
Quoted 4 lines · Dave Chinner
Quoted 4 lines
fs/xfs/xfs_buf.c +0
-0@@ -1258,7 +1192,7 @@ xfs_buf_ioend_async( struct xfs_buf *bp) {INIT_WORK (&bp->b_ioend_work, xfs_buf_ioend_work);Quoted 8 lines
fs/xfs/xfs_buf.c xfs_buf_ioend_async( · line 1196
Quoted 3 lines
fs/xfs/xfs_buf.c +1
-1-queue_work (bp->b_ioend_wq, &bp->b_ioend_work); +queue_work (bp->b_target->bt_mount->m_buf_workqueue, &bp->b_ioend_work); }It'd be nice to keep
bp->b_ioend_wqto avoid pointer chasing here. Perhaps we could set it up in_xfs_buf_alloc()where we are pretty much guaranteed to have thexfs_mounthot in cache, and then it's set for the life of the buffer. Just a thought, but either way:
Quoted 4 lines
We'd need to benchmark this particular case, but the bloat to the
xfs_bufstructure probably cancels out at least one pointer dereference. That being said we probably could at least remove one indirection without bloating the buf, I'll take a look at that.
FWIW, I already plan to move the buf workqueue to the xfs_buftarg so maybe that's the way to go here...
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
PATCH 19/20 xfs: properly type the b_log_item field in struct xfs_buf
In reply to Christoph HellwigNow that the log code doesn't abuse this field any more we can declare it as a struct xfs_buf_log_item pointer.
--- fs/xfs/xfs_buf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/xfs/xfs_buf.h +1 -1
@@ -156,7 +156,7 @@ typedef struct xfs_buf {struct work_struct b_ioend_work;xfs_buf_iodone_t b_iodone;/* I/O completion function */ struct completion b_iowait;/* queue for I/O waiters */ -void *b_log_item; +struct xfs_buf_log_item *b_log_item;struct list_head b_li_list;/* Log items list head */ struct xfs_trans *b_transp;struct page **b_pages;/* array of page pointers */
Signature · 1 line
2.20.1
Re: [PATCH 19/20] xfs: properly type the b_log_item field in struct xfs_buf
Quoted 4 lines · Christoph Hellwig
Now that the log code doesn't abuse this field any more we can declare it as a struct
xfs_buf_log_itempointer.Signed-off-by Christoph Hellwig <[email protected]>
*nod*
Signature · 2 lines
Dave Chinner [email protected]
PATCH 20/20 xfs: remove the b_io_length field in struct xfs_buf
In reply to Christoph HellwigThis field is now always idential to b_length.
--- fs/xfs/xfs_buf.c | 5 ++--- fs/xfs/xfs_buf.h | 1 - fs/xfs/xfs_log_recover.c | 9 ++++----- 3 files changed, 6 insertions(+), 9 deletions(-)
fs/xfs/xfs_buf.c +2 -3
@@ -263,7 +263,6 @@ _xfs_buf_alloc( bp->b_maps[i].bm_len = map[i].bm_len; bp->b_length += map[i].bm_len; } - bp->b_io_length = bp->b_length;atomic_set (&bp->b_pin_count,0 );init_waitqueue_head (&bp->b_waiters);
@@ -1407,7 +1406,7 @@ _xfs_buf_ioapply(* subsequent call. */ offset = bp->b_offset; - size =BBTOB (bp->b_io_length); + size =BBTOB (bp->b_length);blk_start_plug (&plug);for (i =0 ; i < bp->b_map_count; i++) {xfs_buf_ioapply_map (bp, i, &offset, &size, op, op_flags);
@@ -1545,7 +1544,7 @@ xfs_buf_iomove( page_offset = (boff + bp->b_offset) & ~PAGE_MASK ; page = bp->b_pages[page_index]; csize =min_t (size_t,PAGE_SIZE - page_offset, -BBTOB (bp->b_io_length) - boff); +BBTOB (bp->b_length) - boff);ASSERT ((csize + page_offset) <=PAGE_SIZE );
fs/xfs/xfs_buf.h +0 -1
@@ -164,7 +164,6 @@ typedef struct xfs_buf {struct xfs_buf_map *b_maps;/* compound buffer map */ struct xfs_buf_map __b_map;/* inline compound buffer map */ int b_map_count; -int b_io_length;/* IO size in BBs */ atomic_t b_pin_count;/* pin count */ atomic_t b_io_remaining;/* #outstanding I/O requests */ unsigned int b_page_count;/* size of page array */
fs/xfs/xfs_log_recover.c +4 -5
@@ -2086,7 +2086,7 @@ xlog_recover_do_inode_buffer(if (xfs_sb_version_hascrc (&mp->m_sb)) bp->b_ops = &xfs_inode_buf_ops; - inodes_per_buf =BBTOB (bp->b_io_length) >> mp->m_sb.sb_inodelog; + inodes_per_buf =BBTOB (bp->b_length) >> mp->m_sb.sb_inodelog;for (i =0 ; i < inodes_per_buf; i++) { next_unlinked_offset = (i * mp->m_sb.sb_inodesize) + offsetof(xfs_dinode_t , di_next_unlinked);
@@ -2128,8 +2128,7 @@ xlog_recover_do_inode_buffer(ASSERT (item->ri_buf[item_index].i_addr !=NULL );ASSERT ((item->ri_buf[item_index].i_len %XFS_BLF_CHUNK ) ==0 ); -ASSERT ((reg_buf_offset + reg_buf_bytes) <= -BBTOB (bp->b_io_length)); +ASSERT ((reg_buf_offset + reg_buf_bytes) <=BBTOB (bp->b_length));/* * The current logged region contains a copy of the
@@ -2594,7 +2593,7 @@ xlog_recover_do_reg_buffer(ASSERT (nbits >0 );ASSERT (item->ri_buf[i].i_addr !=NULL );ASSERT (item->ri_buf[i].i_len %XFS_BLF_CHUNK ==0 ); -ASSERT (BBTOB (bp->b_io_length) >= +ASSERT (BBTOB (bp->b_length) >= ((uint )bit <<XFS_BLF_SHIFT ) + (nbits <<XFS_BLF_SHIFT ));/*
@@ -2817,7 +2816,7 @@ xlog_recover_buffer_pass2(*/ if (XFS_DINODE_MAGIC ==be16_to_cpu (*((__be16 *)xfs_buf_offset (bp,0 ))) && - (BBTOB (bp->b_io_length) !=max (log->l_mp->m_sb.sb_blocksize, + (BBTOB (bp->b_length) !=max (log->l_mp->m_sb.sb_blocksize, (uint32_t )log->l_mp->m_inode_cluster_size))) {xfs_buf_stale (bp); error =xfs_bwrite (bp);
Signature · 1 line
2.20.1
Re: [PATCH 20/20] xfs: remove the b_io_length field in struct xfs_buf
Quoted 8 lines · Christoph Hellwig
This field is now always idential to
b_length.Signed-off-by Christoph Hellwig <[email protected]>--- fs/xfs/xfs_buf.c | 5 ++--- fs/xfs/xfs_buf.h | 1 - fs/xfs/xfs_log_recover.c | 9 ++++----- 3 files changed, 6 insertions(+), 9 deletions(-)
One less size field to confuse me.i Looks like most of the remaining uses of that variable should have been using b_length in the first place, so this is good to finally have cleaned up.
Signature · 2 lines
Dave Chinner [email protected]
Quoted 6 lines · Christoph Hellwig
Hi all,
this series switches the log writing and log recovery code to use bios directly, and remove various special cases from the buffer cache code. Note that I have developed it on top of the previous series of log item related cleanups, so if you don't have that applied there is a small
Hmm, /I/ don't have that applied. :/
Can you resend that series in its current form with (or without) all the suggested review cleanups, please? :)
--D
Quoted 19 lines
conflict. To make life easier I have pushed out a git branche here:
git://git.infradead.org/users/hch/xfs.gitxfs-log-bioGitweb:
http://git.infradead.org/users/hch/xfs.git/shortlog/refs/heads/xfs-log-bio
Changes since v2:
- rename the 'flush' flag to 'need_flush'
- spelling fixes
- minor cleanupsChanges since v1:
- rebased to not required the log item series first
- split the bio-related code out ofxfs_log_recover.cinto a new file to ease usingxfs_log_recover.cin xfsprogs
- usekmem_alloc_largeinstead of vmalloc to allocate the buffer
- additional minor cleanups
Quoted 8 lines · Darrick J. Wong
Quoted 6 lines · Christoph Hellwig
Hi all,
this series switches the log writing and log recovery code to use bios directly, and remove various special cases from the buffer cache code. Note that I have developed it on top of the previous series of log item related cleanups, so if you don't have that applied there is a small
Quoted 4 lines
Hmm, /I/ don't have that applied. :/
Can you resend that series in its current form with (or without) all the suggested review cleanups, please? :)
Actually that sentence above is stale. It applied to v1, but not v2 or this v3.
Quoted 9 lines · Christoph Hellwig
Quoted 8 lines · Darrick J. Wong
Quoted 6 lines · Christoph Hellwig
Hi all,
this series switches the log writing and log recovery code to use bios directly, and remove various special cases from the buffer cache code. Note that I have developed it on top of the previous series of log item related cleanups, so if you don't have that applied there is a small
Quoted 7 lines
Quoted 4 lines
Hmm, /I/ don't have that applied. :/
Can you resend that series in its current form with (or without) all the suggested review cleanups, please? :)
Actually that sentence above is stale. It applied to v1, but not v2 or this v3.
I assume you're going to resubmit the other log item cleanup series at some point, right? :)
--D
Quoted 2 lines · Darrick J. Wong
I assume you're going to resubmit the other log item cleanup series at some point, right? :)
Yes. I've been waiting for Brian to finish reviewing the remaining patches before resending it.
Quoted 6 lines · Christoph Hellwig
Quoted 2 lines · Darrick J. Wong
I assume you're going to resubmit the other log item cleanup series at some point, right? :)
Yes. I've been waiting for Brian to finish reviewing the remaining patches before resending it.
I thought I made it through the last post of that series...
Brian
Quoted 9 lines · Brian Foster
Quoted 6 lines · Christoph Hellwig
Quoted 2 lines · Darrick J. Wong
I assume you're going to resubmit the other log item cleanup series at some point, right? :)
Yes. I've been waiting for Brian to finish reviewing the remaining patches before resending it.
I thought I made it through the last post of that series...
Oh, ok. I'll find some time to respin it then.