1501 taskq_create_proc ... TQ_DYNAMIC puts tasks in p0

Review Request #24 — Created April 10, 2015 and submitted

gwr
illumos-gate
1501
general

See https://www.illumos.org/issues/1501
The SMB server code needs its taskq threads to be in the correct zone.

This was actually reviewed quite some time ago. See:
http://www.listbox.com/member/archive/182179/2011/09/sort/time_rev/page/1/?search_for=1501

The global-zone use case has had extensive use at customers.
The NGZ use case has had internal testing.

  • 0
  • 0
  • 1
  • 3
  • 4
Description From Last Updated
gwr
gwr
risto3
  1. 
      
  2. usr/src/uts/common/os/taskq.c (Diff revision 1)
     
     

    This returns '(NULL)' but task_qid_t is a uintptr_t... please 'return (0)' here in anticipation of 5218.

    1. I see your point, but I'd rather not combine the change you suggest with my fix for 1501.
      Please do that as a separate issue. ("Not this bug".)

  3. usr/src/uts/common/os/taskq.c (Diff revision 1)
     
     

    same as line 695

    1. Ditto ("Not this bug".)

  4. usr/src/uts/common/os/taskq.c (Diff revision 1)
     
     

    taskqid_t is a uintptr_t so please 'return (0)'
    instead of 'return (NULL)' here in anticipation of 5218

    1. Ditto ("Not this bug".)

  5. 
      
gwr
gwr
gwr
gwr
jeffpc
  1. 
      
  2. usr/src/uts/common/os/taskq.c (Diff revision 3)
     
     

    Overall, I like the simplicity of this change.

    I'll have to think about the implications of changing this condition from "any non p0 thread will have a lwp" to "any SDC thread will have a lwp". Obviously, the SDC case is correct. I just want to make sure that I fully understand the non-SDC case. Can there be a non-SDC thread that relied on having a lwp and it used to get one because it wasn't in p0? Just thinking out loud.

    1. There are not many consumers of taskq_create_proc in the gate, and I looked at them all. I believe this change will be fine.

    2. It certainly looks harmless. Since lofi and zfs are the two important consumers, I'd say if those two work properly with this change then LGTM.

  3. 
      
gdamore
  1. 
      
  2. usr/src/uts/common/os/taskq.c (Diff revision 3)
     
     

    This comment (not yours) is confusing due to double negative (without a non-p0). Can we change it while we're here to be a bit clearer, such as
    "Cannot have a DUTY_CYCLE with a p0 kernel process."

    1. Pushed to the smbsrv-zones3-rti branch.

  3. 
      
gwr
Review request changed

Status: Closed (submitted)

Loading...