lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 27 Oct 2010 09:47:30 +0100 From: Grant Likely <grant.likely@...retlab.ca> To: ilkka.koskinen@...ia.com Cc: linux-input@...r.kernel.org, dmitry.torokhov@...il.com, spi-devel-general@...ts.sourceforge.net, linux-kernel@...r.kernel.org Subject: Re: [PATCH] input: spi: Driver for SPI data stream driven vibrator On Tue, Oct 26, 2010 at 06:50:33PM +0200, ilkka.koskinen@...ia.com wrote: > Hi Grant and thanks for comments, [...] > >> +static int vibra_spi_playback(struct input_dev *input, int effect_id, > >int value) > >> +{ > >> + struct vibra_data *vibra = input_get_drvdata(input); > >> + struct effect_info *einfo = &vibra->effects[effect_id]; > >> + struct ff_effect *ff_effect = &input->ff->effects[effect_id]; > >> + > >> + if (!vibra->workqueue) > >> + return -ENODEV; > >> + > >> + if (test_bit(FF_EFFECT_UPLOADING, &einfo->flags)) > >> + return -EBUSY; > >> + > >> + if (value == 0) { > >> + /* Abort the given effect */ > >> + if (test_bit(FF_EFFECT_PLAYING, &einfo->flags)) > >> + __set_bit(FF_EFFECT_ABORTING, &einfo->flags); > >> + > >> + __clear_bit(FF_EFFECT_QUEUED, &einfo->flags); > >> + } else { > >> + /* Move the given effect as the next one */ > >> + __clear_bit(FF_EFFECT_QUEUED, > >> + &vibra->effects[vibra->next_effect].flags); > >> + > >> + vibra->next_effect = effect_id; > >> + __set_bit(FF_EFFECT_QUEUED, &einfo->flags); > >> + __clear_bit(FF_EFFECT_ABORTING, &einfo->flags); > >> + einfo->stop_at = jiffies + > >> + msecs_to_jiffies(ff_effect->replay.length); > >> + > >> + if (vibra->status == IDLE) { > >> + vibra->status = STARTED; > >> + queue_work(vibra->workqueue, &vibra->play_work); > >> + } > >> + } > > > >I can't speak anything about the input event handling because I'm not > >very familiar with it. However, it looks like the shared effect data > >(vibra->effects) is getting modified outside of a critical region. Is > >this safe? Hmmm, I don't know why the force feedback layer is using a spin lock, but it looks like overkill. Since you're already deferring work, I would look at queueing the request and pushing down the spin lock exposure as much as possible, but I'm really not the expert on the input layer. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists