Re: kdevtmpfs oops since yesterdays vfs merge

From: Al Viro
Date: Mon Jul 25 2011 - 01:13:24 EST


On Mon, Jul 25, 2011 at 12:58:52AM -0400, Dave Jones wrote:
> On Mon, Jul 25, 2011 at 03:44:44AM +0100, Al Viro wrote:
>
> > > when it triggers the bug_on(), it's that second nodename that is garbage.
> >
> > Interesting... The next experiment would be to stick BUG_ON(!req.dev)
> > into devtmpfs_create_node() right after the assigment to that field.
>
> couldn't get that to trigger.

Interesting...

> > We couldn't be hit by the lack of barriers here, could we? Store to
> > req.dev happens before spin_unlock(&req_lock), so by the time when
> > that request is seen by loop in devtmpfsd() and passed to handle() it
> > should be seen - we have grabbed req_lock, found a pointer to req, dropped
> > req_lock and called handle(). Should've been enough...
> >
> > Might be interesting to print &req from devtmpfs_create_node(), both on
> > entry and on exit, and print req right before the call of handle()...
>
> Here's latest..
>
> https://s3.amazonaws.com/twitpic/photos/full/355219312.jpg?AWSAccessKeyId=AKIAJF3XCCKACR3QDMOA&Expires=1311570683&Signature=xr3tusulMiV2bIsxux9YNrawUDA%3D
>
> apologies for crappy picture, but it's legible at fullsize..
>
> interesting thing here is that the req that causes the oops, I couldn't
> find any call to create_handle for that address, so where devtmpfsd got it
> is a mystery. The address is curious too, in that it's way off from all the
> reqs created around that time.

Arrgh... OK, I see what's going on.

req->err = handle(req->name, req->mode, req->dev);
complete(&req->done);
req = req->next;
is letting the request creator to continue; if it leaves the scope, guess
what is left in *req? That's right, garbage... Including req->next.
All right, try this and let's see if it fixes the problem:

diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
index 3644dd4..49b6cba 100644
--- a/drivers/base/devtmpfs.c
+++ b/drivers/base/devtmpfs.c
@@ -406,9 +406,10 @@ static int devtmpfsd(void *p)
requests = NULL;
spin_unlock(&req_lock);
while (req) {
+ struct req *next = req->next;
req->err = handle(req->name, req->mode, req->dev);
complete(&req->done);
- req = req->next;
+ req = next;
}
spin_lock(&req_lock);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/