Thanks for your response(s).
--- On Mon, 3/21/11, Dave Kilroy <kilroyd@xxxxxxxxxxxxxx> wrote:
> From: Dave Kilroy <kilroyd@xxxxxxxxxxxxxx>
> Subject: Re: [Orinoco-devel] [PATCH 2.6.38-rc8-wl RESEND] orinoco: Clear dangling pointer on hardware busy
> To: armadefuego@xxxxxxxxx
> Cc: orinoco-devel@xxxxxxxxxxxxxxxxxxxxx, linux-wireless@xxxxxxxxxxxxxxx
> Date: Monday, March 21, 2011, 6:40 PM
> On 21/03/2011 08:21, armadefuego@xxxxxxxxx
> wrote:
> > On hardware busy the scan request pointer should be
> cleared, as higher levels will release. This avoids a crash
> when that pointer is erroneously used later.
>
> I think you need to add line breaks for the git log
>
Sorry i am new to this.
logs on git don't have a column length restriction. Is there a "80", or
so, column restriction suggested for this?
> > Signed-off-by: Joseph J. Gunn<armadefuego@xxxxxxxxx>
> > ---
> > When the hardware is busy the error is propagated to
> higher levels on the stack. Those layers release the buffer.
> Therefore the copy of the pointer must be erased. Otherwise
> subsequent events checking this pointer ma crash.
> > ---
> > diff --git a/drivers/net/wireless/orinoco/cfg.c
> b/drivers/net/wireless/orinoco/cfg.c
> > index 09fae2f..2022815 100644
> > --- a/drivers/net/wireless/orinoco/cfg.c
> > +++ b/drivers/net/wireless/orinoco/cfg.c
> > @@ -151,8 +151,17 @@ static int orinoco_scan(struct
> wiphy *wiphy, struct net_device *dev,
> >
> return -EBUSY;
> >
> >
> priv->scan_request = request;
> > + DEBUG(3, "orinoco_scan():"
> > + " scan_request
> %p wiphy %p, dev %p\n",
> > +
> priv->scan_request,
> > +
> priv->scan_request->wiphy,
> > +
> priv->scan_request->dev
> > + );
> >
> > err =
> orinoco_hw_trigger_scan(priv, request->ssids);
> > + /* On EBUSY the hardware is busy.
> We aren't processing the request */
> > + if (err == -EBUSY)
>
> We should reset priv->scan_request on all errors, not
> just -EBUSY. Were
> you getting this in a particular situation? If so,
> highlighting it in
> the commit log is useful.
>
> I notice -EBUSY is returned when we can't get the
> orinoco_lock - are you
> having an issue with lock cotention on a particular
> device?
The calls for orinoco_lock seem to be well placed from a software point.
There are other cases where the low level hardware routines return EBUSY. When the hardware can not accept the command. This is the situation i can
reproduce.
It is possible, probable even, that all cases of error do not propagate
the scan request. I don't know the driver that well yet.
I chose to limit the effect of the patch to the case i could prove.
>
> > +
> priv->scan_request = NULL;
> >
> > return err;
> > }
> > diff --git a/drivers/net/wireless/orinoco/scan.c
> b/drivers/net/wireless/orinoco/scan.c
> > index e99ca1c..698e9ff 100644
> > --- a/drivers/net/wireless/orinoco/scan.c
> > +++ b/drivers/net/wireless/orinoco/scan.c
> > @@ -230,6 +230,12 @@ void
> orinoco_add_hostscan_results(struct orinoco_private *priv,
> >
> > scan_abort:
> > if
> (priv->scan_request) {
> > + DEBUG(3,
> "orinoco_add_hostscan_results():"
> > +
> " scan_request %p wiphy %p, dev %p\n",
> > +
> priv->scan_request,
> > +
> priv->scan_request->wiphy,
> > +
> priv->scan_request->dev
> > +
> );
>
> I'm not a big fan of scattering DEBUG statements about, but
> if we're
> going to, use __FUNCTION__ (or whatever the C99 incarnation
> is) rather
> than explicitly naming the functions.
>
As per the above the patch only fixes one possible scenario. I left the
messages there because they could be used to help prove/disprove that another set of events could produce a similar error.
It appears that some recent changes in user space code make it more likely
for the driver to hit this condition.
>
> Dave.
>
>
Thanks
Joe
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
