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. Thanks, Qu
Attachment:
signature.asc
Description: OpenPGP digital signature
