On Fri, May 22 2015, Krzysztof Opasiak wrote:
> Originally FFS_FL_CALL_CLOSED_CALLBACK flag has been used to
> indicate if we should call ffs_closed_callback().
>
> Commit 4b187fceec3c ("usb: gadget: FunctionFS: add devices
> management code") changed its semantic to indicate if we should
> call ffs_closed() function which does a little bit more.
>
> This situation leads to:
>
> [ 122.362269] ------------[ cut here ]------------
> [ 122.362287] WARNING: CPU: 2 PID: 2384 at
> drivers/usb/gadget/function/f_fs.c:3417 ffs_ep0_write+0x730/0x810 [usb_f_fs]()
> [ 122.362292] Modules linked in:
> [ 122.362555] CPU: 2 PID: 2384 Comm: adbd Tainted: G W
> 4.1.0-0.rc4.git0.1.1.fc22.i686 #1
> [ 122.362561] Hardware name: To be filled by O.E.M. To be filled by
> O.E.M./Aptio CRB, BIOS 5.6.5 07/25/2014
> [ 122.362567] c0d1f947 415badfa 00000000 d1029e64 c0a86e54 00000000
> d1029e94 c045b937
> [ 122.362584] c0c37f94 00000002 00000950 f9b313d4 00000d59 f9b2ebf0
> f9b2ebf0 fffffff0
> [ 122.362600] 00000003 deb53d00 d1029ea4 c045ba42 00000009 00000000
> d1029f08 f9b2ebf0
> [ 122.362617] Call Trace:
> [ 122.362633] [<c0a86e54>] dump_stack+0x41/0x52
> [ 122.362645] [<c045b937>] warn_slowpath_common+0x87/0xc0
> [ 122.362658] [<f9b2ebf0>] ? ffs_ep0_write+0x730/0x810 [usb_f_fs]
> [ 122.362668] [<f9b2ebf0>] ? ffs_ep0_write+0x730/0x810 [usb_f_fs]
> [ 122.362678] [<c045ba42>] warn_slowpath_null+0x22/0x30
> [ 122.362689] [<f9b2ebf0>] ffs_ep0_write+0x730/0x810 [usb_f_fs]
> [ 122.362702] [<f9b2e4c0>] ? ffs_ep0_read+0x380/0x380 [usb_f_fs]
> [ 122.362712] [<c05a1c1f>] __vfs_write+0x2f/0x100
> [ 122.362722] [<c05a42f2>] ? __sb_start_write+0x52/0x110
> [ 122.362731] [<c05a2534>] vfs_write+0x94/0x1b0
> [ 122.362740] [<c0a8a1c0>] ? mutex_lock+0x10/0x30
> [ 122.362749] [<c05a2f41>] SyS_write+0x51/0xb0
> [ 122.362759] [<c0a8c71f>] sysenter_do_call+0x12/0x12
> [ 122.362766] ---[ end trace 0673d3467cecf8db ]---
>
> in some cases (reproduction path below). This commit get back
> semantic of that flag and ensures that ffs_closed() is called
> always when needed but ffs_closed_callback() is called only
> if this flag is set.
>
> Reproduction path:
> Compile kernel without any UDC driver or bound some gadget
> to existing one and then:
>
> $ modprobe g_ffs
> $ mount none -t functionfs mount_point
> $ ffs-example mount_point
>
> This will fail with -ENODEV as there is no udc.
>
> $ ffs-example mount_point
>
> This will fail with -EBUSY because ffs_data has not been
> properly cleaned up.
>
> Signed-off-by: Krzysztof Opasiak <[email protected]>
Acked-by: Michal Nazarewicz <[email protected]>
> ---
> drivers/usb/gadget/function/f_fs.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_fs.c
> b/drivers/usb/gadget/function/f_fs.c
> index 6bdb570..71f68c4 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -315,7 +315,6 @@ static ssize_t ffs_ep0_write(struct file *file, const
> char __user *buf,
> return ret;
> }
>
> - set_bit(FFS_FL_CALL_CLOSED_CALLBACK, &ffs->flags);
> return len;
> }
> break;
> @@ -1463,8 +1462,7 @@ static void ffs_data_clear(struct ffs_data *ffs)
> {
> ENTER();
>
> - if (test_and_clear_bit(FFS_FL_CALL_CLOSED_CALLBACK, &ffs->flags))
> - ffs_closed(ffs);
> + ffs_closed(ffs);
>
> BUG_ON(ffs->gadget);
>
> @@ -3422,9 +3420,13 @@ static int ffs_ready(struct ffs_data *ffs)
> ffs_obj->desc_ready = true;
> ffs_obj->ffs_data = ffs;
>
> - if (ffs_obj->ffs_ready_callback)
> + if (ffs_obj->ffs_ready_callback) {
> ret = ffs_obj->ffs_ready_callback(ffs);
> + if (ret)
> + goto done;
> + }
>
> + set_bit(FFS_FL_CALL_CLOSED_CALLBACK, &ffs->flags);
> done:
> ffs_dev_unlock();
> return ret;
> @@ -3443,7 +3445,8 @@ static void ffs_closed(struct ffs_data *ffs)
>
> ffs_obj->desc_ready = false;
>
> - if (ffs_obj->ffs_closed_callback)
> + if (test_and_clear_bit(FFS_FL_CALL_CLOSED_CALLBACK, &ffs->flags) &&
> + ffs_obj->ffs_closed_callback)
> ffs_obj->ffs_closed_callback(ffs);
>
> if (!ffs_obj->opts || ffs_obj->opts->no_configfs
> --
> 1.7.9.5
>
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +--<[email protected]>--<xmpp:[email protected]>--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
