* [PATCH] xfs: fix node forward in xfs_node_toosmall
@ 2013-09-20 22:05 Mark Tinguely
2013-09-23 0:08 ` Dave Chinner
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Mark Tinguely @ 2013-09-20 22:05 UTC (permalink / raw)
To: xfs
[-- Attachment #1: xfs-fix-xfs_da3_node_toosmall-test.patch --]
[-- Type: text/plain, Size: 1670 bytes --]
Commit f5ea1100 cleans up the disk to host conversions for
node directory entries, but because a variable is reused in
xfs_node_toosmall() the next node is not correctly found.
If the original node is small enough (<= 3/8 of the node size),
this change may incorrectly cause a node collapse when it should
not. That will cause an assert in xfstest generic/319:
Assertion failed: first <= last && last < BBTOB(bp->b_length),
file: /root/newest/xfs/fs/xfs/xfs_trans_buf.c, line: 569
Keep the original node header to get the correct forward node.
Signed-off-by: Mark Tinguely <tinguely@sgi.com>
---
fs/xfs/xfs_da_btree.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
Index: b/fs/xfs/xfs_da_btree.c
===================================================================
--- a/fs/xfs/xfs_da_btree.c
+++ b/fs/xfs/xfs_da_btree.c
@@ -1163,6 +1163,7 @@ xfs_da3_node_toosmall(
xfs_dablk_t blkno;
struct xfs_buf *bp;
struct xfs_da3_icnode_hdr nodehdr;
+ struct xfs_da3_icnode_hdr firstnhdr;
int count;
int forward;
int error;
@@ -1221,13 +1222,14 @@ xfs_da3_node_toosmall(
count -= state->node_ents >> 2;
count -= nodehdr.count;
+ firstnhdr = nodehdr;
/* start with smaller blk num */
forward = nodehdr.forw < nodehdr.back;
for (i = 0; i < 2; forward = !forward, i++) {
if (forward)
- blkno = nodehdr.forw;
+ blkno = firstnhdr.forw;
else
- blkno = nodehdr.back;
+ blkno = firstnhdr.back;
if (blkno == 0)
continue;
error = xfs_da3_node_read(state->args->trans, state->args->dp,
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] xfs: fix node forward in xfs_node_toosmall 2013-09-20 22:05 [PATCH] xfs: fix node forward in xfs_node_toosmall Mark Tinguely @ 2013-09-23 0:08 ` Dave Chinner 2013-09-23 13:38 ` Mark Tinguely 2013-09-23 17:18 ` [PATCH] xfs: v2 " Mark Tinguely 2013-09-23 21:34 ` [PATCH] xfs: " Michael L. Semon 2 siblings, 1 reply; 15+ messages in thread From: Dave Chinner @ 2013-09-23 0:08 UTC (permalink / raw) To: Mark Tinguely; +Cc: xfs On Fri, Sep 20, 2013 at 05:05:08PM -0500, Mark Tinguely wrote: > Commit f5ea1100 cleans up the disk to host conversions for > node directory entries, but because a variable is reused in > xfs_node_toosmall() the next node is not correctly found. > If the original node is small enough (<= 3/8 of the node size), > this change may incorrectly cause a node collapse when it should > not. That will cause an assert in xfstest generic/319: > > Assertion failed: first <= last && last < BBTOB(bp->b_length), > file: /root/newest/xfs/fs/xfs/xfs_trans_buf.c, line: 569 > > Keep the original node header to get the correct forward node. > > Signed-off-by: Mark Tinguely <tinguely@sgi.com> > --- > fs/xfs/xfs_da_btree.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > Index: b/fs/xfs/xfs_da_btree.c > =================================================================== > --- a/fs/xfs/xfs_da_btree.c > +++ b/fs/xfs/xfs_da_btree.c > @@ -1163,6 +1163,7 @@ xfs_da3_node_toosmall( > xfs_dablk_t blkno; > struct xfs_buf *bp; > struct xfs_da3_icnode_hdr nodehdr; > + struct xfs_da3_icnode_hdr firstnhdr; > int count; > int forward; > int error; > @@ -1221,13 +1222,14 @@ xfs_da3_node_toosmall( > count -= state->node_ents >> 2; > count -= nodehdr.count; > > + firstnhdr = nodehdr; > /* start with smaller blk num */ > forward = nodehdr.forw < nodehdr.back; > for (i = 0; i < 2; forward = !forward, i++) { > if (forward) > - blkno = nodehdr.forw; > + blkno = firstnhdr.forw; > else > - blkno = nodehdr.back; > + blkno = firstnhdr.back; > if (blkno == 0) > continue; > error = xfs_da3_node_read(state->args->trans, state->args->dp, Yes, that definitely a bug, but I think that the change doesn't scope correctly. The original node header doesn't need to be saved like this - the node header decoded in the loop needs a loop-scope variable. i.e.: /* start with smaller blk num */ forward = nodehdr.forw < nodehdr.back; for (i = 0; i < 2; forward = !forward, i++) { + struct xfs_da3_icnode_hdr thdr; + if (forward) blkno = nodehdr.forw; else blkno = nodehdr.back; if (blkno == 0) continue; error = xfs_da3_node_read(state->args->trans, state->args->dp, blkno, -1, &bp, state->args->whichfork); if (error) return(error); node = bp->b_addr; - xfs_da3_node_hdr_from_disk(&nodehdr, node); + xfs_da3_node_hdr_from_disk(&thdr, node); xfs_trans_brelse(state->args->trans, bp); - if (count - nodehdr.count >= 0) + if (count - thdr.count >= 0) break; /* fits with at least 25% to spare */ } 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] 15+ messages in thread
* Re: [PATCH] xfs: fix node forward in xfs_node_toosmall 2013-09-23 0:08 ` Dave Chinner @ 2013-09-23 13:38 ` Mark Tinguely 0 siblings, 0 replies; 15+ messages in thread From: Mark Tinguely @ 2013-09-23 13:38 UTC (permalink / raw) To: Dave Chinner; +Cc: xfs On 09/22/13 19:08, Dave Chinner wrote: > On Fri, Sep 20, 2013 at 05:05:08PM -0500, Mark Tinguely wrote: >> Commit f5ea1100 cleans up the disk to host conversions for >> node directory entries, but because a variable is reused in >> xfs_node_toosmall() the next node is not correctly found. >> If the original node is small enough (<= 3/8 of the node size), >> this change may incorrectly cause a node collapse when it should >> not. That will cause an assert in xfstest generic/319: >> >> Assertion failed: first<= last&& last< BBTOB(bp->b_length), >> file: /root/newest/xfs/fs/xfs/xfs_trans_buf.c, line: 569 >> >> Keep the original node header to get the correct forward node. >> >> Signed-off-by: Mark Tinguely<tinguely@sgi.com> >> --- ... > Yes, that definitely a bug, but I think that the change doesn't > scope correctly. The original node header doesn't need to be saved > like this - the node header decoded in the loop needs a loop-scope > variable. i.e.: > > /* start with smaller blk num */ > forward = nodehdr.forw< nodehdr.back; > for (i = 0; i< 2; forward = !forward, i++) { > + struct xfs_da3_icnode_hdr thdr; > + > if (forward) > blkno = nodehdr.forw; > else > blkno = nodehdr.back; > if (blkno == 0) > continue; > error = xfs_da3_node_read(state->args->trans, state->args->dp, > blkno, -1,&bp, state->args->whichfork); > if (error) > return(error); > > node = bp->b_addr; > - xfs_da3_node_hdr_from_disk(&nodehdr, node); > + xfs_da3_node_hdr_from_disk(&thdr, node); > xfs_trans_brelse(state->args->trans, bp); > > - if (count - nodehdr.count>= 0) > + if (count - thdr.count>= 0) > break; /* fits with at least 25% to spare */ > } > > Cheers, > > Dave. Okay thanks. --Mark. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] xfs: v2 fix node forward in xfs_node_toosmall 2013-09-20 22:05 [PATCH] xfs: fix node forward in xfs_node_toosmall Mark Tinguely 2013-09-23 0:08 ` Dave Chinner @ 2013-09-23 17:18 ` Mark Tinguely 2013-09-23 23:48 ` Dave Chinner 2013-09-23 21:34 ` [PATCH] xfs: " Michael L. Semon 2 siblings, 1 reply; 15+ messages in thread From: Mark Tinguely @ 2013-09-23 17:18 UTC (permalink / raw) To: xfs, stable [-- Attachment #1: v2-xfs-fix-xfs_da3_node_toosmall-test.patch --] [-- Type: text/plain, Size: 1680 bytes --] Commit f5ea1100 cleans up the disk to host conversions for node directory entries, but because a variable is reused in xfs_node_toosmall() the next node is not correctly found. If the original node is small enough (<= 3/8 of the node size), this change may incorrectly cause a node collapse when it should not. That will cause an assert in xfstest generic/319: Assertion failed: first <= last && last < BBTOB(bp->b_length), file: /root/newest/xfs/fs/xfs/xfs_trans_buf.c, line: 569 Keep the original node header to get the correct forward node. Signed-off-by: Mark Tinguely <tinguely@sgi.com> --- v2 -> Dave's local variable approach. -> send to -stable this bug is in 3.10 and 3.11 fs/xfs/xfs_da_btree.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) Index: b/fs/xfs/xfs_da_btree.c =================================================================== --- a/fs/xfs/xfs_da_btree.c +++ b/fs/xfs/xfs_da_btree.c @@ -1224,6 +1224,7 @@ xfs_da3_node_toosmall( /* start with smaller blk num */ forward = nodehdr.forw < nodehdr.back; for (i = 0; i < 2; forward = !forward, i++) { + struct xfs_da3_icnode_hdr thdr; if (forward) blkno = nodehdr.forw; else @@ -1236,10 +1237,10 @@ xfs_da3_node_toosmall( return(error); node = bp->b_addr; - xfs_da3_node_hdr_from_disk(&nodehdr, node); + xfs_da3_node_hdr_from_disk(&thdr, node); xfs_trans_brelse(state->args->trans, bp); - if (count - nodehdr.count >= 0) + if (count - thdr.count >= 0) break; /* fits with at least 25% to spare */ } if (i >= 2) { _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] xfs: v2 fix node forward in xfs_node_toosmall 2013-09-23 17:18 ` [PATCH] xfs: v2 " Mark Tinguely @ 2013-09-23 23:48 ` Dave Chinner 2013-09-24 17:35 ` Mark Tinguely 0 siblings, 1 reply; 15+ messages in thread From: Dave Chinner @ 2013-09-23 23:48 UTC (permalink / raw) To: Mark Tinguely; +Cc: stable, xfs On Mon, Sep 23, 2013 at 12:18:58PM -0500, Mark Tinguely wrote: > Commit f5ea1100 cleans up the disk to host conversions for > node directory entries, but because a variable is reused in > xfs_node_toosmall() the next node is not correctly found. > If the original node is small enough (<= 3/8 of the node size), > this change may incorrectly cause a node collapse when it should > not. The comment about the size of the node triggering a collapse is irrelevant - nodes always collapse at that given size. What this doesn't tell us is why the crash occurs - "the next node is not correctly found" is not particularly obvious, and would require quite a bit of code reading to work out from first principles a couple of years down the track. The commit message should be more precise and describe what the underlying cause of the failure was. i.e. that the node is finding itself as the merge candidate because we go forward, overwrite the pointers and the new block's backward sibling is the original block which is where we end up on teh second loop. And vice versa if we go backwards first... Also, the "next node" is correctly termed a "sibling", and it's either the forwards or backwards sibling, not the "next" sibling as the direction of movement is important. So perhaps this is better written as: "When a node is considered for a merge with a sibling, it overwrites the sibling pointers of the original node with the sibling's pointers. This leads to loop considering the original node as a merge candidate with itself in the second pass, and so it incorrectly determines a merge should occur." > That will cause an assert in xfstest generic/319: > > Assertion failed: first <= last && last < BBTOB(bp->b_length), > file: /root/newest/xfs/fs/xfs/xfs_trans_buf.c, line: 569 > > Keep the original node header to get the correct forward node. > > Signed-off-by: Mark Tinguely <tinguely@sgi.com> > --- > v2 -> Dave's local variable approach. > -> send to -stable this bug is in 3.10 and 3.11 The patch title not include "v2". That goes in the "[PATCH v2]" prefix that gets stripped before commit so that commit messages don't have patch version numbers in them.... Otherwise the code looks good. 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] 15+ messages in thread
* Re: [PATCH] xfs: v2 fix node forward in xfs_node_toosmall 2013-09-23 23:48 ` Dave Chinner @ 2013-09-24 17:35 ` Mark Tinguely 2013-09-24 18:59 ` Ben Myers 2013-09-24 21:06 ` Dave Chinner 0 siblings, 2 replies; 15+ messages in thread From: Mark Tinguely @ 2013-09-24 17:35 UTC (permalink / raw) To: Dave Chinner; +Cc: stable, xfs On 09/23/13 18:48, Dave Chinner wrote: > On Mon, Sep 23, 2013 at 12:18:58PM -0500, Mark Tinguely wrote: >> Commit f5ea1100 cleans up the disk to host conversions for >> node directory entries, but because a variable is reused in >> xfs_node_toosmall() the next node is not correctly found. >> If the original node is small enough (<= 3/8 of the node size), >> this change may incorrectly cause a node collapse when it should >> not. > > The comment about the size of the node triggering a collapse is > irrelevant - nodes always collapse at that given size. What this > doesn't tell us is why the crash occurs - "the next node is not > correctly found" is not particularly obvious, and would require > quite a bit of code reading to work out from first principles a > couple of years down the track. > > The commit message should be more precise and describe what the > underlying cause of the failure was. i.e. that the node is finding itself as the merge > candidate because we go forward, overwrite the pointers and the new > block's backward sibling is the original block which is where we end > up on teh second loop. And vice versa if we go backwards first... > > Also, the "next node" is correctly termed a "sibling", and it's > either the forwards or backwards sibling, not the "next" sibling as > the direction of movement is important. So perhaps this > is better written as: > > "When a node is considered for a merge with a sibling, it overwrites > the sibling pointers of the original node with the sibling's > pointers. This leads to loop considering the original node as a > merge candidate with itself in the second pass, and so it > incorrectly determines a merge should occur." > Are you done ranting? Get the @#$% bug patched. --Mark. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] xfs: v2 fix node forward in xfs_node_toosmall 2013-09-24 17:35 ` Mark Tinguely @ 2013-09-24 18:59 ` Ben Myers 2013-09-24 21:06 ` Dave Chinner 1 sibling, 0 replies; 15+ messages in thread From: Ben Myers @ 2013-09-24 18:59 UTC (permalink / raw) To: Mark Tinguely, Dave Chinner; +Cc: stable, xfs On Tue, Sep 24, 2013 at 12:35:44PM -0500, Mark Tinguely wrote: > On 09/23/13 18:48, Dave Chinner wrote: > >On Mon, Sep 23, 2013 at 12:18:58PM -0500, Mark Tinguely wrote: > >>Commit f5ea1100 cleans up the disk to host conversions for > >>node directory entries, but because a variable is reused in > >>xfs_node_toosmall() the next node is not correctly found. > >>If the original node is small enough (<= 3/8 of the node size), > >>this change may incorrectly cause a node collapse when it should > >>not. > > > >The comment about the size of the node triggering a collapse is > >irrelevant - nodes always collapse at that given size. What this > >doesn't tell us is why the crash occurs - "the next node is not > >correctly found" is not particularly obvious, and would require > >quite a bit of code reading to work out from first principles a > >couple of years down the track. > > > >The commit message should be more precise and describe what the > >underlying cause of the failure was. i.e. that the node is finding itself as the merge > >candidate because we go forward, overwrite the pointers and the new > >block's backward sibling is the original block which is where we end > >up on teh second loop. And vice versa if we go backwards first... > > > >Also, the "next node" is correctly termed a "sibling", and it's > >either the forwards or backwards sibling, not the "next" sibling as > >the direction of movement is important. So perhaps this > >is better written as: > > > >"When a node is considered for a merge with a sibling, it overwrites > >the sibling pointers of the original node with the sibling's > >pointers. This leads to loop considering the original node as a > >merge candidate with itself in the second pass, and so it > >incorrectly determines a merge should occur." > > > > Are you done ranting? Get the @#$% bug patched. /me rolls his eyes. I gather this means you're not inclined to repost. I'll add Dave's suggestions to the commit header and move on. Reviewed-by: Ben Myers <bpm@sgi.com> Applied. -Ben _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] xfs: v2 fix node forward in xfs_node_toosmall 2013-09-24 17:35 ` Mark Tinguely 2013-09-24 18:59 ` Ben Myers @ 2013-09-24 21:06 ` Dave Chinner 2013-09-24 21:34 ` Mark Tinguely 1 sibling, 1 reply; 15+ messages in thread From: Dave Chinner @ 2013-09-24 21:06 UTC (permalink / raw) To: Mark Tinguely; +Cc: stable, xfs On Tue, Sep 24, 2013 at 12:35:44PM -0500, Mark Tinguely wrote: > On 09/23/13 18:48, Dave Chinner wrote: > >On Mon, Sep 23, 2013 at 12:18:58PM -0500, Mark Tinguely wrote: > >>Commit f5ea1100 cleans up the disk to host conversions for > >>node directory entries, but because a variable is reused in > >>xfs_node_toosmall() the next node is not correctly found. > >>If the original node is small enough (<= 3/8 of the node size), > >>this change may incorrectly cause a node collapse when it should > >>not. > > > >The comment about the size of the node triggering a collapse is > >irrelevant - nodes always collapse at that given size. What this > >doesn't tell us is why the crash occurs - "the next node is not > >correctly found" is not particularly obvious, and would require > >quite a bit of code reading to work out from first principles a > >couple of years down the track. > > > >The commit message should be more precise and describe what the > >underlying cause of the failure was. i.e. that the node is finding itself as the merge > >candidate because we go forward, overwrite the pointers and the new > >block's backward sibling is the original block which is where we end > >up on teh second loop. And vice versa if we go backwards first... > > > >Also, the "next node" is correctly termed a "sibling", and it's > >either the forwards or backwards sibling, not the "next" sibling as > >the direction of movement is important. So perhaps this > >is better written as: > > > >"When a node is considered for a merge with a sibling, it overwrites > >the sibling pointers of the original node with the sibling's > >pointers. This leads to loop considering the original node as a > >merge candidate with itself in the second pass, and so it > >incorrectly determines a merge should occur." > > > > Are you done ranting? Get the @#$% bug patched. I'm deeply sorry you feel that way about the review process - it's not just code that matters. Experience has shown us time and time again that accurate and complete commit messages are extremely valuable as they document the symptoms of a problem being fixed and why the fix was needed. If someone needs to look at this commit in a couple of years to determine if it matches a problem that a customer reported, they shouldn't have to work out what the problem was and guess at it's symptoms and impact from code analysis. The commit message should tell them all the information they need(*). It took you quite some time and effort to find the problem, so it's worthy of spending a few minutes to document that effort for posterity. That way when someone asks you a question about the problem, all you need to do is point them at the commit and all the information is right at their fingertips. I'm not asking you to do anything I don't do already. Have you ever wondered why I write long, verbose commit messages and changes with verbose comments? They aren't for the reviewer - I can answer questions in real-time about the change. The message is for someone looking at the change in 2-3 years time when when nobody remembers the exact circumstances of the fix anymore. IOWs, by clearly documenting the problem being fixed, the root cause analysis and verification that was performed *using standard terminology*, we make it far easier for someone to come along in 2-3 years time and understand the fix without needing any further information about it. Software engineering best practices have come a long way since the early 90's - writing meaningful commit messages to go along with your code changes has been considered a best practise for at least the last 10 years.... Cheers, Dave. (*) If you've ever spent any time looking at the old XFS archives, then you'll understand exactly why what I've asked for is important. Trying to reverse-engineer why a change was made in the old XFS code is just about impossible because all they generally have is single line commit messages and nothing else to describe the change. Sometimes they just point at a bug number, without any other information at all..... -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] xfs: v2 fix node forward in xfs_node_toosmall 2013-09-24 21:06 ` Dave Chinner @ 2013-09-24 21:34 ` Mark Tinguely 2013-09-24 23:33 ` Dave Chinner 0 siblings, 1 reply; 15+ messages in thread From: Mark Tinguely @ 2013-09-24 21:34 UTC (permalink / raw) To: Dave Chinner; +Cc: stable, xfs On 09/24/13 16:06, Dave Chinner wrote: > On Tue, Sep 24, 2013 at 12:35:44PM -0500, Mark Tinguely wrote: >> On 09/23/13 18:48, Dave Chinner wrote: >>> On Mon, Sep 23, 2013 at 12:18:58PM -0500, Mark Tinguely wrote: >>>> Commit f5ea1100 cleans up the disk to host conversions for >>>> node directory entries, but because a variable is reused in >>>> xfs_node_toosmall() the next node is not correctly found. >>>> If the original node is small enough (<= 3/8 of the node size), >>>> this change may incorrectly cause a node collapse when it should >>>> not. >>> >>> The comment about the size of the node triggering a collapse is >>> irrelevant - nodes always collapse at that given size. What this >>> doesn't tell us is why the crash occurs - "the next node is not >>> correctly found" is not particularly obvious, and would require >>> quite a bit of code reading to work out from first principles a >>> couple of years down the track. >>> >>> The commit message should be more precise and describe what the >>> underlying cause of the failure was. i.e. that the node is finding itself as the merge >>> candidate because we go forward, overwrite the pointers and the new >>> block's backward sibling is the original block which is where we end >>> up on teh second loop. And vice versa if we go backwards first... >>> >>> Also, the "next node" is correctly termed a "sibling", and it's >>> either the forwards or backwards sibling, not the "next" sibling as >>> the direction of movement is important. So perhaps this >>> is better written as: >>> >>> "When a node is considered for a merge with a sibling, it overwrites >>> the sibling pointers of the original node with the sibling's >>> pointers. This leads to loop considering the original node as a >>> merge candidate with itself in the second pass, and so it >>> incorrectly determines a merge should occur." >>> >> >> Are you done ranting? Get the @#$% bug patched. > > I'm deeply sorry you feel that way about the review process - it's > not just code that matters. Experience has shown us time and time > again that accurate and complete commit messages are extremely > valuable as they document the symptoms of a problem being fixed and > why the fix was needed. > > If someone needs to look at this commit in a couple of years to > determine if it matches a problem that a customer reported, they > shouldn't have to work out what the problem was and guess at it's > symptoms and impact from code analysis. The commit message should > tell them all the information they need(*). > > It took you quite some time and effort to find the problem, so it's > worthy of spending a few minutes to document that effort for > posterity. That way when someone asks you a question about the > problem, all you need to do is point them at the commit and all the > information is right at their fingertips. > > I'm not asking you to do anything I don't do already. Have you ever > wondered why I write long, verbose commit messages and changes with > verbose comments? They aren't for the reviewer - I can answer > questions in real-time about the change. The message is for someone > looking at the change in 2-3 years time when when nobody remembers > the exact circumstances of the fix anymore. > > IOWs, by clearly documenting the problem being fixed, the root cause > analysis and verification that was performed *using standard > terminology*, we make it far easier for someone to come along in 2-3 > years time and understand the fix without needing any further > information about it. > > Software engineering best practices have come a long way since the > early 90's - writing meaningful commit messages to go along with > your code changes has been considered a best practise for at least > the last 10 years.... > > Cheers, > > Dave. > > (*) If you've ever spent any time looking at the old XFS archives, > then you'll understand exactly why what I've asked for is important. > Trying to reverse-engineer why a change was made in the old XFS code > is just about impossible because all they generally have is single > line commit messages and nothing else to describe the change. > Sometimes they just point at a bug number, without any other > information at all..... You do not always practice what you preach - here is the commit message from the patch that caused this whole problem: commit f5ea110044fa858925a880b4fa9f551bfa2dfc38 Author: Dave Chinner <dchinner@redhat.com> Date: Wed Apr 24 18:58:02 2013 +1000 xfs: add CRCs to dir2/da node blocks Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Ben Myers <bpm@sgi.com> Signed-off-by: Ben Myers <bpm@sgi.com> Did this patch just add CRCs to node blocks? no. And how about your patches that do not need a commit message at all?! Are these exceptions? No, I have done enough reviews of your patches, and some of your commit message are less than stunning. when your preaching, and not so subtle digs at me or SGI gets to the point of rewriting commit messages even I can't hold my tongue any more. PS. I told Ben that I do not appreciate commit messages being changed. --Mark. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] xfs: v2 fix node forward in xfs_node_toosmall 2013-09-24 21:34 ` Mark Tinguely @ 2013-09-24 23:33 ` Dave Chinner 2013-09-25 18:38 ` Ben Myers 0 siblings, 1 reply; 15+ messages in thread From: Dave Chinner @ 2013-09-24 23:33 UTC (permalink / raw) To: Mark Tinguely; +Cc: stable, xfs On Tue, Sep 24, 2013 at 04:34:53PM -0500, Mark Tinguely wrote: > On 09/24/13 16:06, Dave Chinner wrote: > >On Tue, Sep 24, 2013 at 12:35:44PM -0500, Mark Tinguely wrote: > >>On 09/23/13 18:48, Dave Chinner wrote: > >>>On Mon, Sep 23, 2013 at 12:18:58PM -0500, Mark Tinguely wrote: > >>>>Commit f5ea1100 cleans up the disk to host conversions for > >>>>node directory entries, but because a variable is reused in > >>>>xfs_node_toosmall() the next node is not correctly found. > >>>>If the original node is small enough (<= 3/8 of the node size), > >>>>this change may incorrectly cause a node collapse when it should > >>>>not. .... > >>>Also, the "next node" is correctly termed a "sibling", and it's > >>>either the forwards or backwards sibling, not the "next" sibling as > >>>the direction of movement is important. So perhaps this > >>>is better written as: > >>> > >>>"When a node is considered for a merge with a sibling, it overwrites > >>>the sibling pointers of the original node with the sibling's > >>>pointers. This leads to loop considering the original node as a > >>>merge candidate with itself in the second pass, and so it > >>>incorrectly determines a merge should occur." > >>> > >> > >>Are you done ranting? Get the @#$% bug patched. > > > >I'm deeply sorry you feel that way about the review process - it's > >not just code that matters. Experience has shown us time and time > >again that accurate and complete commit messages are extremely > >valuable as they document the symptoms of a problem being fixed and > >why the fix was needed. ..... > > You do not always practice what you preach - here is the commit > message from the patch that caused this whole problem: > > commit f5ea110044fa858925a880b4fa9f551bfa2dfc38 > Author: Dave Chinner <dchinner@redhat.com> > Date: Wed Apr 24 18:58:02 2013 +1000 > > xfs: add CRCs to dir2/da node blocks > > Signed-off-by: Dave Chinner <dchinner@redhat.com> > Reviewed-by: Ben Myers <bpm@sgi.com> > Signed-off-by: Ben Myers <bpm@sgi.com> > > Did this patch just add CRCs to node blocks? no. > > And how about your patches that do not need a commit message at all?! The above commit has a very different context surrounding it - it was in the middle of a major feaure addition series where the process of additions had been described in the previous commits, and the one-line description is sufficient for future readers to determine that the commit is not a bug fix. If a reviewer doesn't speak up about something, then that implies the reviewer considers it acceptible. In this case, the reviewer (Ben) seemed to think the one-line description of the commit was acceptible for the context in which it was being proposed and committed. > Are these exceptions? No, I have done enough reviews of your > patches, and some of your commit message are less than stunning. Sure, some of my commit messages need work, especially as I'm spelling and grammar challenged at the best of times. I make no claim that they are perfect, but I do try to make the sufficient for the context in which they are being proposed. However - it is *your responsibility as a reviewer* to ask the author to write a more complete change log or fix omissions or clarify parts of the commit message that need improvement. If the reviewer does not ask for improvements or you chose not to review the proposed patches, then you have no grounds to complain about the contents of patches that were committed on your watch... > when your preaching, and not so subtle digs at me or SGI gets to the > point of rewriting commit messages even I can't hold my tongue any > more. You are reading way too much into a simple request, Mark. Constructive criticism of your code is not a personal attack. Quite frankly, I don't care who submits the code or who they work for - I apply the same *peer review criteria* to all code that is proposed. I'm here to make sure the code is robust, solid and maintainable into the forseeable future. I'm here to tear your patches apart and then help put it back together better than it was before. It simply doesn't matter who submits the code... > PS. I told Ben that I do not appreciate commit messages being changed. Ben did the right thing. Maintainers rewrite commit messages to improve them all the time... 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] 15+ messages in thread
* Re: [PATCH] xfs: v2 fix node forward in xfs_node_toosmall 2013-09-24 23:33 ` Dave Chinner @ 2013-09-25 18:38 ` Ben Myers 2013-09-25 21:03 ` Eric Sandeen 0 siblings, 1 reply; 15+ messages in thread From: Ben Myers @ 2013-09-25 18:38 UTC (permalink / raw) To: Dave Chinner; +Cc: Mark Tinguely, stable, xfs Hey Dave, On Wed, Sep 25, 2013 at 09:33:31AM +1000, Dave Chinner wrote: > On Tue, Sep 24, 2013 at 04:34:53PM -0500, Mark Tinguely wrote: > > On 09/24/13 16:06, Dave Chinner wrote: > > >On Tue, Sep 24, 2013 at 12:35:44PM -0500, Mark Tinguely wrote: > > >>On 09/23/13 18:48, Dave Chinner wrote: > > >>>On Mon, Sep 23, 2013 at 12:18:58PM -0500, Mark Tinguely wrote: > > >>>>Commit f5ea1100 cleans up the disk to host conversions for > > >>>>node directory entries, but because a variable is reused in > > >>>>xfs_node_toosmall() the next node is not correctly found. > > >>>>If the original node is small enough (<= 3/8 of the node size), > > >>>>this change may incorrectly cause a node collapse when it should > > >>>>not. > .... > > >>>Also, the "next node" is correctly termed a "sibling", and it's > > >>>either the forwards or backwards sibling, not the "next" sibling as > > >>>the direction of movement is important. So perhaps this > > >>>is better written as: > > >>> > > >>>"When a node is considered for a merge with a sibling, it overwrites > > >>>the sibling pointers of the original node with the sibling's > > >>>pointers. This leads to loop considering the original node as a > > >>>merge candidate with itself in the second pass, and so it > > >>>incorrectly determines a merge should occur." > > >>> > > >> > > >>Are you done ranting? Get the @#$% bug patched. > > > > > >I'm deeply sorry you feel that way about the review process - it's > > >not just code that matters. Experience has shown us time and time > > >again that accurate and complete commit messages are extremely > > >valuable as they document the symptoms of a problem being fixed and > > >why the fix was needed. > ..... > > > > You do not always practice what you preach - here is the commit > > message from the patch that caused this whole problem: > > > > commit f5ea110044fa858925a880b4fa9f551bfa2dfc38 > > Author: Dave Chinner <dchinner@redhat.com> > > Date: Wed Apr 24 18:58:02 2013 +1000 > > > > xfs: add CRCs to dir2/da node blocks > > > > Signed-off-by: Dave Chinner <dchinner@redhat.com> > > Reviewed-by: Ben Myers <bpm@sgi.com> > > Signed-off-by: Ben Myers <bpm@sgi.com> > > > > Did this patch just add CRCs to node blocks? no. > > > > And how about your patches that do not need a commit message at all?! > > The above commit has a very different context surrounding it - it > was in the middle of a major feaure addition series where the > process of additions had been described in the previous commits, and > the one-line description is sufficient for future readers to > determine that the commit is not a bug fix. > > If a reviewer doesn't speak up about something, then that implies > the reviewer considers it acceptible. I disagree with that statement. A reviewer might not speak up about a flaw in a work for for other reasons. There are more than two alternatives in this context. For example... > In this case, the reviewer > (Ben) seemed to think the one-line description of the commit was > acceptible for the context in which it was being proposed and > committed. I wasn't happy with the lack of the description in those commits, and probably should have spoken up about it. In this case I weighed the frustration due to the argument that would almost certainly have ensued against the benefit of a decent commit message, and punted. That's similar to what happened when I pulled in the 1st half of the userspace crc-dev work after you refused to address my concerns in review. I punted again, because the argument would have been more nuisance than it was worth. Clearly I need to stop punting so often. But since many a suggestion made by Mark or myself is taken as a bloody travesty and an argument ensues... I have to admit to some fatigue in this area. > > Are these exceptions? No, I have done enough reviews of your > > patches, and some of your commit message are less than stunning. > > Sure, some of my commit messages need work, especially as I'm > spelling and grammar challenged at the best of times. I make no > claim that they are perfect, but I do try to make the sufficient for > the context in which they are being proposed. > > However - it is *your responsibility as a reviewer* to ask the > author to write a more complete change log or fix omissions or > clarify parts of the commit message that need improvement. ... > If the > reviewer does not ask for improvements or you chose not to review > the proposed patches, then you have no grounds to complain about the > contents of patches that were committed on your watch... I disagree. The act of committing a patch does not necessarily imply an agreement regarding it's contents. I am often in a position where I have to commit patches that I don't fully agree with. This doesn't imply that I've waived my right to whine about bad commits later. ;) > > when your preaching, and not so subtle digs at me or SGI gets to the > > point of rewriting commit messages even I can't hold my tongue any > > more. > > You are reading way too much into a simple request, Mark. > Constructive criticism of your code is not a personal attack. I suspect the issue is not really with this particular request. There is a cumulative effect to the toxicity that you seem to feel the need to bring to the list, and it sounds like Mark is full up. This is gonna sound soft coming from a grown man, but I'll call it for what I think it is: Bullying. And to the extent that I've taken part or watched others get planed and done nothing, I think I have to take this as a stain on my own character. The constructive criticism is fine. The bullying needs to stop. Technical prowess is not license to act badly. Knock it off. > Quite frankly, I don't care who submits the code or who they work for > - I apply the same *peer review criteria* to all code that is > proposed. I'm here to make sure the code is robust, solid and > maintainable into the forseeable future. I'm here to tear your patches > apart and then help put it back together better than it was before. It > simply doesn't matter who submits the code... I'm pleased to hear that you don't care who submits code, but it sure seems like you're grinding an axe for Mark to me. > > PS. I told Ben that I do not appreciate commit messages being > > changed. > > Ben did the right thing. Maintainers rewrite commit messages to > improve them all the time... I handled it badly. It was probably ok to pull in your suggestion into the commit message as long as it was attributed properly, but it was more aggressive than was necessary, and I can understand why Mark didn't appreciate it. Also, I really needn't have been snarky about it. Apologies to Mark. Regards, Ben _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] xfs: v2 fix node forward in xfs_node_toosmall 2013-09-25 18:38 ` Ben Myers @ 2013-09-25 21:03 ` Eric Sandeen 2013-09-25 22:11 ` Ben Myers 0 siblings, 1 reply; 15+ messages in thread From: Eric Sandeen @ 2013-09-25 21:03 UTC (permalink / raw) To: Ben Myers; +Cc: Mark Tinguely, xfs [stable@vger.kernel.org stripped from this fascinating thread] On 9/25/13 1:38 PM, Ben Myers wrote: > Hey Dave, > > On Wed, Sep 25, 2013 at 09:33:31AM +1000, Dave Chinner wrote: <snip> >> If a reviewer doesn't speak up about something, then that implies >> the reviewer considers it acceptible. > > I disagree with that statement. A reviewer might not speak up about a > flaw in a work for for other reasons. There are more than two > alternatives in this context. For example... <snip> <apologies if I snipped too much context, not my intention> >> If the >> reviewer does not ask for improvements or you chose not to review >> the proposed patches, then you have no grounds to complain about the >> contents of patches that were committed on your watch... > > I disagree. The act of committing a patch does not necessarily imply an > agreement regarding it's contents. I am often in a position where I > have to commit patches that I don't fully agree with. This doesn't > imply that I've waived my right to whine about bad commits later. ;) If you don't like it, don't add reviewed-by. If you don't like the reviews, don't sign off, don't merge it. As maintainer you have that right, but to be a good maintainer, you need a strong reason. And if you have a strong technical concern, then it's the patch author's duty to take it seriously, or risk not getting the patch merged. And the author might argue back. And other people might argue back. And in the end, if we can all keep our cool, the code will be better for it. If you apply clear and consistent merge criteria then people will know what to expect. When you send a Reviewed-by: that's a pretty strong indication of agreement. There shouldn't be a lot of misunderstanding about what it means; the kernel doc (Documentation/SubmittingPatches) is very clear: === Reviewer's statement of oversight By offering my Reviewed-by: tag, I state that: (a) I have carried out a technical review of this patch to evaluate its appropriateness and readiness for inclusion into the mainline kernel. (b) Any problems, concerns, or questions relating to the patch have been communicated back to the submitter. I am satisfied with the submitter's response to my comments. (c) While there may be things that could be improved with this submission, I believe that it is, at this time, (1) a worthwhile modification to the kernel, and (2) free of known issues which would argue against its inclusion. (d) While I have reviewed the patch and believe it to be sound, I do not (unless explicitly stated elsewhere) make any warranties or guarantees that it will achieve its stated purpose or function properly in any given situation. === If you can't get behind that, don't add your Reviewed-by: and continue to work it out until you can. Reviewers should be sure to pay attention to point (c) as well, something I sometimes forget myself. Reviews don't have to be heeded, either. They don't have to be gauntlets thrown down or lines in the sand. "You've corrupted memory here" is different from "you could use an args struct instead of 8 arguments" or "your commit log isn't very descriptive." Discretion abounds. As a reviewer once said on another list, "I'm free to share what occurs to me and you're free to tell me to go jump in a lake." Especially for cosmetic issues, that's a pretty good POV. Sometimes it's trickier. As maintainer I guess you get to decide if a review concern has enough merit to hold up merging. Ignoring a compelling technical concern could be risky. Maybe one reviewer is dissatisfied, and another is satisfied. Then you get to play Solomon again. Life goes on, I hope. -Eric _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] xfs: v2 fix node forward in xfs_node_toosmall 2013-09-25 21:03 ` Eric Sandeen @ 2013-09-25 22:11 ` Ben Myers 0 siblings, 0 replies; 15+ messages in thread From: Ben Myers @ 2013-09-25 22:11 UTC (permalink / raw) To: Eric Sandeen; +Cc: Mark Tinguely, xfs Hey, On Wed, Sep 25, 2013 at 04:03:06PM -0500, Eric Sandeen wrote: > [stable@vger.kernel.org stripped from this fascinating thread] Good idea. I should have done, in retrospect. > On 9/25/13 1:38 PM, Ben Myers wrote: > > Hey Dave, > > > > On Wed, Sep 25, 2013 at 09:33:31AM +1000, Dave Chinner wrote: > > <snip> > > >> If a reviewer doesn't speak up about something, then that implies > >> the reviewer considers it acceptible. > > > > I disagree with that statement. A reviewer might not speak up about a > > flaw in a work for for other reasons. There are more than two > > alternatives in this context. For example... > > <snip> > > <apologies if I snipped too much context, not my intention> > > >> If the > >> reviewer does not ask for improvements or you chose not to review > >> the proposed patches, then you have no grounds to complain about the > >> contents of patches that were committed on your watch... > > > > I disagree. The act of committing a patch does not necessarily imply an > > agreement regarding it's contents. I am often in a position where I > > have to commit patches that I don't fully agree with. This doesn't > > imply that I've waived my right to whine about bad commits later. ;) > > If you don't like it, don't add reviewed-by. > > If you don't like the reviews, don't sign off, don't merge it. > As maintainer you have that right, but to be a good maintainer, > you need a strong reason. And if you have a strong technical concern, > then it's the patch author's duty to take it seriously, or risk not > getting the patch merged. And the author might argue back. And other > people might argue back. And in the end, if we can all keep our cool, > the code will be better for it. If you apply clear and consistent merge > criteria then people will know what to expect. > > When you send a Reviewed-by: that's a pretty strong indication of agreement. Gah. Yeah, I overstated. Maybe 'does not necessarily imply an agreement' should read something more like 'does not necessarily imply it is beyond criticism', that's a bit more reasonable. > There shouldn't be a lot of misunderstanding about what it means; > the kernel doc (Documentation/SubmittingPatches) is very clear: > > === > Reviewer's statement of oversight > > By offering my Reviewed-by: tag, I state that: > > (a) I have carried out a technical review of this patch to > evaluate its appropriateness and readiness for inclusion into > the mainline kernel. > > (b) Any problems, concerns, or questions relating to the patch > have been communicated back to the submitter. I am satisfied > with the submitter's response to my comments. > > (c) While there may be things that could be improved with this > submission, I believe that it is, at this time, (1) a > worthwhile modification to the kernel, and (2) free of known > issues which would argue against its inclusion. > > (d) While I have reviewed the patch and believe it to be sound, I > do not (unless explicitly stated elsewhere) make any > warranties or guarantees that it will achieve its stated > purpose or function properly in any given situation. > === > > If you can't get behind that, don't add your Reviewed-by: and continue to > work it out until you can. Reviewers should be sure to pay attention to > point (c) as well, something I sometimes forget myself. I haven't read that in awhile. Nice to have a brush up, thanks. I think it probably comes short of '...you have no grounds to complain about the contents of patches that were committed on your watch...', which is the idea I disagree with. Maybe others don't think so, you could argue that I failed on point b in my review by not choosing to hound Dave for a commit message. ;) > Reviews don't have to be heeded, either. They don't have to be gauntlets > thrown down or lines in the sand. "You've corrupted memory here" is > different from "you could use an args struct instead of 8 arguments" > or "your commit log isn't very descriptive." Discretion abounds. > > As a reviewer once said on another list, "I'm free to share > what occurs to me and you're free to tell me to go jump in a lake." > Especially for cosmetic issues, that's a pretty good POV. Sometimes > it's trickier. > > As maintainer I guess you get to decide if a review concern has enough > merit to hold up merging. Ignoring a compelling technical concern > could be risky. I agree that ignoring a compelling technical concern can be risky. > Maybe one reviewer is dissatisfied, and another is satisfied. Then > you get to play Solomon again. Life goes on, I hope. Yeah. Life goes on. Thanks Eric, Ben _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] xfs: fix node forward in xfs_node_toosmall 2013-09-20 22:05 [PATCH] xfs: fix node forward in xfs_node_toosmall Mark Tinguely 2013-09-23 0:08 ` Dave Chinner 2013-09-23 17:18 ` [PATCH] xfs: v2 " Mark Tinguely @ 2013-09-23 21:34 ` Michael L. Semon 2013-09-23 21:45 ` Mark Tinguely 2 siblings, 1 reply; 15+ messages in thread From: Michael L. Semon @ 2013-09-23 21:34 UTC (permalink / raw) To: Mark Tinguely; +Cc: xfs On 09/20/2013 06:05 PM, Mark Tinguely wrote: > Commit f5ea1100 cleans up the disk to host conversions for > node directory entries, but because a variable is reused in > xfs_node_toosmall() the next node is not correctly found. > If the original node is small enough (<= 3/8 of the node size), > this change may incorrectly cause a node collapse when it should > not. That will cause an assert in xfstest generic/319: > > Assertion failed: first <= last && last < BBTOB(bp->b_length), > file: /root/newest/xfs/fs/xfs/xfs_trans_buf.c, line: 569 > > Keep the original node header to get the correct forward node. This works for xfstests generic/319 on 32-bit x86. The xfstests run was okay, with a lockdep in the middle and a crash in xfs/300. Neither the lockdep nor the crash could reproduced by running individual tests one by one. generic/319 itself fails only because it couldn't find its _cleanup routine. I'm not sure if I'm using v1 or v2 of generic/319. The kernel is set to the commit just before the latest aio-next merge, which is causing severe problems here. xfsprogs was the latest production xfsprogs because my copy of xfstests is having trouble checking v4 XFS filesystems otherwise. Will test xfs_node_toosmall patch v2 once I get home. Thanks! Michael _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] xfs: fix node forward in xfs_node_toosmall 2013-09-23 21:34 ` [PATCH] xfs: " Michael L. Semon @ 2013-09-23 21:45 ` Mark Tinguely 0 siblings, 0 replies; 15+ messages in thread From: Mark Tinguely @ 2013-09-23 21:45 UTC (permalink / raw) To: Michael L. Semon; +Cc: xfs On 09/23/13 16:34, Michael L. Semon wrote: > On 09/20/2013 06:05 PM, Mark Tinguely wrote: >> Commit f5ea1100 cleans up the disk to host conversions for >> node directory entries, but because a variable is reused in >> xfs_node_toosmall() the next node is not correctly found. >> If the original node is small enough (<= 3/8 of the node size), >> this change may incorrectly cause a node collapse when it should >> not. That will cause an assert in xfstest generic/319: >> >> Assertion failed: first<= last&& last< BBTOB(bp->b_length), >> file: /root/newest/xfs/fs/xfs/xfs_trans_buf.c, line: 569 >> >> Keep the original node header to get the correct forward node. > > This works for xfstests generic/319 on 32-bit x86. The xfstests > run was okay, with a lockdep in the middle and a crash in xfs/300. > Neither the lockdep nor the crash could reproduced by running > individual tests one by one. > > generic/319 itself fails only because it couldn't find its > _cleanup routine. I'm not sure if I'm using v1 or v2 of > generic/319. > > The kernel is set to the commit just before the latest aio-next > merge, which is causing severe problems here. xfsprogs was the > latest production xfsprogs because my copy of xfstests is having > trouble checking v4 XFS filesystems otherwise. > > Will test xfs_node_toosmall patch v2 once I get home. > > Thanks! > > Michael > 319 v1 had the cleanup and removed in v2 because of feedback. IMO, it should be there even basically empty. --Mark. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2013-09-25 22:11 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-09-20 22:05 [PATCH] xfs: fix node forward in xfs_node_toosmall Mark Tinguely 2013-09-23 0:08 ` Dave Chinner 2013-09-23 13:38 ` Mark Tinguely 2013-09-23 17:18 ` [PATCH] xfs: v2 " Mark Tinguely 2013-09-23 23:48 ` Dave Chinner 2013-09-24 17:35 ` Mark Tinguely 2013-09-24 18:59 ` Ben Myers 2013-09-24 21:06 ` Dave Chinner 2013-09-24 21:34 ` Mark Tinguely 2013-09-24 23:33 ` Dave Chinner 2013-09-25 18:38 ` Ben Myers 2013-09-25 21:03 ` Eric Sandeen 2013-09-25 22:11 ` Ben Myers 2013-09-23 21:34 ` [PATCH] xfs: " Michael L. Semon 2013-09-23 21:45 ` Mark Tinguely
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.