Re: [PATCH v5] usb: gadget: f_fs: add no_disconnect mode
Hi, On Thu, Dec 18, 2014 at 09:55:10AM +0100, Robert Baldyga wrote: Since we can compose gadgets from many functions, there is the problem related to gadget breakage while FunctionFS daemon being closed. FFS function is userspace code so there is no way to know when it will close files (it doesn't matter what is the reason of this situation, it can be daemon logic, program breakage, process kill or any other). So when we have another function in gadget which, for example, sends some amount of data, does some software update or implements some real-time functionality, we may want to keep the gadget connected despite FFS function is no longer functional. We can't just remove one of functions from gadget since it has been enumerated, so the only way to keep entire gadget working is to make broken FFS function deactivated but still visible to host. For this purpose this patch introduces no_disconnect mode. It can be enabled by setting mount option no_disconnect=1, and results with defering function disconnect to the moment of reopen ep0 file or filesystem unmount. After closing all endpoint files, FunctionFS is set to state FFS_DEACTIVATED. When ffs-state == FFS_DEACTIVATED: - function is still bound and visible to host, - setup requests are automatically stalled, - transfers on other endpoints are refused, - epfiles, except ep0, are deleted from the filesystem, - opening ep0 causes the function to be closed, and then FunctionFS is ready for descriptors and string write, - altsetting change causes the function to be closed - we want to keep function alive until another functions are potentialy used, altsetting change means that another configuration is being selected or USB cable was unplugged, which indicates that we don't need to stay longer in FFS_DEACTIVATED state - unmounting of the FunctionFS instance causes the function to be closed. Signed-off-by: Robert Baldyga r.bald...@samsung.com Changelog: v5: - close function on altsetting change v4: https://lkml.org/lkml/2014/10/9/224 - use ffs_data_reset() instead of ffs_data_clear() to reset ffs data properly after ffs-ref refcount reach 0 (or under in no_disconnect mode) in ffs_data_put() function v3: https://lkml.org/lkml/2014/10/9/170 - change option name to more descriptive and less scary, - fix cleaning up on unmount (call ffs_data_closed() in ffs_fs_kill_sb(), and ffs_data_clear() in ffs_data_closed() if ffs-opened is negative). v2: https://lkml.org/lkml/2014/10/7/109 - delete epfiles, excepting ep0, when FFS is in zombie mode, - add description of FFS_ZOMBIE state, - minor cleanups. v1: https://lkml.org/lkml/2014/10/6/128 This changelog should be after the tearline, I've manually editted it and also applied the small modification sugested by Michal. It should show up on my testing/next soonish. -- balbi signature.asc Description: Digital signature
Re: [PATCH v5] usb: gadget: f_fs: add no_disconnect mode
On Thu, Dec 18 2014, Robert Baldyga wrote: Since we can compose gadgets from many functions, there is the problem related to gadget breakage while FunctionFS daemon being closed. FFS function is userspace code so there is no way to know when it will close files (it doesn't matter what is the reason of this situation, it can be daemon logic, program breakage, process kill or any other). So when we have another function in gadget which, for example, sends some amount of data, does some software update or implements some real-time functionality, we may want to keep the gadget connected despite FFS function is no longer functional. We can't just remove one of functions from gadget since it has been enumerated, so the only way to keep entire gadget working is to make broken FFS function deactivated but still visible to host. For this purpose this patch introduces no_disconnect mode. It can be enabled by setting mount option no_disconnect=1, and results with defering function disconnect to the moment of reopen ep0 file or filesystem unmount. After closing all endpoint files, FunctionFS is set to state FFS_DEACTIVATED. When ffs-state == FFS_DEACTIVATED: - function is still bound and visible to host, - setup requests are automatically stalled, - transfers on other endpoints are refused, - epfiles, except ep0, are deleted from the filesystem, - opening ep0 causes the function to be closed, and then FunctionFS is ready for descriptors and string write, - altsetting change causes the function to be closed - we want to keep function alive until another functions are potentialy used, altsetting change means that another configuration is being selected or USB cable was unplugged, which indicates that we don't need to stay longer in FFS_DEACTIVATED state - unmounting of the FunctionFS instance causes the function to be closed. Signed-off-by: Robert Baldyga r.bald...@samsung.com Acked-by: Michal Nazarewicz min...@mina86.com @@ -1417,7 +1429,11 @@ static void ffs_data_opened(struct ffs_data *ffs) ENTER(); atomic_inc(ffs-ref); - atomic_inc(ffs-opened); + if (atomic_add_return(1, ffs-opened) == 1) + if (ffs-state == FFS_DEACTIVATED) { + ffs-state = FFS_CLOSING; + ffs_data_reset(ffs); + } Wrong indention. Why not: + if (atomic_add_return(1, ffs-opened) == 1 + ffs-state == FFS_DEACTIVATED) { + ffs-state = FFS_CLOSING; + ffs_data_reset(ffs); + } } static void ffs_data_put(struct ffs_data *ffs) @@ -1439,6 +1455,21 @@ static void ffs_data_closed(struct ffs_data *ffs) -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo-- -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] usb: gadget: f_fs: add no_disconnect mode
On Thu, Dec 18, 2014 at 09:55:10AM +0100, Robert Baldyga wrote: Since we can compose gadgets from many functions, there is the problem related to gadget breakage while FunctionFS daemon being closed. FFS function is userspace code so there is no way to know when it will close files (it doesn't matter what is the reason of this situation, it can be daemon logic, program breakage, process kill or any other). So when we have another function in gadget which, for example, sends some amount of data, does some software update or implements some real-time functionality, we may want to keep the gadget connected despite FFS function is no longer functional. We can't just remove one of functions from gadget since it has been enumerated, so the only way to keep entire gadget working is to make broken FFS function deactivated but still visible to host. For this purpose this patch introduces no_disconnect mode. It can be enabled by setting mount option no_disconnect=1, and results with defering function disconnect to the moment of reopen ep0 file or filesystem unmount. After closing all endpoint files, FunctionFS is set to state FFS_DEACTIVATED. When ffs-state == FFS_DEACTIVATED: - function is still bound and visible to host, - setup requests are automatically stalled, - transfers on other endpoints are refused, - epfiles, except ep0, are deleted from the filesystem, - opening ep0 causes the function to be closed, and then FunctionFS is ready for descriptors and string write, - altsetting change causes the function to be closed - we want to keep function alive until another functions are potentialy used, altsetting change means that another configuration is being selected or USB cable was unplugged, which indicates that we don't need to stay longer in FFS_DEACTIVATED state - unmounting of the FunctionFS instance causes the function to be closed. Signed-off-by: Robert Baldyga r.bald...@samsung.com David, can you test it with your setup ? cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH v5] usb: gadget: f_fs: add no_disconnect mode
On Tue, Dec 23, 2014 at 12:48:58PM -0600, Felipe Balbi wrote: On Thu, Dec 18, 2014 at 09:55:10AM +0100, Robert Baldyga wrote: Since we can compose gadgets from many functions, there is the problem related to gadget breakage while FunctionFS daemon being closed. FFS function is userspace code so there is no way to know when it will close files (it doesn't matter what is the reason of this situation, it can be daemon logic, program breakage, process kill or any other). So when we have another function in gadget which, for example, sends some amount of data, does some software update or implements some real-time functionality, we may want to keep the gadget connected despite FFS function is no longer functional. We can't just remove one of functions from gadget since it has been enumerated, so the only way to keep entire gadget working is to make broken FFS function deactivated but still visible to host. For this purpose this patch introduces no_disconnect mode. It can be enabled by setting mount option no_disconnect=1, and results with defering function disconnect to the moment of reopen ep0 file or filesystem unmount. After closing all endpoint files, FunctionFS is set to state FFS_DEACTIVATED. When ffs-state == FFS_DEACTIVATED: - function is still bound and visible to host, - setup requests are automatically stalled, - transfers on other endpoints are refused, - epfiles, except ep0, are deleted from the filesystem, - opening ep0 causes the function to be closed, and then FunctionFS is ready for descriptors and string write, - altsetting change causes the function to be closed - we want to keep function alive until another functions are potentialy used, altsetting change means that another configuration is being selected or USB cable was unplugged, which indicates that we don't need to stay longer in FFS_DEACTIVATED state - unmounting of the FunctionFS instance causes the function to be closed. Signed-off-by: Robert Baldyga r.bald...@samsung.com David, can you test it with your setup ? Works fine on my side. Tested-by: David Cohen david.a.co...@linux.intel.com cheers -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] usb: gadget: f_fs: add no_disconnect mode
On Tue, Dec 23, 2014 at 12:47:55PM -0800, David Cohen wrote: On Tue, Dec 23, 2014 at 12:48:58PM -0600, Felipe Balbi wrote: On Thu, Dec 18, 2014 at 09:55:10AM +0100, Robert Baldyga wrote: Since we can compose gadgets from many functions, there is the problem related to gadget breakage while FunctionFS daemon being closed. FFS function is userspace code so there is no way to know when it will close files (it doesn't matter what is the reason of this situation, it can be daemon logic, program breakage, process kill or any other). So when we have another function in gadget which, for example, sends some amount of data, does some software update or implements some real-time functionality, we may want to keep the gadget connected despite FFS function is no longer functional. We can't just remove one of functions from gadget since it has been enumerated, so the only way to keep entire gadget working is to make broken FFS function deactivated but still visible to host. For this purpose this patch introduces no_disconnect mode. It can be enabled by setting mount option no_disconnect=1, and results with defering function disconnect to the moment of reopen ep0 file or filesystem unmount. After closing all endpoint files, FunctionFS is set to state FFS_DEACTIVATED. When ffs-state == FFS_DEACTIVATED: - function is still bound and visible to host, - setup requests are automatically stalled, - transfers on other endpoints are refused, - epfiles, except ep0, are deleted from the filesystem, - opening ep0 causes the function to be closed, and then FunctionFS is ready for descriptors and string write, - altsetting change causes the function to be closed - we want to keep function alive until another functions are potentialy used, altsetting change means that another configuration is being selected or USB cable was unplugged, which indicates that we don't need to stay longer in FFS_DEACTIVATED state - unmounting of the FunctionFS instance causes the function to be closed. Signed-off-by: Robert Baldyga r.bald...@samsung.com David, can you test it with your setup ? Works fine on my side. Tested-by: David Cohen david.a.co...@linux.intel.com just to make sure, did you try with and without the new parameter ? I remember someone complaining about regressions when the new parameter was used. cheers -- balbi signature.asc Description: Digital signature
[PATCH v5] usb: gadget: f_fs: add no_disconnect mode
Since we can compose gadgets from many functions, there is the problem related to gadget breakage while FunctionFS daemon being closed. FFS function is userspace code so there is no way to know when it will close files (it doesn't matter what is the reason of this situation, it can be daemon logic, program breakage, process kill or any other). So when we have another function in gadget which, for example, sends some amount of data, does some software update or implements some real-time functionality, we may want to keep the gadget connected despite FFS function is no longer functional. We can't just remove one of functions from gadget since it has been enumerated, so the only way to keep entire gadget working is to make broken FFS function deactivated but still visible to host. For this purpose this patch introduces no_disconnect mode. It can be enabled by setting mount option no_disconnect=1, and results with defering function disconnect to the moment of reopen ep0 file or filesystem unmount. After closing all endpoint files, FunctionFS is set to state FFS_DEACTIVATED. When ffs-state == FFS_DEACTIVATED: - function is still bound and visible to host, - setup requests are automatically stalled, - transfers on other endpoints are refused, - epfiles, except ep0, are deleted from the filesystem, - opening ep0 causes the function to be closed, and then FunctionFS is ready for descriptors and string write, - altsetting change causes the function to be closed - we want to keep function alive until another functions are potentialy used, altsetting change means that another configuration is being selected or USB cable was unplugged, which indicates that we don't need to stay longer in FFS_DEACTIVATED state - unmounting of the FunctionFS instance causes the function to be closed. Signed-off-by: Robert Baldyga r.bald...@samsung.com Changelog: v5: - close function on altsetting change v4: https://lkml.org/lkml/2014/10/9/224 - use ffs_data_reset() instead of ffs_data_clear() to reset ffs data properly after ffs-ref refcount reach 0 (or under in no_disconnect mode) in ffs_data_put() function v3: https://lkml.org/lkml/2014/10/9/170 - change option name to more descriptive and less scary, - fix cleaning up on unmount (call ffs_data_closed() in ffs_fs_kill_sb(), and ffs_data_clear() in ffs_data_closed() if ffs-opened is negative). v2: https://lkml.org/lkml/2014/10/7/109 - delete epfiles, excepting ep0, when FFS is in zombie mode, - add description of FFS_ZOMBIE state, - minor cleanups. v1: https://lkml.org/lkml/2014/10/6/128 --- drivers/usb/gadget/function/f_fs.c | 56 ++ drivers/usb/gadget/function/u_fs.h | 24 2 files changed, 75 insertions(+), 5 deletions(-) diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index 63314ed..d49af15 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -606,6 +606,8 @@ static unsigned int ffs_ep0_poll(struct file *file, poll_table *wait) } case FFS_CLOSING: break; + case FFS_DEACTIVATED: + break; } mutex_unlock(ffs-mutex); @@ -1180,6 +1182,7 @@ struct ffs_sb_fill_data { struct ffs_file_perms perms; umode_t root_mode; const char *dev_name; + bool no_disconnect; struct ffs_data *ffs_data; }; @@ -1250,6 +1253,12 @@ static int ffs_fs_parse_opts(struct ffs_sb_fill_data *data, char *opts) /* Interpret option */ switch (eq - opts) { + case 13: + if (!memcmp(opts, no_disconnect, 13)) + data-no_disconnect = !!value; + else + goto invalid; + break; case 5: if (!memcmp(opts, rmode, 5)) data-root_mode = (value 0555) | S_IFDIR; @@ -1314,6 +1323,7 @@ ffs_fs_mount(struct file_system_type *t, int flags, .gid = GLOBAL_ROOT_GID, }, .root_mode = S_IFDIR | 0500, + .no_disconnect = false, }; struct dentry *rv; int ret; @@ -1330,6 +1340,7 @@ ffs_fs_mount(struct file_system_type *t, int flags, if (unlikely(!ffs)) return ERR_PTR(-ENOMEM); ffs-file_perms = data.perms; + ffs-no_disconnect = data.no_disconnect; ffs-dev_name = kstrdup(dev_name, GFP_KERNEL); if (unlikely(!ffs-dev_name)) { @@ -1361,6 +1372,7 @@ ffs_fs_kill_sb(struct super_block *sb) kill_litter_super(sb); if (sb-s_fs_info) { ffs_release_dev(sb-s_fs_info); + ffs_data_closed(sb-s_fs_info); ffs_data_put(sb-s_fs_info); } } @@ -1417,7 +1429,11 @@ static void ffs_data_opened(struct ffs_data *ffs) ENTER();