| Date | Sat, 9 Jul 2005 14:16:12 +0200 | From | Pavel Machek <> | Subject | Re: [PATCH] [37/48] Suspend2 2.1.9.8 for 2.6.12: 613-pageflags.patch |
| |
Hi!
You know what I think about plugins, right? There are already plugins at block device level, like raid5, LVM, device mapper... I do not see why you should need another one.
> +/* > + * expected_compression_ratio > + * > + * Returns the expected ratio between the amount of memory > + * to be saved and the amount of space required on the > + * storage device. > + */ > +int expected_compression_ratio(void) > +{ > + struct suspend_plugin_ops * this_filter; > + unsigned long ratio = 100; > + > + list_for_each_entry(this_filter, &suspend_filters, ops.filter.filter_list) { > + if (this_filter->disabled) > + continue; > + if (this_filter->ops.filter.expected_compression) > + ratio = ratio * this_filter->ops.filter.expected_compression() / 100; > + } > + > + return (int) ratio; > +}
Why the cast and why long in the first place?
> +struct suspend_plugin_ops * find_plugin_given_name(char * name) > +{ > + struct suspend_plugin_ops * this_plugin, * found_plugin = NULL; > + > + list_for_each_entry(this_plugin, &suspend_plugins, plugin_list) { > + if (!strcmp(name, this_plugin->name)) { > + found_plugin = this_plugin; > + break; > + } > + } > + > + return found_plugin; > +}
How often are you doing this? Would hash table be prefered? How many plugins do you have? Maybe statical registration is right solution?
Pavel -- teflon -- maybe it is a trademark, but it should not be. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|