On 02/05/2016 06:24 PM, Gwendal Grignou wrote: > We should not used kmalloc when get events, these functions are called > quite often. > Gwendal. > > On Fri, Feb 5, 2016 at 5:32 AM, Tomeu Vizoso <tomeu.vizoso@xxxxxxxxxxxxx > <mailto:tomeu.vizoso@xxxxxxxxxxxxx>> wrote: > > +static int cros_ec_get_host_command_version_mask(struct > cros_ec_device *ec_dev, > + u16 cmd, u32 *mask) > +{ > + struct ec_params_get_cmd_versions *pver; > + struct ec_response_get_cmd_versions *rver; > + struct cros_ec_command *msg; > + int ret; > + > + msg = kmalloc(sizeof(*msg) + max(sizeof(rver), sizeof(pver)), > + GFP_KERNEL); > + if (!msg) > + return -ENOMEM; > > Victor's version in https://chromium-review.googlesource.com/272954 > looks cleaner: no malloc, no need to cast rver. I agree that it looks cleaner, but how would you allocate the payload at build time if it has to be max(sizeof(*pver), sizeof(*rver))? > +static int get_next_event(struct cros_ec_device *ec_dev) > +{ > + struct cros_ec_command *msg; > + int ret; > + > + msg = kmalloc(sizeof(*msg) + sizeof(ec_dev->event_data), > GFP_KERNEL); > + if (!msg) > + return -ENOMEM; > > Same grip about malloc(). This function will be called very often when > sensors are used. Ok. > +static int get_keyboard_state_event(struct cros_ec_device *ec_dev) > +{ > + struct cros_ec_command *msg; > + > + msg = kmalloc(sizeof(*msg) + sizeof(ec_dev->event_data.data), > + GFP_KERNEL); > + if (!msg) > + return -ENOMEM; > > Given this command will be used on every key press, it is better to > pre-allocate data (like > in https://chromium-review.googlesource.com/276768) and use msg on the > stack like Victor's changes. Ok. Thanks, Tomeu -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
