RE: [PATCH] move exit_task_work() before exit_fs().

From: Ma, Xindong
Date: Sun Dec 28 2014 - 20:35:21 EST


> On 12/28/2014 07:58 PM, Ma, Xindong wrote:
> >>
> >> On 12/26, Leon Ma wrote:
> >>>
> >>> We encountered following panic. The scenario is the process is
> >>> exiting and executing its task work. When closing dev node, the
> >>> driver triggers a firmware reload according to device status.
> >>> Because task->fs is
> >> set to NULL in exit_fs(), panic happens.
> >>
> >> I think this should be fixed somewhere else...
> > Yes, for this panic, I also think driver is not perfect and need a fix. But
> kernel should not add the limitation like this...
> >>
> >>> Task work is a common interface, we should not limite the resource
> >>> the
> >> user will utilize.
> >>
> >> Exactly. And note that with this patch exit_mm()..disassociate_ctty()
> >> paths can't use task works.
> > I don't get this. Currently disassociate_ctty() is also called after exit_mm()
> and exit_task_work(). My patch didn't change this.
>
> ??
>
> 742- if (group_dead)
> 743: disassociate_ctty(1);
> 744- exit_task_namespaces(tsk);
> 745- exit_task_work(tsk);
> 746- exit_thread();
Sorry, I was checking the old source code when applying this mail. I did a check on latest code, and did not find disassociate_ctty() was using task work. So why this matters?
>
> >> Not to mention that this patch moves exit_files() up, even before
> >> exit_mm(), without any explanation.
> > Moving exit_files() up is because exit_files() closes files and add tasks to
> task works.
> >>
> >> Add Al. May be we can move exit_fs() down after exit_task_work(), I
> >> dunno, but to me it would be better to change the driver.
> >>
> > I'm OK with this suggestion to fix this issue. I'm not sure whether in the
> future task work users will access other resources and expose other issues.
> >