[lkml]   [2018]   [Jul]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH] net/p9/trans_fd.c: fix double list_del()
Tomas Bortoli wrote on Tue, Jul 24, 2018:
> >> @@ -228,6 +226,7 @@ static void p9_conn_cancel(struct p9_conn *m, int err)
> >> req->t_err = err;
> >> p9_client_cb(m->client, req, REQ_STATUS_ERROR);
> >> }
> >> + spin_unlock(&m->client->lock);
> >
> > If you want to expand the ranges of client->lock, the cancel_list will not
> > be necessary, you can optimize this code.
> >
> Unfortunately, not. Moving the spin_lock() before the for makes the
> crash appear again. This because the calls to list_move() in the for
> before delete all the elements from req->req_list, so the list is empty,
> another call to list_del() would trigger a double del.
> That's why we hold the lock to update the status of all those requests..
> otherwise we have again the race with p9_fd_cancel().

What (I think) he meant is that since you're holding the lock all the
way, you don't need to transfer all the items to a temporary list to
loop on it immediately afterwards, but you could call the client cb

I'm personally not a fan of this approach as that would duplicate the
code, even if the loop isn't big...

This code is only called at disconnect time so I think using the extra
list doesn't hurt anyone; but as usual do what you feel is better; I
don't mind much either way.

Dominique Martinet

 \ /
  Last update: 2018-07-24 12:20    [W:0.076 / U:2.224 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site