newfs locks entire machine for 20seconds

[Eric Anderson]

Steven Hartland wrote:
> 
> ----- Original Message ----- From: "Eric Anderson" <anderson at freebsd.org>
> 
>> I saw this once before, a long time back, and every time I went 
>> through a debugging session, it came to some kind of lock on the 
>> sysctl tree with regards to the geom info (maybe the XML kind of tree 
>> dump or something).  I don't recall all the details, but it was 
>> something like that.
> 
> Yep thats where I've traced it to its requesting: kern.geom.confxml
> 
> Which does:-
> static int
> sysctl_kern_geom_confxml(SYSCTL_HANDLER_ARGS)
> {
>    int error;
>    struct sbuf *sb;
> 
>    sb = sbuf_new(NULL, NULL, 0, SBUF_AUTOEXTEND);
>    g_waitfor_event(g_confxml, sb, M_WAITOK, NULL);
>    error = SYSCTL_OUT(req, sbuf_data(sb), sbuf_len(sb) + 1);
>    sbuf_delete(sb);
>    return error;
> }
> 
> What I dont understand is why this would lock the entire machine.
> 
> I've enabled LOCK_PROFILING and reran and I get the following which
> seems to indicate the culpret is: SYSCTL_LOCK()
> 
>  From what I can tell g_waitfor_event is returning EAGAIN for a large
> amount of time which means we get stuck in:-
> userland_sysctl
> ...
>    SYSCTL_LOCK();
> 
>    do {
>        req.oldidx = 0;
>        req.newidx = 0;
>        error = sysctl_root(0, name, namelen, &req);
>    } while (error == EAGAIN);
> 
>    if (req.lock == REQ_WIRED && req.validlen > 0)
>        vsunlock(req.oldptr, req.validlen);
> 
>    SYSCTL_UNLOCK();
> ...
> 
> The only reason I can see for returning EAGAIN is g_destroy_geom
> calling g_cancel_event


Wait - if it returns EAGAIN for a while, then look at that code above. 
It will hold the sysctl lock for some indefinite amount of time.  Maybe 
it should look like this instead:


    do {
        SYSCTL_LOCK();
        req.oldidx = 0;
        req.newidx = 0;
        error = sysctl_root(0, name, namelen, &req);
        SYSCTL_UNLOCK();
    } while (error == EAGAIN);

    if (req.lock == REQ_WIRED && req.validlen > 0)
        vsunlock(req.oldptr, req.validlen);


Can you try that?

Eric