Hi Peter, On 04/15, Peter Hurley wrote: > > > > If we just want to add a preemption point then perhaps we should use > > cond_resched/might_resched ? > > Yeah, it's just a preemption point that pre-dates cond_resched.
OK, so perhaps s/schedule/might_reched/ makes sense to make it more clear. > > So we can only get here if tty->ops->open returns -ERESTARTSYS without > > signal_pending(). This doesn't look good. But OK, lets forget this. > > tty_port_block_til_ready() returns -ERESTARTSYS if !ASYNC_HUP_NOTIFY and > tty was hungup or the h/w was reset Yes, yes, I saw this code, and I am not saying this is buggy. I meant this looks confusing. At least to me, until I grepped for ERESTARTSYS I thought that these code paths do something non-trivial with signal handling. IMHO, ERESTARTSYS should only be used if signal_pending() is true and we are going to return this error code to user-space. But tty_port_block_til_ready() returns ERESTARTSYS if !ASYNC_HUP_NOTIFY _or_ signal_pending(), that is why tty_open() has to check signal_pending() too. I think this deserves a cleanup, but this is minor and of course subjective, please forget. > > 2148 /* > > 2149 * Need to reset f_op in case a hangup happened. > > 2150 */ > > 2151 if (tty_hung_up_p(filp)) > > 2152 filp->f_op = &tty_fops; > > > > And this is called after tty_add_file() which makes this filp visible to > > __tty_hangup(), and without tty_lock(). > > tty_release() has removed the filp from the files list already (with the > tty lock held), Heh. Can't understand how did I miss that ;) Thanks Peter. Oleg.
