From: Mark Tinguely <tinguely@sgi.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 2/2] xfs: fix the buffer log format for contiguous buffers
Date: Wed, 21 Nov 2012 07:21:09 -0600 [thread overview]
Message-ID: <50ACD545.8030102@sgi.com> (raw)
In-Reply-To: <20121121095422.GB23339@infradead.org>
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
> cases.
>
>> 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)
>> xfs_buf_item_relse(bp);
>
> 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.
--Mark.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2012-11-21 14:19 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-11-20 22:41 [PATCH 0/2] discontiguous buffer patches Mark Tinguely
2012-11-20 22:41 ` [PATCH 1/2] xfs: use b_maps[] for discontiguous buffers Mark Tinguely
2012-11-21 9:51 ` Christoph Hellwig
2012-11-23 1:32 ` Dave Chinner
2012-11-20 22:41 ` [PATCH 2/2] xfs: fix the buffer log format for contiguous buffers Mark Tinguely
2012-11-21 9:54 ` Christoph Hellwig
2012-11-21 13:21 ` Mark Tinguely [this message]
2012-11-23 1:47 ` Dave Chinner
2012-11-23 7:37 ` Christoph Hellwig
2012-11-25 18:59 ` Mark Tinguely
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=50ACD545.8030102@sgi.com \
--to=tinguely@sgi.com \
--cc=hch@infradead.org \
--cc=xfs@oss.sgi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.