* [PATCH v3 1/5] xfs: use b_maps[] for discontiguous buffers
2012-12-04 23:18 [PATCH v3 0/5] discontiguous buffer patches Mark Tinguely
@ 2012-12-04 23:18 ` Mark Tinguely
2012-12-08 12:23 ` Christoph Hellwig
2012-12-04 23:18 ` [PATCH v3 2/5] xfs: rename bli_format to avoid confusion with bli_formats Mark Tinguely
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Mark Tinguely @ 2012-12-04 23:18 UTC (permalink / raw)
To: xfs
[-- Attachment #1: v3-1-3-xfs-use-b_maps-for-discontiguous-buffers.patch --]
[-- Type: text/plain, Size: 3482 bytes --]
Commits starting at 77c1a08 introduced a multiple segment support
to xfs_buf. xfs_trans_buf_item_match() could not find a multi-segment
buffer in the transaction because it was looking at the single segment
block number rather than the multi-segment b_maps[0].bm.bn. This
results on a recursive buffer lock that can never be satisfied.
This patch:
1) Changed the remaining b_map accesses to be b_maps[0] accesses.
2) Renames the single segment b_map structure to __b_map to avoid
future confusion.
Signed-off-by: Mark Tinguely <tinguely@sgi.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/xfs_buf.c | 12 ++++++------
fs/xfs/xfs_buf.h | 6 +++---
2 files changed, 9 insertions(+), 9 deletions(-)
Index: b/fs/xfs/xfs_buf.c
===================================================================
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -175,7 +175,7 @@ xfs_buf_get_maps(
bp->b_map_count = map_count;
if (map_count == 1) {
- bp->b_maps = &bp->b_map;
+ bp->b_maps = &bp->__b_map;
return 0;
}
@@ -193,7 +193,7 @@ static void
xfs_buf_free_maps(
struct xfs_buf *bp)
{
- if (bp->b_maps != &bp->b_map) {
+ if (bp->b_maps != &bp->__b_map) {
kmem_free(bp->b_maps);
bp->b_maps = NULL;
}
@@ -377,8 +377,8 @@ xfs_buf_allocate_memory(
}
use_alloc_page:
- start = BBTOB(bp->b_map.bm_bn) >> PAGE_SHIFT;
- end = (BBTOB(bp->b_map.bm_bn + bp->b_length) + PAGE_SIZE - 1)
+ start = BBTOB(bp->b_maps[0].bm_bn) >> PAGE_SHIFT;
+ end = (BBTOB(bp->b_maps[0].bm_bn + bp->b_length) + PAGE_SIZE - 1)
>> PAGE_SHIFT;
page_count = end - start;
error = _xfs_buf_get_pages(bp, page_count, flags);
@@ -640,7 +640,7 @@ _xfs_buf_read(
xfs_buf_flags_t flags)
{
ASSERT(!(flags & XBF_WRITE));
- ASSERT(bp->b_map.bm_bn != XFS_BUF_DADDR_NULL);
+ ASSERT(bp->b_maps[0].bm_bn != XFS_BUF_DADDR_NULL);
bp->b_flags &= ~(XBF_WRITE | XBF_ASYNC | XBF_READ_AHEAD);
bp->b_flags |= flags & (XBF_READ | XBF_ASYNC | XBF_READ_AHEAD);
@@ -1709,7 +1709,7 @@ xfs_buf_cmp(
struct xfs_buf *bp = container_of(b, struct xfs_buf, b_list);
xfs_daddr_t diff;
- diff = ap->b_map.bm_bn - bp->b_map.bm_bn;
+ diff = ap->b_maps[0].bm_bn - bp->b_maps[0].bm_bn;
if (diff < 0)
return -1;
if (diff > 0)
Index: b/fs/xfs/xfs_buf.h
===================================================================
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -151,7 +151,7 @@ typedef struct xfs_buf {
struct page **b_pages; /* array of page pointers */
struct page *b_page_array[XB_PAGES]; /* inline pages */
struct xfs_buf_map *b_maps; /* compound buffer map */
- struct xfs_buf_map b_map; /* inline 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 */
@@ -330,8 +330,8 @@ void xfs_buf_stale(struct xfs_buf *bp);
* In future, uncached buffers will pass the block number directly to the io
* request function and hence these macros will go away at that point.
*/
-#define XFS_BUF_ADDR(bp) ((bp)->b_map.bm_bn)
-#define XFS_BUF_SET_ADDR(bp, bno) ((bp)->b_map.bm_bn = (xfs_daddr_t)(bno))
+#define XFS_BUF_ADDR(bp) ((bp)->b_maps[0].bm_bn)
+#define XFS_BUF_SET_ADDR(bp, bno) ((bp)->b_maps[0].bm_bn = (xfs_daddr_t)(bno))
static inline void xfs_buf_set_ref(struct xfs_buf *bp, int lru_ref)
{
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 1/5] xfs: use b_maps[] for discontiguous buffers
2012-12-04 23:18 ` [PATCH v3 1/5] xfs: use b_maps[] for discontiguous buffers Mark Tinguely
@ 2012-12-08 12:23 ` Christoph Hellwig
0 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2012-12-08 12:23 UTC (permalink / raw)
To: Mark Tinguely; +Cc: xfs
On Tue, Dec 04, 2012 at 05:18:02PM -0600, Mark Tinguely wrote:
> Commits starting at 77c1a08 introduced a multiple segment support
> to xfs_buf. xfs_trans_buf_item_match() could not find a multi-segment
> buffer in the transaction because it was looking at the single segment
> block number rather than the multi-segment b_maps[0].bm.bn. This
> results on a recursive buffer lock that can never be satisfied.
>
> This patch:
> 1) Changed the remaining b_map accesses to be b_maps[0] accesses.
> 2) Renames the single segment b_map structure to __b_map to avoid
> future confusion.
>
> Signed-off-by: Mark Tinguely <tinguely@sgi.com>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 2/5] xfs: rename bli_format to avoid confusion with bli_formats
2012-12-04 23:18 [PATCH v3 0/5] discontiguous buffer patches Mark Tinguely
2012-12-04 23:18 ` [PATCH v3 1/5] xfs: use b_maps[] for discontiguous buffers Mark Tinguely
@ 2012-12-04 23:18 ` Mark Tinguely
2012-12-08 12:23 ` Christoph Hellwig
2012-12-04 23:18 ` [PATCH v3 3/5] xfs: fix segment in xfs_buf_item_format_segment Mark Tinguely
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Mark Tinguely @ 2012-12-04 23:18 UTC (permalink / raw)
To: xfs
[-- Attachment #1: v3-2a-rename-bli_format.patch --]
[-- Type: text/plain, Size: 7331 bytes --]
Rename the bli_format structure to __bli_format to avoid
accidently confusing them with the bli_formats pointer.
Signed-off-by: Mark Tinguely <tinguely@sgi.com>
---
fs/xfs/xfs_buf_item.c | 22 +++++++++++-----------
fs/xfs/xfs_buf_item.h | 2 +-
fs/xfs/xfs_trans_buf.c | 24 ++++++++++++------------
3 files changed, 24 insertions(+), 24 deletions(-)
Index: b/fs/xfs/xfs_buf_item.c
===================================================================
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -71,7 +71,7 @@ xfs_buf_item_log_debug(
chunk_num = byte >> XFS_BLF_SHIFT;
word_num = chunk_num >> BIT_TO_WORD_SHIFT;
bit_num = chunk_num & (NBWORD - 1);
- wordp = &(bip->bli_format.blf_data_map[word_num]);
+ wordp = &(bip->__bli_format.blf_data_map[word_num]);
bit_set = *wordp & (1 << bit_num);
ASSERT(bit_set);
byte++;
@@ -237,7 +237,7 @@ xfs_buf_item_size(
* cancel flag in it.
*/
trace_xfs_buf_item_size_stale(bip);
- ASSERT(bip->bli_format.blf_flags & XFS_BLF_CANCEL);
+ ASSERT(bip->__bli_format.blf_flags & XFS_BLF_CANCEL);
return bip->bli_format_count;
}
@@ -278,7 +278,7 @@ xfs_buf_item_format_segment(
uint buffer_offset;
/* copy the flags across from the base format item */
- blfp->blf_flags = bip->bli_format.blf_flags;
+ blfp->blf_flags = bip->__bli_format.blf_flags;
/*
* Base size is the actual size of the ondisk structure - it reflects
@@ -371,7 +371,7 @@ xfs_buf_item_format_segment(
nbits++;
}
}
- bip->bli_format.blf_size = nvecs;
+ bip->__bli_format.blf_size = nvecs;
return vecp;
}
@@ -405,7 +405,7 @@ xfs_buf_item_format(
if (bip->bli_flags & XFS_BLI_INODE_BUF) {
if (!((bip->bli_flags & XFS_BLI_INODE_ALLOC_BUF) &&
xfs_log_item_in_current_chkpt(lip)))
- bip->bli_format.blf_flags |= XFS_BLF_INODE_BUF;
+ bip->__bli_format.blf_flags |= XFS_BLF_INODE_BUF;
bip->bli_flags &= ~XFS_BLI_INODE_BUF;
}
@@ -485,7 +485,7 @@ xfs_buf_item_unpin(
ASSERT(bip->bli_flags & XFS_BLI_STALE);
ASSERT(xfs_buf_islocked(bp));
ASSERT(XFS_BUF_ISSTALE(bp));
- ASSERT(bip->bli_format.blf_flags & XFS_BLF_CANCEL);
+ ASSERT(bip->__bli_format.blf_flags & XFS_BLF_CANCEL);
trace_xfs_buf_item_unpin_stale(bip);
@@ -631,7 +631,7 @@ xfs_buf_item_unlock(
*/
if (bip->bli_flags & XFS_BLI_STALE) {
trace_xfs_buf_item_unlock_stale(bip);
- ASSERT(bip->bli_format.blf_flags & XFS_BLF_CANCEL);
+ ASSERT(bip->__bli_format.blf_flags & XFS_BLF_CANCEL);
if (!aborted) {
atomic_dec(&bip->bli_refcount);
return;
@@ -644,8 +644,8 @@ xfs_buf_item_unlock(
* If the buf item isn't tracking any data, free it, otherwise drop the
* reference we hold to it.
*/
- if (xfs_bitmap_empty(bip->bli_format.blf_data_map,
- bip->bli_format.blf_map_size))
+ if (xfs_bitmap_empty(bip->__bli_format.blf_data_map,
+ bip->__bli_format.blf_map_size))
xfs_buf_item_relse(bp);
else
atomic_dec(&bip->bli_refcount);
@@ -716,7 +716,7 @@ xfs_buf_item_get_format(
bip->bli_format_count = count;
if (count == 1) {
- bip->bli_formats = &bip->bli_format;
+ bip->bli_formats = &bip->__bli_format;
return 0;
}
@@ -731,7 +731,7 @@ STATIC void
xfs_buf_item_free_format(
struct xfs_buf_log_item *bip)
{
- if (bip->bli_formats != &bip->bli_format) {
+ if (bip->bli_formats != &bip->__bli_format) {
kmem_free(bip->bli_formats);
bip->bli_formats = NULL;
}
Index: b/fs/xfs/xfs_buf_item.h
===================================================================
--- a/fs/xfs/xfs_buf_item.h
+++ b/fs/xfs/xfs_buf_item.h
@@ -104,7 +104,7 @@ typedef struct xfs_buf_log_item {
#endif
int bli_format_count; /* count of headers */
struct xfs_buf_log_format *bli_formats; /* array of in-log header ptrs */
- struct xfs_buf_log_format bli_format; /* embedded in-log header */
+ struct xfs_buf_log_format __bli_format; /* embedded in-log header */
} xfs_buf_log_item_t;
void xfs_buf_item_init(struct xfs_buf *, struct xfs_mount *);
Index: b/fs/xfs/xfs_trans_buf.c
===================================================================
--- a/fs/xfs/xfs_trans_buf.c
+++ b/fs/xfs/xfs_trans_buf.c
@@ -93,7 +93,7 @@ _xfs_trans_bjoin(
xfs_buf_item_init(bp, tp->t_mountp);
bip = bp->b_fspriv;
ASSERT(!(bip->bli_flags & XFS_BLI_STALE));
- ASSERT(!(bip->bli_format.blf_flags & XFS_BLF_CANCEL));
+ ASSERT(!(bip->__bli_format.blf_flags & XFS_BLF_CANCEL));
ASSERT(!(bip->bli_flags & XFS_BLI_LOGGED));
if (reset_recur)
bip->bli_recur = 0;
@@ -432,7 +432,7 @@ xfs_trans_brelse(xfs_trans_t *tp,
bip = bp->b_fspriv;
ASSERT(bip->bli_item.li_type == XFS_LI_BUF);
ASSERT(!(bip->bli_flags & XFS_BLI_STALE));
- ASSERT(!(bip->bli_format.blf_flags & XFS_BLF_CANCEL));
+ ASSERT(!(bip->__bli_format.blf_flags & XFS_BLF_CANCEL));
ASSERT(atomic_read(&bip->bli_refcount) > 0);
trace_xfs_trans_brelse(bip);
@@ -519,7 +519,7 @@ xfs_trans_bhold(xfs_trans_t *tp,
ASSERT(bp->b_transp == tp);
ASSERT(bip != NULL);
ASSERT(!(bip->bli_flags & XFS_BLI_STALE));
- ASSERT(!(bip->bli_format.blf_flags & XFS_BLF_CANCEL));
+ ASSERT(!(bip->__bli_format.blf_flags & XFS_BLF_CANCEL));
ASSERT(atomic_read(&bip->bli_refcount) > 0);
bip->bli_flags |= XFS_BLI_HOLD;
@@ -539,7 +539,7 @@ xfs_trans_bhold_release(xfs_trans_t *tp,
ASSERT(bp->b_transp == tp);
ASSERT(bip != NULL);
ASSERT(!(bip->bli_flags & XFS_BLI_STALE));
- ASSERT(!(bip->bli_format.blf_flags & XFS_BLF_CANCEL));
+ ASSERT(!(bip->__bli_format.blf_flags & XFS_BLF_CANCEL));
ASSERT(atomic_read(&bip->bli_refcount) > 0);
ASSERT(bip->bli_flags & XFS_BLI_HOLD);
@@ -598,7 +598,7 @@ xfs_trans_log_buf(xfs_trans_t *tp,
bip->bli_flags &= ~XFS_BLI_STALE;
ASSERT(XFS_BUF_ISSTALE(bp));
XFS_BUF_UNSTALE(bp);
- bip->bli_format.blf_flags &= ~XFS_BLF_CANCEL;
+ bip->__bli_format.blf_flags &= ~XFS_BLF_CANCEL;
}
tp->t_flags |= XFS_TRANS_DIRTY;
@@ -657,8 +657,8 @@ xfs_trans_binval(
*/
ASSERT(XFS_BUF_ISSTALE(bp));
ASSERT(!(bip->bli_flags & (XFS_BLI_LOGGED | XFS_BLI_DIRTY)));
- ASSERT(!(bip->bli_format.blf_flags & XFS_BLF_INODE_BUF));
- ASSERT(bip->bli_format.blf_flags & XFS_BLF_CANCEL);
+ ASSERT(!(bip->__bli_format.blf_flags & XFS_BLF_INODE_BUF));
+ ASSERT(bip->__bli_format.blf_flags & XFS_BLF_CANCEL);
ASSERT(bip->bli_item.li_desc->lid_flags & XFS_LID_DIRTY);
ASSERT(tp->t_flags & XFS_TRANS_DIRTY);
return;
@@ -668,10 +668,10 @@ xfs_trans_binval(
bip->bli_flags |= XFS_BLI_STALE;
bip->bli_flags &= ~(XFS_BLI_INODE_BUF | XFS_BLI_LOGGED | XFS_BLI_DIRTY);
- bip->bli_format.blf_flags &= ~XFS_BLF_INODE_BUF;
- bip->bli_format.blf_flags |= XFS_BLF_CANCEL;
- memset((char *)(bip->bli_format.blf_data_map), 0,
- (bip->bli_format.blf_map_size * sizeof(uint)));
+ bip->__bli_format.blf_flags &= ~XFS_BLF_INODE_BUF;
+ bip->__bli_format.blf_flags |= XFS_BLF_CANCEL;
+ memset((char *)(bip->__bli_format.blf_data_map), 0,
+ (bip->__bli_format.blf_map_size * sizeof(uint)));
bip->bli_item.li_desc->lid_flags |= XFS_LID_DIRTY;
tp->t_flags |= XFS_TRANS_DIRTY;
}
@@ -775,5 +775,5 @@ xfs_trans_dquot_buf(
type == XFS_BLF_GDQUOT_BUF);
ASSERT(atomic_read(&bip->bli_refcount) > 0);
- bip->bli_format.blf_flags |= type;
+ bip->__bli_format.blf_flags |= type;
}
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v3 3/5] xfs: fix segment in xfs_buf_item_format_segment
2012-12-04 23:18 [PATCH v3 0/5] discontiguous buffer patches Mark Tinguely
2012-12-04 23:18 ` [PATCH v3 1/5] xfs: use b_maps[] for discontiguous buffers Mark Tinguely
2012-12-04 23:18 ` [PATCH v3 2/5] xfs: rename bli_format to avoid confusion with bli_formats Mark Tinguely
@ 2012-12-04 23:18 ` Mark Tinguely
2012-12-08 12:29 ` Christoph Hellwig
2012-12-04 23:18 ` [PATCH v3 4/5] xfs: fix the multi-segment log buffer format Mark Tinguely
2012-12-04 23:18 ` [PATCH v3 5/5] xfs remove the XFS_TRANS_DEBUG routines Mark Tinguely
4 siblings, 1 reply; 13+ messages in thread
From: Mark Tinguely @ 2012-12-04 23:18 UTC (permalink / raw)
To: xfs
[-- Attachment #1: v3-2b-handle-non-busy-segments.patch --]
[-- Type: text/plain, Size: 1858 bytes --]
Not every segment in a multi-segment buffer is dirty in a
transaction and they will not be outputted. The assert in
xfs_buf_item_format_segment() that checks for the at least
one chunk of data in the segment to be used is not necessary
true for multi-segmented buffers.
Signed-off-by: Mark Tinguely <tinguely@sgi.com>
---
fs/xfs/xfs_buf_item.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
Index: b/fs/xfs/xfs_buf_item.c
===================================================================
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -287,6 +287,17 @@ xfs_buf_item_format_segment(
*/
base_size = offsetof(struct xfs_buf_log_format, blf_data_map) +
(blfp->blf_map_size * sizeof(blfp->blf_data_map[0]));
+
+ nvecs = 0;
+ first_bit = xfs_next_bit(blfp->blf_data_map, blfp->blf_map_size, 0);
+ if (!(bip->bli_flags & XFS_BLI_STALE) && first_bit == -1) {
+ /*
+ * If the map is not be dirty in the transaction, mark
+ * the size as zero and do not advance the vector pointer.
+ */
+ goto out;
+ }
+
vecp->i_addr = blfp;
vecp->i_len = base_size;
vecp->i_type = XLOG_REG_TYPE_BFORMAT;
@@ -301,15 +312,13 @@ xfs_buf_item_format_segment(
*/
trace_xfs_buf_item_format_stale(bip);
ASSERT(blfp->blf_flags & XFS_BLF_CANCEL);
- blfp->blf_size = nvecs;
- return vecp;
+ goto out;
}
/*
* Fill in an iovec for each set of contiguous chunks.
*/
- first_bit = xfs_next_bit(blfp->blf_data_map, blfp->blf_map_size, 0);
- ASSERT(first_bit != -1);
+
last_bit = first_bit;
nbits = 1;
for (;;) {
@@ -371,7 +380,8 @@ xfs_buf_item_format_segment(
nbits++;
}
}
- bip->__bli_format.blf_size = nvecs;
+out:
+ blfp->blf_size = nvecs;
return vecp;
}
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 3/5] xfs: fix segment in xfs_buf_item_format_segment
2012-12-04 23:18 ` [PATCH v3 3/5] xfs: fix segment in xfs_buf_item_format_segment Mark Tinguely
@ 2012-12-08 12:29 ` Christoph Hellwig
2012-12-10 1:34 ` Dave Chinner
0 siblings, 1 reply; 13+ messages in thread
From: Christoph Hellwig @ 2012-12-08 12:29 UTC (permalink / raw)
To: Mark Tinguely; +Cc: xfs
On Tue, Dec 04, 2012 at 05:18:04PM -0600, Mark Tinguely wrote:
> Not every segment in a multi-segment buffer is dirty in a
> transaction and they will not be outputted. The assert in
> xfs_buf_item_format_segment() that checks for the at least
> one chunk of data in the segment to be used is not necessary
> true for multi-segmented buffers.
>
> Signed-off-by: Mark Tinguely <tinguely@sgi.com>
> ---
> fs/xfs/xfs_buf_item.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> Index: b/fs/xfs/xfs_buf_item.c
> ===================================================================
> --- a/fs/xfs/xfs_buf_item.c
> +++ b/fs/xfs/xfs_buf_item.c
> @@ -287,6 +287,17 @@ xfs_buf_item_format_segment(
> */
> base_size = offsetof(struct xfs_buf_log_format, blf_data_map) +
> (blfp->blf_map_size * sizeof(blfp->blf_data_map[0]));
> +
> + nvecs = 0;
> + first_bit = xfs_next_bit(blfp->blf_data_map, blfp->blf_map_size, 0);
> + if (!(bip->bli_flags & XFS_BLI_STALE) && first_bit == -1) {
> + /*
> + * If the map is not be dirty in the transaction, mark
> + * the size as zero and do not advance the vector pointer.
> + */
> + goto out;
> + }
> +
I don't really like the style of this check. What's the problem of
doing it this way:
1) fill out the first vecp
2) do the stale check as-is
3) handle the the first_bit == -1 case ala:
if (first_bit == -1) {
blfp->blf_size = 0;
return vecp;
}
4) only then increcement vecp
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 3/5] xfs: fix segment in xfs_buf_item_format_segment
2012-12-08 12:29 ` Christoph Hellwig
@ 2012-12-10 1:34 ` Dave Chinner
0 siblings, 0 replies; 13+ messages in thread
From: Dave Chinner @ 2012-12-10 1:34 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Mark Tinguely, xfs
On Sat, Dec 08, 2012 at 07:29:21AM -0500, Christoph Hellwig wrote:
> On Tue, Dec 04, 2012 at 05:18:04PM -0600, Mark Tinguely wrote:
> > Not every segment in a multi-segment buffer is dirty in a
> > transaction and they will not be outputted. The assert in
> > xfs_buf_item_format_segment() that checks for the at least
> > one chunk of data in the segment to be used is not necessary
> > true for multi-segmented buffers.
> >
> > Signed-off-by: Mark Tinguely <tinguely@sgi.com>
> > ---
> > fs/xfs/xfs_buf_item.c | 20 +++++++++++++++-----
> > 1 file changed, 15 insertions(+), 5 deletions(-)
> >
> > Index: b/fs/xfs/xfs_buf_item.c
> > ===================================================================
> > --- a/fs/xfs/xfs_buf_item.c
> > +++ b/fs/xfs/xfs_buf_item.c
> > @@ -287,6 +287,17 @@ xfs_buf_item_format_segment(
> > */
> > base_size = offsetof(struct xfs_buf_log_format, blf_data_map) +
> > (blfp->blf_map_size * sizeof(blfp->blf_data_map[0]));
> > +
> > + nvecs = 0;
> > + first_bit = xfs_next_bit(blfp->blf_data_map, blfp->blf_map_size, 0);
> > + if (!(bip->bli_flags & XFS_BLI_STALE) && first_bit == -1) {
> > + /*
> > + * If the map is not be dirty in the transaction, mark
> > + * the size as zero and do not advance the vector pointer.
> > + */
> > + goto out;
> > + }
> > +
>
> I don't really like the style of this check. What's the problem of
> doing it this way:
>
> 1) fill out the first vecp
> 2) do the stale check as-is
> 3) handle the the first_bit == -1 case ala:
>
> if (first_bit == -1) {
> blfp->blf_size = 0;
> return vecp;
> }
Because if we have already written all the dirty regions and we have
a trailing clean buffer segment, vecp points past the memory
allocated for the vectors. Hence we can end up with memory
corruption if we write to vecp in this case.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 4/5] xfs: fix the multi-segment log buffer format
2012-12-04 23:18 [PATCH v3 0/5] discontiguous buffer patches Mark Tinguely
` (2 preceding siblings ...)
2012-12-04 23:18 ` [PATCH v3 3/5] xfs: fix segment in xfs_buf_item_format_segment Mark Tinguely
@ 2012-12-04 23:18 ` Mark Tinguely
2012-12-08 12:38 ` Christoph Hellwig
2012-12-04 23:18 ` [PATCH v3 5/5] xfs remove the XFS_TRANS_DEBUG routines Mark Tinguely
4 siblings, 1 reply; 13+ messages in thread
From: Mark Tinguely @ 2012-12-04 23:18 UTC (permalink / raw)
To: xfs
[-- Attachment #1: v3-3-3-xfs-fix-the-multi-segment-log-buffer-format.patch --]
[-- Type: text/plain, Size: 2439 bytes --]
Per Dave Chinner suggestion, this patch:
1) Corrects the detection of whether a multi-segment buffer is
still tracking data.
2) Clears all the buffer log formats for a multi-segment buffer.
Signed-off-by: Mark Tinguely <tinguely@sgi.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_buf_item.c | 13 ++++++++++---
fs/xfs/xfs_buf_item.c | 13 ++++++++++---
fs/xfs/xfs_trans_buf.c | 7 +++++--
2 files changed, 15 insertions(+), 5 deletions(-)
Index: b/fs/xfs/xfs_buf_item.c
===================================================================
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -611,7 +611,7 @@ xfs_buf_item_unlock(
{
struct xfs_buf_log_item *bip = BUF_ITEM(lip);
struct xfs_buf *bp = bip->bli_buf;
- int aborted;
+ int aborted, clean, i;
uint hold;
/* Clear the buffer's association with this transaction. */
@@ -654,8 +654,15 @@ xfs_buf_item_unlock(
* If the buf item isn't tracking any data, free it, otherwise drop the
* reference we hold to it.
*/
- if (xfs_bitmap_empty(bip->__bli_format.blf_data_map,
- bip->__bli_format.blf_map_size))
+ clean = 1;
+ for (i = 0; i < bip->bli_format_count; i++) {
+ if (!xfs_bitmap_empty(bip->bli_formats[i].blf_data_map,
+ bip->bli_formats[i].blf_map_size)) {
+ clean = 0;
+ break;
+ }
+ }
+ if (clean)
xfs_buf_item_relse(bp);
else
atomic_dec(&bip->bli_refcount);
Index: b/fs/xfs/xfs_trans_buf.c
===================================================================
--- a/fs/xfs/xfs_trans_buf.c
+++ b/fs/xfs/xfs_trans_buf.c
@@ -643,6 +643,7 @@ xfs_trans_binval(
xfs_buf_t *bp)
{
xfs_buf_log_item_t *bip = bp->b_fspriv;
+ int i;
ASSERT(bp->b_transp == tp);
ASSERT(bip != NULL);
@@ -670,8 +671,10 @@ xfs_trans_binval(
bip->bli_flags &= ~(XFS_BLI_INODE_BUF | XFS_BLI_LOGGED | XFS_BLI_DIRTY);
bip->__bli_format.blf_flags &= ~XFS_BLF_INODE_BUF;
bip->__bli_format.blf_flags |= XFS_BLF_CANCEL;
- memset((char *)(bip->__bli_format.blf_data_map), 0,
- (bip->__bli_format.blf_map_size * sizeof(uint)));
+ for (i = 0; i < bip->bli_format_count; i++) {
+ memset(bip->bli_formats[i].blf_data_map, 0,
+ (bip->bli_formats[i].blf_map_size * sizeof(uint)));
+ }
bip->bli_item.li_desc->lid_flags |= XFS_LID_DIRTY;
tp->t_flags |= XFS_TRANS_DIRTY;
}
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 4/5] xfs: fix the multi-segment log buffer format
2012-12-04 23:18 ` [PATCH v3 4/5] xfs: fix the multi-segment log buffer format Mark Tinguely
@ 2012-12-08 12:38 ` Christoph Hellwig
2012-12-10 1:59 ` Dave Chinner
0 siblings, 1 reply; 13+ messages in thread
From: Christoph Hellwig @ 2012-12-08 12:38 UTC (permalink / raw)
To: Mark Tinguely; +Cc: xfs
On Tue, Dec 04, 2012 at 05:18:05PM -0600, Mark Tinguely wrote:
> Per Dave Chinner suggestion, this patch:
> 1) Corrects the detection of whether a multi-segment buffer is
> still tracking data.
> 2) Clears all the buffer log formats for a multi-segment buffer.
>
> Signed-off-by: Mark Tinguely <tinguely@sgi.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/xfs_buf_item.c | 13 ++++++++++---
> fs/xfs/xfs_buf_item.c | 13 ++++++++++---
> fs/xfs/xfs_trans_buf.c | 7 +++++--
> 2 files changed, 15 insertions(+), 5 deletions(-)
>
> Index: b/fs/xfs/xfs_buf_item.c
> ===================================================================
> --- a/fs/xfs/xfs_buf_item.c
> +++ b/fs/xfs/xfs_buf_item.c
> @@ -611,7 +611,7 @@ xfs_buf_item_unlock(
> {
> struct xfs_buf_log_item *bip = BUF_ITEM(lip);
> struct xfs_buf *bp = bip->bli_buf;
> - int aborted;
> + int aborted, clean, i;
> uint hold;
>
> /* Clear the buffer's association with this transaction. */
> @@ -654,8 +654,15 @@ xfs_buf_item_unlock(
> * If the buf item isn't tracking any data, free it, otherwise drop the
> * reference we hold to it.
> */
> - if (xfs_bitmap_empty(bip->__bli_format.blf_data_map,
> - bip->__bli_format.blf_map_size))
> + clean = 1;
> + for (i = 0; i < bip->bli_format_count; i++) {
> + if (!xfs_bitmap_empty(bip->bli_formats[i].blf_data_map,
> + bip->bli_formats[i].blf_map_size)) {
> + clean = 0;
> + break;
> + }
> + }
> + if (clean)
> xfs_buf_item_relse(bp);
> else
> atomic_dec(&bip->bli_refcount);
Looks ok, although avoiding the clean variable would be even better:
for (i = 0; i < bip->bli_format_count; i++) {
if (!xfs_bitmap_empty(bip->bli_formats[i].blf_data_map,
bip->bli_formats[i].blf_map_size)) {
atomic_dec(&bip->bli_refcount);
goto out;
}
}
xfs_buf_item_relse(bp);
out:
bu that might be getting a bit too much into bikeshedding.
What I'm worried more about is how we semi-duplicate this bli_refcount
decrement vs xfs_buf_item_relse in xfs_trans_brelse, but use the
xfs_buf_item_dirty (aka XFS_BLI_DIRTY) check there instead.
It seems like the proper fix might be to:
- only set XFS_BLI_DIRTY in xfs_buf_item_log if we actually set
any bits in a bitmap
- use the XFS_BLI_DIRTY check in xfs_buf_item_unlock as well
- kill the useless xfs_buf_item_dirty wrapper
Probably both of these aren't worth doing it for now as we'll need to
get fixes into Linus tree quickly, so:
Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 4/5] xfs: fix the multi-segment log buffer format
2012-12-08 12:38 ` Christoph Hellwig
@ 2012-12-10 1:59 ` Dave Chinner
0 siblings, 0 replies; 13+ messages in thread
From: Dave Chinner @ 2012-12-10 1:59 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Mark Tinguely, xfs
On Sat, Dec 08, 2012 at 07:38:21AM -0500, Christoph Hellwig wrote:
> On Tue, Dec 04, 2012 at 05:18:05PM -0600, Mark Tinguely wrote:
> > Per Dave Chinner suggestion, this patch:
> > 1) Corrects the detection of whether a multi-segment buffer is
> > still tracking data.
> > 2) Clears all the buffer log formats for a multi-segment buffer.
> >
> > Signed-off-by: Mark Tinguely <tinguely@sgi.com>
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > ---
> > fs/xfs/xfs_buf_item.c | 13 ++++++++++---
> > fs/xfs/xfs_buf_item.c | 13 ++++++++++---
> > fs/xfs/xfs_trans_buf.c | 7 +++++--
> > 2 files changed, 15 insertions(+), 5 deletions(-)
> >
> > Index: b/fs/xfs/xfs_buf_item.c
> > ===================================================================
> > --- a/fs/xfs/xfs_buf_item.c
> > +++ b/fs/xfs/xfs_buf_item.c
> > @@ -611,7 +611,7 @@ xfs_buf_item_unlock(
> > {
> > struct xfs_buf_log_item *bip = BUF_ITEM(lip);
> > struct xfs_buf *bp = bip->bli_buf;
> > - int aborted;
> > + int aborted, clean, i;
> > uint hold;
> >
> > /* Clear the buffer's association with this transaction. */
> > @@ -654,8 +654,15 @@ xfs_buf_item_unlock(
> > * If the buf item isn't tracking any data, free it, otherwise drop the
> > * reference we hold to it.
> > */
> > - if (xfs_bitmap_empty(bip->__bli_format.blf_data_map,
> > - bip->__bli_format.blf_map_size))
> > + clean = 1;
> > + for (i = 0; i < bip->bli_format_count; i++) {
> > + if (!xfs_bitmap_empty(bip->bli_formats[i].blf_data_map,
> > + bip->bli_formats[i].blf_map_size)) {
> > + clean = 0;
> > + break;
> > + }
> > + }
> > + if (clean)
> > xfs_buf_item_relse(bp);
> > else
> > atomic_dec(&bip->bli_refcount);
>
> Looks ok, although avoiding the clean variable would be even better:
>
> for (i = 0; i < bip->bli_format_count; i++) {
> if (!xfs_bitmap_empty(bip->bli_formats[i].blf_data_map,
> bip->bli_formats[i].blf_map_size)) {
> atomic_dec(&bip->bli_refcount);
> goto out;
> }
> }
>
> xfs_buf_item_relse(bp);
> out:
Definitely better.
>
> bu that might be getting a bit too much into bikeshedding.
>
> What I'm worried more about is how we semi-duplicate this bli_refcount
> decrement vs xfs_buf_item_relse in xfs_trans_brelse, but use the
> xfs_buf_item_dirty (aka XFS_BLI_DIRTY) check there instead.
Well, we only get to the case in xfs_trans_brelse() if the buffer
was not modified in this transaction. Hence the check is for whether
it was modified in a previous transaction (and hence is in the AIL)
and not whether the buffer has any changes in the bitmap or not.
So to me the checks seem to be for two different cases - one if so
whether the buffer has physical changes, the other for whether it is
currently in the AIL.
A further complication is that the XFS_BLI_DIRTY flag is cleared
when the buffer is marked stale, so any path that looks at this flag
needs to specifically handle the XFS_BLI_STALE case before the dirty
case.
It seems to me that the one place that XFS_BLI_DIRTY is checked
could actually be replaced with a:
if (!(bip->bli_item.li_flags & XFS_LI_IN_AIL)) {
....
}
check and hence remove the reason for it's existence completely.
At that point, the flag could be repurposed as you suggest here:
> It seems like the proper fix might be to:
>
> - only set XFS_BLI_DIRTY in xfs_buf_item_log if we actually set
> any bits in a bitmap
> - use the XFS_BLI_DIRTY check in xfs_buf_item_unlock as well
> - kill the useless xfs_buf_item_dirty wrapper
>
> Probably both of these aren't worth doing it for now as we'll need to
> get fixes into Linus tree quickly, so:
Agreed.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 5/5] xfs remove the XFS_TRANS_DEBUG routines
2012-12-04 23:18 [PATCH v3 0/5] discontiguous buffer patches Mark Tinguely
` (3 preceding siblings ...)
2012-12-04 23:18 ` [PATCH v3 4/5] xfs: fix the multi-segment log buffer format Mark Tinguely
@ 2012-12-04 23:18 ` Mark Tinguely
2012-12-08 12:39 ` Christoph Hellwig
4 siblings, 1 reply; 13+ messages in thread
From: Mark Tinguely @ 2012-12-04 23:18 UTC (permalink / raw)
To: xfs
[-- Attachment #1: v3-4-xfs-remove-XFS_TRANS_DEBUG.patch --]
[-- Type: text/plain, Size: 9406 bytes --]
Remove the XFS_TRANS_DEBUG routines. They are no longer appropriate
and have not been used in years
Signed-off-by: Mark Tinguely <tinguely@sgi.com>
---
fs/xfs/xfs_buf_item.c | 106 -----------------------------------------------
fs/xfs/xfs_buf_item.h | 14 ------
fs/xfs/xfs_inode.c | 6 --
fs/xfs/xfs_inode_item.c | 16 -------
fs/xfs/xfs_inode_item.h | 4 -
fs/xfs/xfs_trans_ail.c | 14 ------
fs/xfs/xfs_trans_inode.c | 41 ------------------
7 files changed, 201 deletions(-)
Index: b/fs/xfs/xfs_buf_item.c
===================================================================
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -37,109 +37,6 @@ static inline struct xfs_buf_log_item *B
return container_of(lip, struct xfs_buf_log_item, bli_item);
}
-
-#ifdef XFS_TRANS_DEBUG
-/*
- * This function uses an alternate strategy for tracking the bytes
- * that the user requests to be logged. This can then be used
- * in conjunction with the bli_orig array in the buf log item to
- * catch bugs in our callers' code.
- *
- * We also double check the bits set in xfs_buf_item_log using a
- * simple algorithm to check that every byte is accounted for.
- */
-STATIC void
-xfs_buf_item_log_debug(
- xfs_buf_log_item_t *bip,
- uint first,
- uint last)
-{
- uint x;
- uint byte;
- uint nbytes;
- uint chunk_num;
- uint word_num;
- uint bit_num;
- uint bit_set;
- uint *wordp;
-
- ASSERT(bip->bli_logged != NULL);
- byte = first;
- nbytes = last - first + 1;
- bfset(bip->bli_logged, first, nbytes);
- for (x = 0; x < nbytes; x++) {
- chunk_num = byte >> XFS_BLF_SHIFT;
- word_num = chunk_num >> BIT_TO_WORD_SHIFT;
- bit_num = chunk_num & (NBWORD - 1);
- wordp = &(bip->__bli_format.blf_data_map[word_num]);
- bit_set = *wordp & (1 << bit_num);
- ASSERT(bit_set);
- byte++;
- }
-}
-
-/*
- * This function is called when we flush something into a buffer without
- * logging it. This happens for things like inodes which are logged
- * separately from the buffer.
- */
-void
-xfs_buf_item_flush_log_debug(
- xfs_buf_t *bp,
- uint first,
- uint last)
-{
- xfs_buf_log_item_t *bip = bp->b_fspriv;
- uint nbytes;
-
- if (bip == NULL || (bip->bli_item.li_type != XFS_LI_BUF))
- return;
-
- ASSERT(bip->bli_logged != NULL);
- nbytes = last - first + 1;
- bfset(bip->bli_logged, first, nbytes);
-}
-
-/*
- * This function is called to verify that our callers have logged
- * all the bytes that they changed.
- *
- * It does this by comparing the original copy of the buffer stored in
- * the buf log item's bli_orig array to the current copy of the buffer
- * and ensuring that all bytes which mismatch are set in the bli_logged
- * array of the buf log item.
- */
-STATIC void
-xfs_buf_item_log_check(
- xfs_buf_log_item_t *bip)
-{
- char *orig;
- char *buffer;
- int x;
- xfs_buf_t *bp;
-
- ASSERT(bip->bli_orig != NULL);
- ASSERT(bip->bli_logged != NULL);
-
- bp = bip->bli_buf;
- ASSERT(bp->b_length > 0);
- ASSERT(bp->b_addr != NULL);
- orig = bip->bli_orig;
- buffer = bp->b_addr;
- for (x = 0; x < BBTOB(bp->b_length); x++) {
- if (orig[x] != buffer[x] && !btst(bip->bli_logged, x)) {
- xfs_emerg(bp->b_mount,
- "%s: bip %x buffer %x orig %x index %d",
- __func__, bip, bp, orig, x);
- ASSERT(0);
- }
- }
-}
-#else
-#define xfs_buf_item_log_debug(x,y,z)
-#define xfs_buf_item_log_check(x)
-#endif
-
STATIC void xfs_buf_do_callbacks(struct xfs_buf *bp);
/*
@@ -429,7 +326,6 @@ xfs_buf_item_format(
* Check to make sure everything is consistent.
*/
trace_xfs_buf_item_format(bip);
- xfs_buf_item_log_check(bip);
}
/*
@@ -915,8 +811,6 @@ xfs_buf_item_log_segment(
mask = (1 << end_bit) - 1;
*wordp |= mask;
}
-
- xfs_buf_item_log_debug(bip, first, last);
}
/*
Index: b/fs/xfs/xfs_buf_item.h
===================================================================
--- a/fs/xfs/xfs_buf_item.h
+++ b/fs/xfs/xfs_buf_item.h
@@ -98,10 +98,6 @@ typedef struct xfs_buf_log_item {
unsigned int bli_flags; /* misc flags */
unsigned int bli_recur; /* lock recursion count */
atomic_t bli_refcount; /* cnt of tp refs */
-#ifdef XFS_TRANS_DEBUG
- char *bli_orig; /* original buffer copy */
- char *bli_logged; /* bytes logged (bitmap) */
-#endif
int bli_format_count; /* count of headers */
struct xfs_buf_log_format *bli_formats; /* array of in-log header ptrs */
struct xfs_buf_log_format __bli_format; /* embedded in-log header */
@@ -117,16 +113,6 @@ void xfs_buf_attach_iodone(struct xfs_bu
void xfs_buf_iodone_callbacks(struct xfs_buf *);
void xfs_buf_iodone(struct xfs_buf *, struct xfs_log_item *);
-#ifdef XFS_TRANS_DEBUG
-void
-xfs_buf_item_flush_log_debug(
- struct xfs_buf *bp,
- uint first,
- uint last);
-#else
-#define xfs_buf_item_flush_log_debug(bp, first, last)
-#endif
-
#endif /* __KERNEL__ */
#endif /* __XFS_BUF_ITEM_H__ */
Index: b/fs/xfs/xfs_inode.c
===================================================================
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2379,9 +2379,6 @@ xfs_iflush_fork(
char *cp;
xfs_ifork_t *ifp;
xfs_mount_t *mp;
-#ifdef XFS_TRANS_DEBUG
- int first;
-#endif
static const short brootflag[2] =
{ XFS_ILOG_DBROOT, XFS_ILOG_ABROOT };
static const short dataflag[2] =
@@ -2724,9 +2721,6 @@ xfs_iflush_int(
xfs_inode_log_item_t *iip;
xfs_dinode_t *dip;
xfs_mount_t *mp;
-#ifdef XFS_TRANS_DEBUG
- int first;
-#endif
ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL|XFS_ILOCK_SHARED));
ASSERT(xfs_isiflocked(ip));
Index: b/fs/xfs/xfs_inode_item.c
===================================================================
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -269,17 +269,6 @@ xfs_inode_item_format(
} else {
ASSERT(!(iip->ili_fields &
XFS_ILOG_DBROOT));
-#ifdef XFS_TRANS_DEBUG
- if (iip->ili_root_size > 0) {
- ASSERT(iip->ili_root_size ==
- ip->i_df.if_broot_bytes);
- ASSERT(memcmp(iip->ili_orig_root,
- ip->i_df.if_broot,
- iip->ili_root_size) == 0);
- } else {
- ASSERT(ip->i_df.if_broot_bytes == 0);
- }
-#endif
iip->ili_fields &= ~XFS_ILOG_DBROOT;
}
break;
@@ -678,11 +667,6 @@ void
xfs_inode_item_destroy(
xfs_inode_t *ip)
{
-#ifdef XFS_TRANS_DEBUG
- if (ip->i_itemp->ili_root_size != 0) {
- kmem_free(ip->i_itemp->ili_orig_root);
- }
-#endif
kmem_zone_free(xfs_ili_zone, ip->i_itemp);
}
Index: b/fs/xfs/xfs_inode_item.h
===================================================================
--- a/fs/xfs/xfs_inode_item.h
+++ b/fs/xfs/xfs_inode_item.h
@@ -148,10 +148,6 @@ typedef struct xfs_inode_log_item {
data exts */
struct xfs_bmbt_rec *ili_aextents_buf; /* array of logged
attr exts */
-#ifdef XFS_TRANS_DEBUG
- int ili_root_size;
- char *ili_orig_root;
-#endif
xfs_inode_log_format_t ili_format; /* logged structure */
} xfs_inode_log_item_t;
Index: b/fs/xfs/xfs_trans_ail.c
===================================================================
--- a/fs/xfs/xfs_trans_ail.c
+++ b/fs/xfs/xfs_trans_ail.c
@@ -55,20 +55,6 @@ xfs_ail_check(
ASSERT(XFS_LSN_CMP(prev_lip->li_lsn, lip->li_lsn) >= 0);
-#ifdef XFS_TRANS_DEBUG
- /*
- * Walk the list checking lsn ordering, and that every entry has the
- * XFS_LI_IN_AIL flag set. This is really expensive, so only do it
- * when specifically debugging the transaction subsystem.
- */
- prev_lip = list_entry(&ailp->xa_ail, xfs_log_item_t, li_ail);
- list_for_each_entry(lip, &ailp->xa_ail, li_ail) {
- if (&prev_lip->li_ail != &ailp->xa_ail)
- ASSERT(XFS_LSN_CMP(prev_lip->li_lsn, lip->li_lsn) <= 0);
- ASSERT((lip->li_flags & XFS_LI_IN_AIL) != 0);
- prev_lip = lip;
- }
-#endif /* XFS_TRANS_DEBUG */
}
#else /* !DEBUG */
#define xfs_ail_check(a,l)
Index: b/fs/xfs/xfs_trans_inode.c
===================================================================
--- a/fs/xfs/xfs_trans_inode.c
+++ b/fs/xfs/xfs_trans_inode.c
@@ -33,14 +33,6 @@
#include "xfs_inode_item.h"
#include "xfs_trace.h"
-#ifdef XFS_TRANS_DEBUG
-STATIC void
-xfs_trans_inode_broot_debug(
- xfs_inode_t *ip);
-#else
-#define xfs_trans_inode_broot_debug(ip)
-#endif
-
/*
* Add a locked inode to the transaction.
*
@@ -67,8 +59,6 @@ xfs_trans_ijoin(
* Get a log_item_desc to point at the new item.
*/
xfs_trans_add_item(tp, &iip->ili_item);
-
- xfs_trans_inode_broot_debug(ip);
}
/*
@@ -135,34 +125,3 @@ xfs_trans_log_inode(
flags |= ip->i_itemp->ili_last_fields;
ip->i_itemp->ili_fields |= flags;
}
-
-#ifdef XFS_TRANS_DEBUG
-/*
- * Keep track of the state of the inode btree root to make sure we
- * log it properly.
- */
-STATIC void
-xfs_trans_inode_broot_debug(
- xfs_inode_t *ip)
-{
- xfs_inode_log_item_t *iip;
-
- ASSERT(ip->i_itemp != NULL);
- iip = ip->i_itemp;
- if (iip->ili_root_size != 0) {
- ASSERT(iip->ili_orig_root != NULL);
- kmem_free(iip->ili_orig_root);
- iip->ili_root_size = 0;
- iip->ili_orig_root = NULL;
- }
- if (ip->i_d.di_format == XFS_DINODE_FMT_BTREE) {
- ASSERT((ip->i_df.if_broot != NULL) &&
- (ip->i_df.if_broot_bytes > 0));
- iip->ili_root_size = ip->i_df.if_broot_bytes;
- iip->ili_orig_root =
- (char*)kmem_alloc(iip->ili_root_size, KM_SLEEP);
- memcpy(iip->ili_orig_root, (char*)(ip->i_df.if_broot),
- iip->ili_root_size);
- }
-}
-#endif
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 13+ messages in thread