On 2015/1/21 9:10, Rafael J. Wysocki wrote:
> On Thursday, January 08, 2015 10:33:04 AM Jiang Liu wrote:
<snit>
>> diff --git a/kernel/resource.c b/kernel/resource.c
>> index 0bcebffc4e77..414183809383 100644
>> --- a/kernel/resource.c
>> +++ b/kernel/resource.c
>> @@ -1529,6 +1529,54 @@ int iomem_is_exclusive(u64 addr)
>> return err;
>> }
>>
>> +struct resource_list_entry *resource_list_alloc(struct resource *res,
>> + size_t extra_size)
>
> What about create_resource_list_entry()? Less confusing surely.
Sure, I will rename it as resource_list_create_entry().
>
>> +{
>> + struct resource_list_entry *entry;
>> +
>> + entry = kzalloc(sizeof(*entry) + extra_size, GFP_KERNEL);
>> + if (entry) {
>> + INIT_LIST_HEAD(&entry->node);
>> + entry->res = res ? res : &entry->__res;
>> + }
>> +
>> + return entry;
>> +}
>> +EXPORT_SYMBOL(resource_list_alloc);
>> +
>> +void resource_list_free(struct resource_list_entry *entry)
>> +{
>> + kfree(entry);
>> +}
>> +EXPORT_SYMBOL(resource_list_free);
>
> Well, I'm not sure I like this. The name suggests that it would free the
> entire list and what's wrong with using kfree() directly on "entry" anyway?
I just want to make interface symmetric. We may also support some type
of callback when freeing resources in future.
>
>> +
>> +void resource_list_insert(struct list_head *head,
>> + struct resource_list_entry *entry, bool tail)
>
> I would call this resource_list_add() if anything.
>
> Also it may be better to have two helpers, one for "add" and one for
> "add_tail"
> (and perhaps define them as static inline?).
We can't use inline functions here because that needs pulling list.h
into ioport.h, then causing building issues to header inclusion order.
>
> And why change the ordering between "head" and "entry". That's alomost
> guaranteed to confuse people.
My fault, will change in next version.
>
>> +{
>> + if (tail)
>> + list_add_tail(&entry->node, head);
>> + else
>> + list_add(&entry->node, head);
>> +}
>> +EXPORT_SYMBOL(resource_list_insert);
>> +
>> +void resource_list_delete(struct resource_list_entry *entry)
>> +{
>> + list_del(&entry->node);
>> +}
>> +EXPORT_SYMBOL(resource_list_delete);
>
> Couldn't this be a static inline)?
Inline will cause header file inclusion order issue:(
>
> Or maybe we can combine the "list_del" with "kfree" in one function?
There are callers which need separating list_del from kfree,
so exported two interfaces here. Will add another helper interface
resource_list_destroy_entry().
Regards!
Gerry
>
>> +
>> +void resource_list_free_list(struct list_head *head)
>> +{
>> + struct resource_list_entry *entry, *tmp;
>> +
>> + list_for_each_entry_safe(entry, tmp, head, node) {
>> + list_del(&entry->node);
>> + resource_list_free(entry);
>> + }
>> +}
>> +EXPORT_SYMBOL(resource_list_free_list);
>> +
>> static int __init strict_iomem(char *str)
>> {
>> if (strstr(str, "relaxed"))
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
