Message-ID: <5547BDA3.8010206@hurleysoftware.com>
Date:	Mon, 04 May 2015 14:42:43 -0400
From:	Peter Hurley <peter@...leysoftware.com>
To:	Michael Matz <matz@...e.de>
CC:	NeilBrown <neilb@...e.de>,
	Nic Percival <Nic.Percival@...rofocus.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jslaby@...e.cz>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH bisected regression] input_available_p() sometimes says
 'no' when it should say 'yes'

On 05/04/2015 12:56 PM, Michael Matz wrote:
> Hi,
> 
> On Mon, 4 May 2015, Peter Hurley wrote:
> 
>> I think it would be a shame if ptrace() usage forced a whole class of 
>> i/o to be synchronous.
> 
> I think Neils patch doesn't do that, does it?

Yes. Non-blocking i/o would now have to wait until the input worker has
completed (at a time when the input worker may not even be running on a
cpu yet).

>  If it has an indication 
> that in fact some data must be there but isn't it pulls it, otherwise it's 
> the same as after your patch/current state?

At the point when the write() returns from the child process, the input
worker may not even have run yet. The patch will now force the reader to
wait until the input worker has started and run to completion.

Moreover, prior to 4.1, the read i/o loop is sloppy wrt kicking the
the input worker, so there is every likelihood that this patch would
force extra waits on a non-blocking reader even though no input was actually
being written.

> (Leaving the debugger question to Nic; but I guess it's similar interface 
> like gdb.  Once you come back into the debugger (breakpoint hit) it looks 
> once for input on the tubes of the debuggee and then enters a prompt; it 
> doesn't continue looking for input until you continue the debuggee (1).

That's exactly what gdb does. Below is simple test jig [2] where the child
outputs while at the gdb prompt.

  
> ptys would be used because it's Cobol, the programs contain data entry 
> masks presumably needing a real tty, not just pipes.  That usecase would 
> be broken now; the tty provided by the debugger doesn't reflect the real 
> screen that the debuggee actually generated before the breakpoint.  Note 
> how pipes in my test program _are_ synchronuous in this sense, just ptys 
> aren't.)

What's interesting about that expectation is that it would never work
on an actual tty.


> (1) And in a single threaded debugger (no matter if the debuggee is 
> multithreaded) it would be awkward to implement.  After read returns 0 
> you'd again call poll, which indicates data is there, and read again.  
> You repeat that until $SOMEWHEN.  But when is it enough?  Two loops, 10, 
> 1000?  To not sit in a tight loop you'd also add some nanosleeps, but that 
> would add unnecessary lags.

That's not what's happening.

poll() with a timeout of 0 returns immediately, even if no file descriptors
are ready for i/o. The poll() is returning 0 because there is no data to read,
so the loop is exiting.

If poll() returned non-zero and read() returned no data, that would
definitely be a bug.

> Basically, whenever poll indicates that read won't block then it should 
> also return some data, not 0, if at all reasonably implementable; i.e. 
> some progress should be guaranteed.

ttys have a further refinement of the behavior of read() and poll(), which
is controlled by the VMIN and VTIME values in the termios structure.

But note: the pty master does not allow its termios to be programmed --
a pty master read() is always non-blocking.

>  I realize that this isn't always the 
> case, but here it is.  In code, this loop:
> 
>   while (poll ([fd, POLLIN], 0) == 1)
>     // So, read won't block, yippie
>     if (read (fd, ...) == 0)
>       continue;
> 
> shouldn't become a tight loop, without the read making progress but the 
> kernel continuously stating "yep, there's data available", until some 
> random point in the future.

Yeah, that would be broken but, again, that's not what's happening here.

Regards,
Peter Hurley


--- >% ---
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <unistd.h>

#define BRKPT  asm("int $3")

void child()
{
	sleep(1);
	printf("Hello, world!");
}

int main()
{
	int child_id;

	setbuf(stdout, NULL);


	child_id = fork();
	switch (child_id) {
	case -1:
		printf("fork: %s (code: %d)\n", strerror(errno), errno);
		exit(EXIT_FAILURE);

	case 0:
		child();
		break;

	default: /* parent */
		BRKPT;
		break;
	}

	return 0;
}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists