> 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 <lczerner@redhat.com>>
Thanks for your review.
>>> >>> > 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.
>>>> > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html