On 2013/2/7 9:00, Tejun Heo wrote:
> (cc'ing Li)
>
> On Wed, Feb 6, 2013 at 4:55 PM, Andrew Morton <a...@linux-foundation.org>
> wrote:
>> Problems in cgroup_load_subsys().
>>
>> In linux-next, that function is now using the `node' storage which your
>> patch removes:
>>
>> @@ -4503,23 +4525,17 @@ int __init_or_module cgroup_load_subsys(struct
>> cgroup_subsys *ss)
>> * this is all done under the css_set_lock.
>> */
>> write_lock(&css_set_lock);
>> - for (i = 0; i < CSS_SET_TABLE_SIZE; i++) {
>> - struct css_set *cg;
>> - struct hlist_node *node, *tmp;
>> - struct hlist_head *bucket = &css_set_table[i], *new_bucket;
>> -
>> - hlist_for_each_entry_safe(cg, node, tmp, bucket, hlist) {
>> - /* skip entries that we already rehashed */
>> - if (cg->subsys[ss->subsys_id])
>> - continue;
>> - /* remove existing entry */
>> - hlist_del(&cg->hlist);
>> - /* set new value */
>> - cg->subsys[ss->subsys_id] = css;
>> - /* recompute hash and restore entry */
>> - new_bucket = css_set_hash(cg->subsys);
>> - hlist_add_head(&cg->hlist, new_bucket);
>> - }
>> + hash_for_each_safe(css_set_table, i, node, tmp, cg, hlist) {
>> + /* skip entries that we already rehashed */
>> + if (cg->subsys[ss->subsys_id])
>> + continue;
>> + /* remove existing entry */
>> + hash_del(&cg->hlist);
>> + /* set new value */
>> + cg->subsys[ss->subsys_id] = css;
>> + /* recompute hash and restore entry */
>> + key = css_set_hash(cg->subsys);
>> + hash_add(css_set_table, node, key); <<<<---- here
>> }
>> write_unlock(&css_set_lock);
>>
>>
>>
>> This didn't show up (apart from a "used unintialized" warning) because
>> your patch forgot to remove the definition of `node'.
>>
>> I did this. Tejun, could you please opine?
>>
>>
>> @@ -4456,7 +4455,7 @@ int __init_or_module cgroup_load_subsys(
>> {
>> struct cgroup_subsys_state *css;
>> int i, ret;
>> - struct hlist_node *node, *tmp;
>> + struct hlist_node *tmp;
>> struct css_set *cg;
>> unsigned long key;
>>
>> @@ -4534,7 +4533,7 @@ int __init_or_module cgroup_load_subsys(
>> cg->subsys[ss->subsys_id] = css;
>> /* recompute hash and restore entry */
>> key = css_set_hash(cg->subsys);
>> - hash_add(css_set_table, node, key);
>> + hash_add(css_set_table, &cg->hlist, key);
>> }
>> write_unlock(&css_set_lock);
>
> Yeah, looks correct & better to me. Li?
>
obviously correct.
--
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/
