On 22.03.2018 16:17, Qu Wenruo wrote: > > > On 2018年03月22日 22:00, David Sterba wrote: >> On Thu, Mar 22, 2018 at 09:53:46PM +0800, Qu Wenruo wrote: >>>>> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c >>>>> index 26484648d090..3866b8ab20f1 100644 >>>>> --- a/fs/btrfs/backref.c >>>>> +++ b/fs/btrfs/backref.c >>>>> @@ -738,7 +738,8 @@ static int add_missing_keys(struct btrfs_fs_info >>>>> *fs_info, >>>>> BUG_ON(ref->key_for_search.type); >>>>> BUG_ON(!ref->wanted_disk_byte); >>>>> >>>>> - eb = read_tree_block(fs_info, ref->wanted_disk_byte, 0); >>>>> + eb = read_tree_block(fs_info, ref->wanted_disk_byte, 0, NULL, >>>>> + 0); >>>> >>>> Please add 2nd function that will take the extended parameters and >>>> keep read_tree_block as is. >>> >>> So for any new caller of read_tree_block(), reviewer is the last person >>> to info the author to use these parameters for safety check? >>> >>> And in fact, the old function should be avoid if possible, I think the >>> new parameters act as a pretty good sign to make any caller double think >>> about this. >> >> I saw half of the new parameters were just 0, NULL, so this looks like a >> lot of code churn and I haven't looked closer if there's a chance to >> fill the parameters in all callsites. So if it's a matter of adding them >> incrementally then fine. >> > I'm afraid some of the call sites (ones I left with NULL, 0) are unable > to pass the new parameters by its nature. > > Such callers include: > 1) Tree root > Just @bytenr and @gen from ROOT_ITEM. No @first_key. > > 2) Backref walker for FULL_BACKREF > Only parent bytenr, no extra info on @first_key. > > But despite of such call sites, every top-down reader should grab first > key and level. (And so I did in the patch). > > BTW, about half of the read_tree_block() callers are using the new > parameters. > So a new function seems a little embarrassing here.
Is it possible to centralise those checks in the read tree verifier, rather than sprinkling them around the code? > > Thanks, > Qu > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
