Messages in this thread | | | Date | Thu, 22 May 2008 12:41:42 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH] eCryptfs: Clean up kthread synchronization |
| |
On Thu, 22 May 2008 14:31:55 -0500 Michael Halcrow <mhalcrow@us.ibm.com> wrote:
> > > +void ecryptfs_destroy_kthread(void) > > > +{ > > > + struct ecryptfs_open_req tmp_req; > > > + struct ecryptfs_open_req *req; > > > + > > > + mutex_lock(&ecryptfs_kthread_ctl.mux); > > > + ecryptfs_kthread_ctl.flags |= ECRYPTFS_KTHREAD_ZOMBIE; > > > + list_for_each_entry(req, &ecryptfs_kthread_ctl.req_list, > > > + kthread_ctl_list) { > > > + mutex_lock(&req->mux); > > > + req->flags |= ECRYPTFS_REQ_ZOMBIE; > > > + wake_up_process(req->requesting_task); > > > + mutex_unlock(&req->mux); > > > + } > > > + memset(&tmp_req, 0, sizeof(tmp_req)); > > > + tmp_req.flags = ECRYPTFS_REQ_ZOMBIE; > > > + list_add_tail(&tmp_req.kthread_ctl_list, > > > + &ecryptfs_kthread_ctl.req_list); > > > + mutex_unlock(&ecryptfs_kthread_ctl.mux); > > > + wake_up(&ecryptfs_kthread_ctl.wait); > > > +} > > > > eh? We attach a local variable to a global list and then return? > > That won't last very long. > > Adding this dummy entry to the list is just my own way of getting the > kthread to wake up and shut down. This actually works, albeit a little > ugly. The list and its contents get dropped on the floor at this point > because (ecryptfs_kthread_ctl.flags & ECRYPTFS_KTHREAD_ZOMBIE). The > only consumer of this list (the kthread) checks for this flag > immediately after getting the mux, and if it is there, it just > exits. The only producer on this list (ecryptfs_privileged_open()) > checks for this flag immediately after getting the mux and bows out if > it is set. In other words, once this flag is set, the list and its > contents become untouchable by anything other than > ecryptfs_destroy_kthread().
Unconvinced.
As soon as ecryptfs_destroy_kthread() returns, tmp_req is destroyed. But it remains on ecryptfs_kthread_ctl.req_list.
> memset(&tmp_req, 0, sizeof(tmp_req)); > tmp_req.flags = ECRYPTFS_REQ_ZOMBIE; > list_add_tail(&tmp_req.kthread_ctl_list, > &ecryptfs_kthread_ctl.req_list); > mutex_unlock(&ecryptfs_kthread_ctl.mux); > wake_up(&ecryptfs_kthread_ctl.wait);
-> it's dead.
> }
| |