lkml.org 
[lkml]   [2014]   [Oct]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] usb: gadget: f_fs: add "zombie" mode
On 10/07/2014 12:07 PM, Michal Nazarewicz wrote:
> On Tue, Oct 07 2014, Robert Baldyga <r.baldyga@samsung.com> wrote:
>> @@ -1411,8 +1425,17 @@ static void ffs_data_closed(struct ffs_data *ffs)
>> ENTER();
>>
>> if (atomic_dec_and_test(&ffs->opened)) {
>> - ffs->state = FFS_CLOSING;
>> - ffs_data_reset(ffs);
>> + if (ffs->zombie_mode) {
>> + ffs->state = FFS_ZOMBIE;
>> + if (ffs->epfiles)
>> + ffs_epfiles_delete(ffs->epfiles,
>> + ffs->eps_count);
>
> This looks suspicious. Why isn't it:
>
> + if (ffs->epfiles) {
> + ffs_epfiles_destroy(ffs->epfiles,
> + ffs->eps_count);
> + ffs->epfiles = NULL;
> + }
>
> If ffs->epfiles is not NULLed, call to ffs_data_reset in ffs_data_open
> will call ffs_epfiles_destroy which we don't want, do we?
>

We do. When epfile->dentry == NULL, ffs_epfiles_destroy() will do only
kfree(epfiles). We want to do it.

We don't want to have ffs->epfiles being equal NULL before calling
ffs_func_eps_disable(), which iterates on this table.

Hmm, we could also change ffs_func_eps_disable() to not touch
ffs->epfiles if it's NULL. It seems to be better solution.

>> + if (ffs->setup_state == FFS_SETUP_PENDING)
>> + __ffs_ep0_stall(ffs);
>> + } else {
>> + ffs->state = FFS_CLOSING;
>> + ffs_data_reset(ffs);
>> + }
>> }
>>
>> ffs_data_put(ffs);
>
>> @@ -93,6 +93,26 @@ enum ffs_state {
>> FFS_ACTIVE,
>>
>> /*
>> + * Function is visible to host, but it's not functional. All
>> + * setup requests are stalled and another transfers are refused.
>
> “and transfers on other endpoints are refused.”
>
>> + * All epfiles, excepting ep0, are deleted so there is no way
>
> s/excepting/except/
>
>> + * to perform any operations on them.
>> + *
>> + * This state is set after closing all functionfs files, when
>> + * mount parameter "zombie=1" has been set. Function will remain
>> + * in zombie state until filesystem will be umounted or ep0 will
>
> s/will be umounted/is unmounted/
> s/ep0 will be/ep0 is/
>
>> + * be opened again. In the second case functionfs state will be
>> + * reseted, and it will be ready for descriptors and strings
>
> s/reseted/reset/
>
>> + * writing.
>> + *
>> + * This is useful only when functionfs is composed to gadget
>> + * with another function which can perform some critical
>> + * operations, and it's strongly desired to have this operations
>> + * completed, even after functionfs files closure.
>> + */
>> + FFS_ZOMBIE,
>> +
>> + /*
>> * All endpoints have been closed. This state is also set if
>> * we encounter an unrecoverable error. The only
>> * unrecoverable error is situation when after reading strings
>

Thanks,
Robert Baldyga
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2014-10-07 13:02    [W:0.046 / U:0.180 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site