> On March 18, 2011, 1:57 p.m., Gabe Black wrote: > > src/sim/syscall_emul.hh, line 503 > > <http://reviews.gem5.org/r/589/diff/1/?file=11013#file11013line503> > > > > 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. > > Vince Weaver wrote: > 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 ) > > > Gabe Black wrote: > 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. > > Steve Reinhardt wrote: > 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. > > Lisa Hsu wrote: > So, what's the consensus here? No checkin until ioctls are better > figured out? > > Gabe Black wrote: > 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.
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. - Marc ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/589/#review981 ----------------------------------------------------------- On March 17, 2011, 4:06 p.m., Lisa Hsu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/589/ > ----------------------------------------------------------- > > (Updated March 17, 2011, 4:06 p.m.) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Description > ------- > > X86 ioctl: Another patch from Vince Weaver > > > Diffs > ----- > > src/arch/x86/linux/syscalls.cc 2e269d6fb3e6 > src/sim/syscall_emul.hh 2e269d6fb3e6 > > Diff: http://reviews.gem5.org/r/589/diff/ > > > Testing > ------- > > > Thanks, > > Lisa Hsu > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
