git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCHv2 2/7] xread: poll on non blocking fds

From
Torsten Bögershausen <tboegi@web.de>
Date
Dec 18, 2015, 08:50 UTC
Message-ID
<5673C8E6.8010001@web.de>
In-Reply-To
<20151218031336.GA8467@sigill.intra.peff.net>
On 18.12.15 04:13, Jeff King wrote:
Show 31 quoted lines
> On Thu, Dec 17, 2015 at 09:42:01PM +0100, Torsten Bögershausen wrote:
> 
>>> Or do you mean to insert another continue in here?
>> I was thinking that we run into similar loop as before:
>> read() returns -1; errno = EAGAIN  /* No data to read */
>> poll() returns -1; errno = EAGAIN /* poll failed. If the fd was OK, the failure may be temporaly,
>>                                     as much as poll() can see.
>>                                     But most probably we run out ouf memory */
>>
>> So the code would look like this:
>>
>>    if (!poll(&pfd, 1, -1))
>>       return -1;
> 
> That changes the semantics of the function. The poll() is just a
> convenience to avoid spinning. If it fails, with Stefan's patch[1] the
> worst case is that we would spin on read() and poll(), instead of
> actually blocking in the poll().
> 
> But if we return on poll() failure, now the caller will see errors from
> poll() even though they don't know or care that we called poll() in the
> first place. Consider what would happen with your code if read got
> EAGAIN and then poll got EINTR. We would report an error, even though
> the whole point of xread() is to loop on these conditions.
> 
> -Peff
> 
> [1] Stefan's patch does have a bug. It forgets to "continue" after
>     calling poll, which means we will block with poll and _then_ exit
>     with -1, instead of restarting the loop.
> --
I love this group: Curing one bug with another doesn't work :-)
/* So the code v2 would look like this: */
    if (!poll(&pfd, 1, -1)) {
        if (errno == EINTR)
            continue;
         return -1; /* poll() failed, this is serious. */
    }
Previous: Jeff KingNext: Jeff King
Message 10 of 19 in “[PATCHv2 0/7] Rerolling sb/submodule-parallel-fetch for the time after 2.7”
  1. Stefan BellerDec 16, 2015
  2. 1/7 submodule.c: write "Fetching submodule <foo>" to stderrStefan Beller, Dec 16, 2015
  3. 2/7 xread: poll on non blocking fdsStefan Beller, Dec 16, 2015
  4. Torsten BögershausenDec 17, 2015
  5. Stefan BellerDec 17, 2015
  6. Torsten BögershausenDec 17, 2015
  7. Stefan BellerDec 17, 2015
  8. Jeff KingDec 18, 2015
  9. Jeff KingDec 18, 2015
  10. Torsten BögershausenDec 18, 2015
  11. Jeff KingDec 18, 2015
  12. Jeff KingDec 18, 2015
  13. 3/7 strbuf: add strbuf_read_once to read without blockingStefan Beller, Dec 16, 2015
  14. 4/7 sigchain: add command to pop all common signalsStefan Beller, Dec 16, 2015
  15. 5/7 run-command: add an asynchronous parallel child processorStefan Beller, Dec 16, 2015
  16. 6/7 fetch_populated_submodules: use new parallel job processingStefan Beller, Dec 16, 2015
  17. 7/7 submodules: allow parallel fetching, add tests and documentationStefan Beller, Dec 16, 2015
  18. Stefan BellerDec 16, 2015
  19. Junio C HamanoDec 16, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.