On Mon, Nov 18, 2013 at 12:37:53PM +0100, Martin Pieuchot wrote: > Even if right now calling task_del() is enough, do you know if there's > an easy way to convert this code without putting the task storage in > the chunk of memory it manipulates? In other words having the "struct > task" outside of the softc?
Well, tasks are stored in softc everywhere I looked for examples. The task storage and the storage for the task's arguments needs to be known when task_set() is called. As I understand, task_set() is called once, at attach time. Then the task gets scheduled or cancelled with task_add()/task_del(), and when the task runs it gets its arguments from the storage given to task_set(). If we wanted to store the task and its arguments outside the softc, I guess we'd need to malloc() task memory at attach time and free it on detach(). I.e. it has the same lifetime as the softc. So I don't see the point of uncoupling it from the softc. Or we would need to call task_set() and task_add() every time we want to schedule the task. I don't think that's what the tasqk API is intending. Or we would have the task itself free its own storage (task and arguments), instead of freeing it when the softc is destroyed. But I don't think that's what the API is intending either. > I'm asking because if you can do it, this will make the task totally > independent and won't require any task_del(). The idea behind that > is that tomorrow we will try to have more kernel threads running in > parallel, and if your task is about to run or already running when > your interface is destroyed you might end up freeing the memory the > task is manipulating. I'm assuming the task won't be interrupted in a way that would make the ifp storage go away while it runs. Perhaps in a new world with more kernel threads that won't hold. But that would cause quite a lot of problems everywhere, not just sppp(4). When the ifp goes away, the task must be not be allowed to run if already scheduled. Because a workq task cannot be cancelled, the bug where the interface is deleted before the workq task gets to run is present in the current code and fixed by switching to taskq, with the task_del() call at interface detach time. How can we fix this bug without task_del()? Would you have the task itself cope with an ifp that has disappeared? > Since the current code is already using allocating some memory for the > arguments of the task, I'd argue that this is better than putting the > task storage on the same memory chunk that it manipulates. But since > this problem exists in other places in our tree, we might think of an > alternative solution/api/whatever. For now, I'd like to stick with the "task in softc" idiom I saw elsewhere. Generally, I think your concern is out of scope for my sppp(4) changes and should be discussed in a separate discussion thread because using a different idiom would affect many drivers.
