On 23 Jul 2020, at 18:42, Jakub Kicinski wrote:

On Thu, 23 Jul 2020 14:58:31 +0200 Eelco Chaudron wrote:

quoted

+	if ((size & (size - 1)) != 0 ||

is_power_of_2() ?

Was not aware of this macro, will replace…

quoted

+	    (size * sizeof(struct mask_cache_entry)) > PCPU_MIN_UNIT_SIZE)
+		return NULL;

quoted

+	new->cache_size = size;
+	if (new->cache_size > 0) {
+		cache = __alloc_percpu(sizeof(struct mask_cache_entry) *
+				       new->cache_size,

array_size() ?

Will change to:

		cache = __alloc_percpu(array_size(
					       sizeof(struct mask_cache_entry),
					       new->cache_size),
				       __alignof__(struct mask_cache_entry));
		if (!cache) {

quoted

+				       __alignof__(struct mask_cache_entry));
+

No need for the new line here

Will remove

quoted

+		if (!cache) {
+			kfree(new);
+			return NULL;
+		}
+	}

quoted

+	if (size == mc->cache_size || (size & (size - 1)) != 0)

why check "is power of 2" twice?

Will remove as it will always call tbl_mask_cache_alloc() which has the 
check above.

quoted

@@ -454,7 +516,7 @@ void ovs_flow_tbl_destroy(struct flow_table 

*table)
 	struct table_instance *ti = rcu_dereference_raw(table->ti);
 	struct table_instance *ufid_ti = 
rcu_dereference_raw(table->ufid_ti);

-	free_percpu(table->mask_cache);
+	call_rcu(&table->mask_cache->rcu, mask_cache_rcu_cb);
 	call_rcu(&table->mask_array->rcu, mask_array_rcu_cb);
 	table_instance_destroy(table, ti, ufid_ti, false);
 }

This adds a new warning :(

net/openvswitch/flow_table.c:519:24: warning: incorrect type in 
argument 1 (different address spaces)
net/openvswitch/flow_table.c:519:24:    expected struct callback_head 
*head
net/openvswitch/flow_table.c:519:24:    got struct callback_head 
[noderef] __rcu *

:( Yes my diff with older did not work… Sure I’ve fixed them all now 
:)


Will sent out a V3 after some testing…