> 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@xxxxxxxxxx> > 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
