Re: [PATCH v2] ext4: fix hole punch failure when depth is greater than 0
From: Ashish Sangwan <hidden>
Date: 2012-07-09 17:37:44
Also in:
lkml
Ok, the code is not very clear, but now I can see it. p_block is actually misused here to store the number of indexes in the current node while diving into the tree. Then on the way up, we are checking that to see if the eh_entries changed or not (which is indicating that something has been freed deeper in the tree).
True, p_block is misused. We tried to fix the problem with minimum code change.
That said, it makes sense to set it before the loop itself because we are actually skipping the path construction while diving into the tree since patch is already initialized and we're starting walking back from 'depth' up in this case. So the patch seems fine. Thanks for catching it and fixing it. You can add Reviewed-by: Lukas Czerner <redacted>
Thanks for your review.
quoted
quoted
Note: there are some indent problems in your patch, like for example this: + path[k].p_block = + le16_to_cpu(path[k].p_hdr->eh_entries)+1;Before submitting the patch, I run checkpatch.pl with --strict option. It did'nt show any error or warning. Should I re-submit the patch with an extra tab before the second line? The call is yours.checkpatch.pl does not catch everything. Just look at how wrapping of long lines is done in the code, there are plenty of examples.
True again. I will resend patch with proper indentation.
quoted
quoted
Anyway, what do you think about the modification ?Also there is 1 modification missing from your patch. ext4_ext_drop_refs(path); kfree(path); + path = NULL; if (err == -EAGAIN) goto again; If path is not set to NULL, it will crash in xfstest #13. Ted has already reported it.Right, I've probably used the old patch as an example. Thanks! -Lukas
Thanks, Ashish