Review Board 2.0.15


X86 ioctl: Another patch from Vince Weaver

Review Request #589 - Created March 17, 2011 and updated

Information
Lisa Hsu
gem5
Reviewers
Default
ali, gblack, nate, stever
X86 ioctl:  Another patch from Vince Weaver

   
Ship it!
Posted (March 17, 2011, 12:12 p.m.)



  
Posted (March 18, 2011, 6:57 a.m.)



  
src/sim/syscall_emul.hh (Diff revision 1)
 
 
Why is this change necessary? I'm not 100% sure why it was the way it was before, but I see no reason to change it either. Changing the fatal to a warn may be necessary to get some benchmark to run, but I'm talking specifically about the ones that would return -ENOTTY.
  1. It's been a while, but let's see if I can see what's going on.
    
    Before we had a short list of ioctls we return -ENOTTY for, any not on the list gave a fatal error.
    
    ENOTTY if I recall correctly is the typical way the kernel reports an ioctl not being implemented.  So by changing all ioctls to warn and then report ENOTTY we are saying that we don't support it, but the program can keep running assuming it handles an ENOTTY properly.
    
    For most of the SPEC benchmarks ioctl() calls aren't important for correctness, so this works.  
    
    We could go back to having a whitelist of known-to-be-OK-to-ignore ioctls(), but it might turn out to be a lengthy list, and also we probably should implement things like isatty() properly as I do think some benchmarks depend on it returning the right value
    (see http://www.cs.binghamton.edu/~msim/wiki/index.php/TIOCISATTY )
    
  2. isatty is actually pretty annoying, and it's important that it -not- always return the right answer. It needs to be consistent regardless of whether your piping output to a file or watching it on the console so runs are deterministic, and if it always thinks it's going to a tty it'll buffer properly for when it's actually going to a tty (and doesn't look like something's broken, basically) and just perform a little worse when it's not. As far as simulator performance it doesn't matter since I'm sure it's a long way from being the critical path, and it should be a reasonable situation as far as simulated performance.
    
    I think a whitelist is a good idea, and we don't have to have -all- the ok to ignore ones on there, just the ok to ignore ones that actually get called. That should be a lot more manageable list. That way you know if something out of the ordinary is happening (the simulator will bomb out) rather than something weird happening and the benchmark stumbling on to eventually die, potentially far from evidence of what happened. The later is a lot harder to debug.
  3. ENOTTY is just the way the kernel reports a TTY-specific ioctl being invoked on a non-TTY device.  Gabe is right that for repeatability you always want the simulator to always act like there's no tty for determinism.  I would prefer to see us figure out what other ioctls need to be added to the TTY-specific list than to make a blanket assumption that any ioctl that's called is TTY-specific.
  4. So, what's the consensus here?  No checkin until ioctls are better figured out?
  5. The change to syscalls.cc is fine since that's just hooking up the existing syscall functions and there isn't anything unusual or objectionable about how that's being done. The change to syscall_emul.hh is not correct as far as I can tell. If there are specific IOCTL constants that are being used, we'll need to find out what those are and then what to do with them.
  6. I ran all of the spec workloads. Unfortunately, I was not able to get all of them to run to completion because of large inputs or other failures. That said, I saw many complain about the same ioctl - TCGETS. Furthermore, I saw no other ioctls occur when running the workloads. 
    
    I have submitted a new patch (since I don't have permissions to update this one) to address this problem.