cygrunsrv + sshd + rsync = 20 times too slow -- throttled?

Corinna Vinschen corinna-cygwin@cygwin.com
Mon Sep 13 20:15:25 GMT 2021


On Sep 14 04:37, Takashi Yano wrote:
> On Mon, 13 Sep 2021 20:32:33 +0200
> Corinna Vinschen wrote:
> > Btw., while looking into the current pipe code, I wonder what select_sem
> > is doing in the pipe code at all so far.  It gets released, but it never
> > gets waited on?!?  Am I missing something?
> 
> The semaphore is waited in select.cc.

Ouch, I missed the get_select_sem call, sorry.

> > I don't understand why calling fork_fixup on query_hdl should depend
> > on the handle count.  If you duplicate a writer, you always have to
> > duplicate query_hdl as well to keep the count, no?  Inheritence is
> > handled by the O_CLOEXEC flag and fork_fixup will do the right thing.
> 
> I thought so, however, counting query_hdl cannot work as expected
> without this check. The number of query_hdl opend seems to exceed
> the number of writer.

If the write handle as well as the query handle are both opened,
duplicated and closed in the same manner, they should never have a
different count, unless the write side is inherited by a non-Cygwin
client.

> There seems to be the case that handle is already inherited here
> without fork_fixup. Any idea?

That should depend on the O_CLOEXEC setting, but identically for
all handles in the fhandler.

I pushed two more patches to topic/pipe in terms of inheritence,
maybe that gives a clue?

> > 
> > > +      if (!DuplicateHandle (GetCurrentProcess (), r,
> > > +			    GetCurrentProcess (), &fhs[1]->query_hdl,
> > > +			    GENERIC_READ, !(mode & O_CLOEXEC), 0))
> > 
> > This is a bug I introduced accidentally during testing.  This
> > GENERIC_READ is actually supposed to be a FILE_READ_ATTRIBUTES.
> > Sorry about that.
> 
> The PoC code uses PeekNamedPipe for query_hdl, so GENERIC_READ is
> necessary I think.

Oh, right, that's how it's documented.  Funny enough, the descriptions
of FSCTL_PIPE_PEEK does not mention any permissions at all.  I tried the
permissions and it's FILE_READ_DATA which is required.


Thanks,
Corinna


More information about the Cygwin-developers mailing list