[Top] [All Lists]

Re: [PATCH 2/2] xfs: fix the buffer log format for contiguous buffers

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 2/2] xfs: fix the buffer log format for contiguous buffers
From: Mark Tinguely <tinguely@xxxxxxx>
Date: Wed, 21 Nov 2012 07:21:09 -0600
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20121121095422.GB23339@xxxxxxxxxxxxx>
References: <20121120224120.224166649@xxxxxxx> <20121120224146.479983109@xxxxxxx> <20121121095422.GB23339@xxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:9.0) Gecko/20120122 Thunderbird/9.0
On 11/21/12 03:54, Christoph Hellwig wrote:
Do we have proper test coverage for the xfs_buf_item_format_segment
changes?  Given that they change what gets written into the log I'd
prefer them separate from a big cleanup patch, and including test
coverage verifying everything works fine when hitting these corner

                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_formats[0].blf_data_map[word_num]);

This change doesn't seem to be mentioned in the changelog?

@@ -644,8 +652,14 @@ 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))
+       empty = 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)) {
+                       empty = 0;
+                       break;
+               }
+       if (empty)

This looks like a bug fix, not just a cleanup.  Again I'd prefer this to
be a patch on its own, with a good description.

The bli_format -> bli_formats changes are bug fixes - the description should say this is a bug fix not just a cleanup.

Eric's test 291 created a buffer with 4 mapped segments. It found most of the issues in the buffer map and in the buffer log format. Dave pointed out the xfs_bitmap_empty() error.


<Prev in Thread] Current Thread [Next in Thread>