Date: Fri, 17 Aug 2012 21:42:45 -0600 From: PseudoCylon <moonlightakkiy@yahoo.ca> To: Bernhard Schmidt <bschmidt@techwires.net> Cc: freebsd-wireless@freebsd.org, Kim Culhan <w8hdkim@gmail.com> Subject: Re: (ANother) stall fixed, please update to HEAD Message-ID: <CAFZ_MY%2Bx39=xxB7e04BhQ0YYR3rq_HBuz86joRn=CLx13Vomng@mail.gmail.com> In-Reply-To: <CAAgh0_a6i50UBLJ9GqKY4a9Ttgd99Xpw3OiVJvqwKNjF73yOAg@mail.gmail.com> References: <CAFZ_MYL%2BPJp=fZGY3HUuL%2BLtMGh1QaAMrEZsoig5sKoV=ESs=w@mail.gmail.com> <CAAgh0_a6i50UBLJ9GqKY4a9Ttgd99Xpw3OiVJvqwKNjF73yOAg@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Aug 17, 2012 at 2:53 AM, Bernhard Schmidt <bschmidt@techwires.net> wrote: > On Thu, Aug 9, 2012 at 11:32 AM, PseudoCylon <moonlightakkiy@yahoo.ca> wrote: >>> ------------------------------ >>> >>> Message: 5 >>> Date: Tue, 7 Aug 2012 12:34:52 -0400 >>> From: Kim Culhan <w8hdkim@gmail.com> >>> Subject: Re: (ANother) stall fixed, please update to HEAD >>> To: Adrian Chadd <adrian.chadd@gmail.com> >>> Cc: freebsd-wireless@freebsd.org >>> Message-ID: >>> <CAKZxVQVbKfW7WVuZjNFg5pGFu3Djzz=60FoSNTS83sz1zf72wQ@mail.gmail.com> >>> Content-Type: text/plain; charset=ISO-8859-1 >>> >>> >>>> Yup. Would you be able to work with PseudoCylon and test his >>>> ieee80211_iterate_nodes() patch? I'd like verification that it fixes >>>> it for you before I tidy it up and commit it to -HEAD. >>> >>> iter.patch and iter2.patch cannot be applied in that order >>> soo.. PseudoCylon could you please generate a diff >>> against -HEAD from your present local source? >>> >> >> No it won't. Only one of them need to apply. I guess I didn't explain well. >> >> iter.path only print outs debug message when the array overflowed >> (most unlikely it will). At this moment, this is what we need. >> >> iter2.patch does iter.patch + revert changes + abort iterating just >> for piece of mind. Probably this is unnecessary. The code need to be >> patched in the way the array won't overflow if it ever happens. (I >> leave it to committers what to commit.) >> >> The attached patch can be applied over iter.patch >> >> Sorry for the confusion. > > Sorry for jumping in that late, but I've a bit of a bellyache with > that change especially the one which got committed. > > You've turned a totally safe function call into one which can fail, > and malloc will fail eventually, therefore all callers need to either > handle the error case or need to clearly state that it is not > necessary to do something. I have the feeling that you've introduced > some cases where due to lost calls the whole system might be hanging > (not as hard as a deadlock, but hard enough to require manual > intervention) by not doing a state change or something. > > Basically, all you've tried to achieve here is an additional unlock to > avoid a deadlock when the called function itself needs to acquire the > lock, performance aside, I'd really prefer if we're simply adding the > unlock and go back to the "restart" approach. At least until someone > either fixes the callers or can come up with a safe enough solution. That's true. Pointers to nodes are copied because we didn't want to unlock while traversing the linked list. Until some one comes up better solution, our options are ether dead lock or malloc. Maybe, driver can allocate the space matches drivers capability on attach. Nicer place to fail. AK
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAFZ_MY%2Bx39=xxB7e04BhQ0YYR3rq_HBuz86joRn=CLx13Vomng>