On 8/5/24 1:44 PM, Amery Hung wrote:
Maybe we should move the common btf used by kptr and graph_root into
btf_record and let the callers of btf_parse_fields() and
btf_record_free() to decide the life cycle of btf in btf_record.
Could you maybe explain if and why moving btf of btf_field_kptr and
btf_field_graph_root to btf_record is necessary? I think letting
callers of btf_parse_fields() and btf_record_free() decide whether or
not to change refcount should be enough. Besides, I personally would
like to keep individual btf in btf_field_kptr and
btf_field_graph_root, so that later we can have special fields
referencing different btf.
Sorry, I didn't express the rough idea clearly enough. I didn't mean to
move btf of btf_field_kptr and btf_field_graph_root to btf_record,
because there are other btf-s which are different with the btf which
creates the struct_meta_tab. What I was trying to suggest is to save one
btf in btf_record and hope it will simplify the pin and the unpin of btf
in btf_record:
1) save the btf which owns the btf_record in btf_record.
2) during btf_parse_kptr() or similar, if the used btf is the same as
the btf in btf_record, there is no need to pin the btf
I assume the used btf is the one that btf_parse is working on.
3) when freeing the btf_record, if the btf saved in btf_field is the
same as the btf in btf_record, there is no need to put it
For btf_field_kptr.btf, is it the same as testing the btf_field_kptr.btf is
btf_is_kernel() or not? How about only does btf_get/put for btf_is_kernel()?
IIUC. It will not be the same. For a map referencing prog btf, I
suppose we should still do btf_get().
I think the core idea is since a btf_record and the prog btf
containing it has the same life time, we don't need to
btf_get()/btf_put() in btf_parse_kptr()/btf_record_free() when a
btf_field_kptr.btf is referencing itself.
However, since btf_parse_kptr() called from btf_parse() and
map_check_btf() all use prog btf, we need a way to differentiate the
two. Hence Hou suggested saving the owner's btf in btf_record, and
map_check_btf() calls btf_parse_kptr(map->btf).
I am missing how it is different from the
btf_new_fd()=>btf_parse()=>btf_parse_kptr(new_btf).
akaik, the map->record has no issue now because bpf_map_free_deferred() does
btf_record_free(map->record) before btf_put(map->btf). In the map->record case,
does the map->record need to take a refcnt of the btf_field_kptr.btf if the
btf_field_kptr.btf is pointing back to itself (map->btf) which is not a kernel btf?
then check if btf_record->btf is the same as the btf_field_kptr.btf in
btf_parse_kptr()/btf_record_free().
I suspect it will have the same end result? The btf_field_kptr.btf is only the
same as the owner's btf when btf_parse_kptr() cannot found the kptr type from a
kernel's btf (the id == -ENOENT case in btf_parse_kptr).
