Re: [PATCH v3 RESEND 2/2] tpm: add support for nonblocking operation

From: James Bottomley
Date: Mon Aug 06 2018 - 19:05:52 EST


On Mon, 2018-08-06 at 14:14 -0700, Tadeusz Struk wrote:
[...]
> +static void tpm_async_work(struct work_struct *work)
> +{
> + struct file_priv *priv =
> + container_of(work, struct file_priv,
> async_work);
> + ssize_t ret;
> +
> + ret = tpm_transmit(priv->chip, priv->space, priv-
> >data_buffer,
> + ÂÂÂsizeof(priv->data_buffer), 0);

Here' you assume the buffer_mutex was taken in write, which is done
(see below). However, here, since there was no change to tpm_transmit,
you'll sleep in the context of the worker queue waiting for the command
to complete and return.

> + tpm_put_ops(priv->chip);
> + if (ret > 0) {
> + priv->data_pending = ret;
> + mod_timer(&priv->user_read_timer, jiffies + (120 *
> HZ));
> + }
> + mutex_unlock(&priv->buffer_mutex);

But you don't release buffer_mutex here until the tpm command has
completed.

> + wake_up_interruptible(&priv->async_wait);
> +}
> +

[...]
> @@ -118,25 +155,48 @@ ssize_t tpm_common_write(struct file *file,
> const char __user *buf,
> Â Â* the char dev is held open.
> Â Â*/
> Â if (tpm_try_get_ops(priv->chip)) {
> - mutex_unlock(&priv->buffer_mutex);
> - return -EPIPE;
> + ret = -EPIPE;
> + goto out;
> Â }
> - out_size = tpm_transmit(priv->chip, priv->space, priv-
> >data_buffer,
> - sizeof(priv->data_buffer), 0);
> Â
> - tpm_put_ops(priv->chip);
> - if (out_size < 0) {
> - mutex_unlock(&priv->buffer_mutex);
> - return out_size;
> + /*
> + Â* If in nonblocking mode schedule an async job to send
> + Â* the command return the size.
> + Â* In case of error the err code will be returned in
> + Â* the subsequent read call.
> + Â*/
> + if (file->f_flags & O_NONBLOCK) {
> + queue_work(tpm_dev_wq, &priv->async_work);
> + return size;

Here you return holding the buffer_mutex, waiting for tpm_async_work to
release it.

But now I've written my tpm work item and got it queued, I can't write
another one without blocking on the buffer_mutex at the top of
tpm_common_write(), and since that doesn't get released until the
previous command completed, I can only queue one command before I
block. For an async interface, shouldn't I be able to queue an
arbitrary number of commands without blocking?

James