From: <gregkh@linuxfoundation.org>
To: mina86@mina86.com, chenyu56@huawei.com,
felipe.balbi@linux.intel.com, john.stultz@linaro.org
Cc: <stable@vger.kernel.org>
Subject: FAILED: patch "[PATCH] usb: gadget: f_fs: stop sleeping in ffs_func_eps_disable" failed to apply to 4.8-stable tree
Date: Thu, 17 Nov 2016 08:49:09 +0100 [thread overview]
Message-ID: <14793689499361@kroah.com> (raw)
The patch below does not apply to the 4.8-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
>From a9e6f83c2df199187a5248f824f31b6787ae23ae Mon Sep 17 00:00:00 2001
From: Michal Nazarewicz <mina86@mina86.com>
Date: Tue, 4 Oct 2016 02:07:34 +0200
Subject: [PATCH] usb: gadget: f_fs: stop sleeping in ffs_func_eps_disable
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
ffs_func_eps_disable is called from atomic context so it cannot sleep
thus cannot grab a mutex. Change the handling of epfile->read_buffer
to use non-sleeping synchronisation method.
Reported-by: Chen Yu <chenyu56@huawei.com>
Signed-off-by: Michał Nazarewicz <mina86@mina86.com>
Fixes: 9353afbbfa7b ("buffer data from ‘oversized’ OUT requests")
Tested-by: John Stultz <john.stultz@linaro.org>
Tested-by: Chen Yu <chenyu56@huawei.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index b31aa9572723..e40d47d47d82 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -136,8 +136,60 @@ struct ffs_epfile {
/*
* Buffer for holding data from partial reads which may happen since
* we’re rounding user read requests to a multiple of a max packet size.
+ *
+ * The pointer is initialised with NULL value and may be set by
+ * __ffs_epfile_read_data function to point to a temporary buffer.
+ *
+ * In normal operation, calls to __ffs_epfile_read_buffered will consume
+ * data from said buffer and eventually free it. Importantly, while the
+ * function is using the buffer, it sets the pointer to NULL. This is
+ * all right since __ffs_epfile_read_data and __ffs_epfile_read_buffered
+ * can never run concurrently (they are synchronised by epfile->mutex)
+ * so the latter will not assign a new value to the pointer.
+ *
+ * Meanwhile ffs_func_eps_disable frees the buffer (if the pointer is
+ * valid) and sets the pointer to READ_BUFFER_DROP value. This special
+ * value is crux of the synchronisation between ffs_func_eps_disable and
+ * __ffs_epfile_read_data.
+ *
+ * Once __ffs_epfile_read_data is about to finish it will try to set the
+ * pointer back to its old value (as described above), but seeing as the
+ * pointer is not-NULL (namely READ_BUFFER_DROP) it will instead free
+ * the buffer.
+ *
+ * == State transitions ==
+ *
+ * • ptr == NULL: (initial state)
+ * ◦ __ffs_epfile_read_buffer_free: go to ptr == DROP
+ * ◦ __ffs_epfile_read_buffered: nop
+ * ◦ __ffs_epfile_read_data allocates temp buffer: go to ptr == buf
+ * ◦ reading finishes: n/a, not in ‘and reading’ state
+ * • ptr == DROP:
+ * ◦ __ffs_epfile_read_buffer_free: nop
+ * ◦ __ffs_epfile_read_buffered: go to ptr == NULL
+ * ◦ __ffs_epfile_read_data allocates temp buffer: free buf, nop
+ * ◦ reading finishes: n/a, not in ‘and reading’ state
+ * • ptr == buf:
+ * ◦ __ffs_epfile_read_buffer_free: free buf, go to ptr == DROP
+ * ◦ __ffs_epfile_read_buffered: go to ptr == NULL and reading
+ * ◦ __ffs_epfile_read_data: n/a, __ffs_epfile_read_buffered
+ * is always called first
+ * ◦ reading finishes: n/a, not in ‘and reading’ state
+ * • ptr == NULL and reading:
+ * ◦ __ffs_epfile_read_buffer_free: go to ptr == DROP and reading
+ * ◦ __ffs_epfile_read_buffered: n/a, mutex is held
+ * ◦ __ffs_epfile_read_data: n/a, mutex is held
+ * ◦ reading finishes and …
+ * … all data read: free buf, go to ptr == NULL
+ * … otherwise: go to ptr == buf and reading
+ * • ptr == DROP and reading:
+ * ◦ __ffs_epfile_read_buffer_free: nop
+ * ◦ __ffs_epfile_read_buffered: n/a, mutex is held
+ * ◦ __ffs_epfile_read_data: n/a, mutex is held
+ * ◦ reading finishes: free buf, go to ptr == DROP
*/
- struct ffs_buffer *read_buffer; /* P: epfile->mutex */
+ struct ffs_buffer *read_buffer;
+#define READ_BUFFER_DROP ((struct ffs_buffer *)ERR_PTR(-ESHUTDOWN))
char name[5];
@@ -736,25 +788,47 @@ static void ffs_epfile_async_io_complete(struct usb_ep *_ep,
schedule_work(&io_data->work);
}
+static void __ffs_epfile_read_buffer_free(struct ffs_epfile *epfile)
+{
+ /*
+ * See comment in struct ffs_epfile for full read_buffer pointer
+ * synchronisation story.
+ */
+ struct ffs_buffer *buf = xchg(&epfile->read_buffer, READ_BUFFER_DROP);
+ if (buf && buf != READ_BUFFER_DROP)
+ kfree(buf);
+}
+
/* Assumes epfile->mutex is held. */
static ssize_t __ffs_epfile_read_buffered(struct ffs_epfile *epfile,
struct iov_iter *iter)
{
- struct ffs_buffer *buf = epfile->read_buffer;
+ /*
+ * Null out epfile->read_buffer so ffs_func_eps_disable does not free
+ * the buffer while we are using it. See comment in struct ffs_epfile
+ * for full read_buffer pointer synchronisation story.
+ */
+ struct ffs_buffer *buf = xchg(&epfile->read_buffer, NULL);
ssize_t ret;
- if (!buf)
+ if (!buf || buf == READ_BUFFER_DROP)
return 0;
ret = copy_to_iter(buf->data, buf->length, iter);
if (buf->length == ret) {
kfree(buf);
- epfile->read_buffer = NULL;
- } else if (unlikely(iov_iter_count(iter))) {
+ return ret;
+ }
+
+ if (unlikely(iov_iter_count(iter))) {
ret = -EFAULT;
} else {
buf->length -= ret;
buf->data += ret;
}
+
+ if (cmpxchg(&epfile->read_buffer, NULL, buf))
+ kfree(buf);
+
return ret;
}
@@ -783,7 +857,15 @@ static ssize_t __ffs_epfile_read_data(struct ffs_epfile *epfile,
buf->length = data_len;
buf->data = buf->storage;
memcpy(buf->storage, data + ret, data_len);
- epfile->read_buffer = buf;
+
+ /*
+ * At this point read_buffer is NULL or READ_BUFFER_DROP (if
+ * ffs_func_eps_disable has been called in the meanwhile). See comment
+ * in struct ffs_epfile for full read_buffer pointer synchronisation
+ * story.
+ */
+ if (unlikely(cmpxchg(&epfile->read_buffer, NULL, buf)))
+ kfree(buf);
return ret;
}
@@ -1097,8 +1179,7 @@ ffs_epfile_release(struct inode *inode, struct file *file)
ENTER();
- kfree(epfile->read_buffer);
- epfile->read_buffer = NULL;
+ __ffs_epfile_read_buffer_free(epfile);
ffs_data_closed(epfile->ffs);
return 0;
@@ -1724,24 +1805,20 @@ static void ffs_func_eps_disable(struct ffs_function *func)
unsigned count = func->ffs->eps_count;
unsigned long flags;
+ spin_lock_irqsave(&func->ffs->eps_lock, flags);
do {
- spin_lock_irqsave(&func->ffs->eps_lock, flags);
/* pending requests get nuked */
if (likely(ep->ep))
usb_ep_disable(ep->ep);
++ep;
- if (epfile)
- epfile->ep = NULL;
- spin_unlock_irqrestore(&func->ffs->eps_lock, flags);
if (epfile) {
- mutex_lock(&epfile->mutex);
- kfree(epfile->read_buffer);
- epfile->read_buffer = NULL;
- mutex_unlock(&epfile->mutex);
+ epfile->ep = NULL;
+ __ffs_epfile_read_buffer_free(epfile);
++epfile;
}
} while (--count);
+ spin_unlock_irqrestore(&func->ffs->eps_lock, flags);
}
static int ffs_func_eps_enable(struct ffs_function *func)
next reply other threads:[~2016-11-17 7:48 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-17 7:49 gregkh [this message]
2016-11-18 21:44 ` FAILED: patch "[PATCH] usb: gadget: f_fs: stop sleeping in ffs_func_eps_disable" failed to apply to 4.8-stable tree Michal Nazarewicz
2016-11-19 8:55 ` Greg KH
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=14793689499361@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=chenyu56@huawei.com \
--cc=felipe.balbi@linux.intel.com \
--cc=john.stultz@linaro.org \
--cc=mina86@mina86.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.