Re: [PATCH]: gnumach - simplify interrupt handling

[Justus Winter]

Samuel Thibault  writes:

> A simpler way for now would be to make deliver_intr and
> device_intr_enable count how many "disables" we have for each irq.
> Both device_intr_register and deliver_intr would increase it (and thus
> disable the irq), and userland would have to call device_intr_enable
> to decrease the counter so the irq can be re-enabled. Of course if
> some userland "forgets" to call device_intr_enable(), the irq will
> stay disabled and we're screwed with anything else sharing the irq
> until userland gets killed for instance.  But at least it seems like a
> workable way for sharing interrupts between different userland processes
> (and gnumach drivers if any, e.g. the current ahci).

I remember thinking about using a send-once right that when deallocated
enables the interrupt again.  This should be a) well optimized, and b)
robust in the face of misbehaving userland.


signature.asc
Description: PGP signature

Re: Reverting "libdiskfs: Shutdown pagers on startup_dosync."

[Justus Winter]

Samuel Thibault <samuel.thiba...@gnu.org> writes:

> Justus Winter, on mer. 17 janv. 2018 11:36:00 +0100, wrote:
>> I'm still convinced that shutting down the servers is the right thing to
>> do.  The alternative is that the filesystem goes away with an unclean
>> disk image.
>
> It's not unclean since after inhibiting more RPCs we resync everything
> and wait for that to finish.  But shutting down properly would be
> better, yes.

But at some point we make the image read-only, and at that point there
are still other servers running.  If other servers are running, they use
some pagers.  Iirc if a library is in use (i.e. a pager is in use) at
system shutdown, we get those 'deleted inode has X links(?), should be
zero' issues.  And these do pile up.

Justus

Re: Reverting "libdiskfs: Shutdown pagers on startup_dosync."

[Justus Winter]

Samuel Thibault  writes:

> Hello Justus,
>
> I had to revert your commit:

Yes, I saw that :/

> “
> commit f05b6bae5cbdf3c221eba05987dea7ab14e6524f
> Author: Samuel Thibault 
> Date:   Mon Jan 15 22:00:11 2018 +0100
>
> Revert "libdiskfs: Shutdown pagers on startup_dosync."
> 
> This reverts commit 89a49ec8b3a847f3b770cff6a4a80b98b94ad7bf.
> 
> There was a typo in diskfs_shutdown (FSYS_GOAWAY_FORCE || 
> FSYS_GOAWAY_RECURSE)

Damn you C compilers >,<

> which was making it FSYS_GOAWAY_NOWAIT, and thus not actually syncing if
> there happens to be clients (it was then returning EBUSY). Fixing this 
> into
> FSYS_GOAWAY_FORCE | FSYS_GOAWAY_RECURSE then makes it wait for clients 
> (and
> NOWAIT is not supported), and there are very often some left, thus not
> syncing either.
> ”
>
> I don't know if the pager actually supports the operation that we want?
> (just crashing clients)

Yes it does.  I had to fix /hurd/startup to cope with that (it was
supposed to wire its pages, but that was broken if memory serves).

I'm still convinced that shutting down the servers is the right thing to
do.  The alternative is that the filesystem goes away with an unclean
disk image.

Justus

Re: [PATCH] PCI Arbiter

[Justus Winter]

Joan Lledó  writes:

>>> +INTR_INTERFACE
>>
>> Leftover here.
>>
>
> I cannot compile the server if I remove the `INTR_INTERFACE', The
> mig-generated header doesn't mach my prototype. Is that what's left
> over?

Oh, I was wrong here.  INTR_INTERFACE indeed only says that the RPCs are
interruptible (libports manages that for you afaik).


Cheers,
Justus


signature.asc
Description: PGP signature

Re: [PATCH] PCI Arbiter

[Justus Winter]

Hi Joan :)

let me be the first to say: Awesome :)

Joan Lledó  writes:

> Attached are two patches. The first one is to be applied to the Hurd
> source tree and contains a new mig pci interface, the pci server and a
> new library which includes the mig client stubs.

So I'd say the libpciclient library is a bit superfluous, that could be
done in the Hurd-specific libpcibackend.

> The second one is to be applied to the debian libpciaccess repository[1] and 
> adds a new module for the Hurd. There's no patch for pciutils for now.

I didn't get that one.

> I've tested them and they seem to be working fine with netdde in my box.

Sweet.

> I'd like to have some comments, no need for a deep review, but I had to make 
> some design decisions and wouldn't like to keep working on a wrong approach.

Ok, I don't have access to my box right now, but I can do a quick review
of the Hurdish bits.

> --
> [1] https://anonscm.debian.org/cgit/pkg-xorg/lib/libpciaccess.git/
> ---
>  Makefile  |   2 +
>  hurd/hurd_types.defs  |  19 ++-
>  hurd/hurd_types.h |   1 +
>  hurd/paths.h  |   1 +
>  hurd/pci.defs |  50 +++
>  hurd/subsystems   |   1 +
>  libpciclient/Makefile |  29 
>  libpciclient/pciclient.c  | 103 +++
>  libpciclient/pciclient.h  |  42 ++
>  pci_arbiter/Makefile  |  40 ++

I'd suggest pci-arbiter.

>  pci_arbiter/config.h  |   5 +
>  pci_arbiter/main.c| 116 
>  pci_arbiter/mig-mutate.h  |  28 
>  pci_arbiter/pci-ops.c |  89 +
>  pci_arbiter/pci_access.c  |  51 +++
>  pci_arbiter/pci_access.h  |  78 +++
>  pci_arbiter/pci_arbiter.h |  46 +++
>  pci_arbiter/x86_pci.c | 331 
> ++
>  pci_arbiter/x86_pci.h |  42 ++
>  19 files changed, 1073 insertions(+), 1 deletion(-)
>  create mode 100644 hurd/pci.defs
>  create mode 100644 libpciclient/Makefile
>  create mode 100644 libpciclient/pciclient.c
>  create mode 100644 libpciclient/pciclient.h
>  create mode 100644 pci_arbiter/Makefile
>  create mode 100644 pci_arbiter/config.h
>  create mode 100644 pci_arbiter/main.c
>  create mode 100644 pci_arbiter/mig-mutate.h
>  create mode 100644 pci_arbiter/pci-ops.c
>  create mode 100644 pci_arbiter/pci_access.c
>  create mode 100644 pci_arbiter/pci_access.h
>  create mode 100644 pci_arbiter/pci_arbiter.h
>  create mode 100644 pci_arbiter/x86_pci.c
>  create mode 100644 pci_arbiter/x86_pci.h
>
> diff --git a/Makefile b/Makefile
> index 119f130b..d3d93b87 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -31,6 +31,7 @@ lib-subdirs = libshouldbeinlibc libihash libiohelp libports 
> libthreads \
> libnetfs libpipe libstore libhurdbugaddr libftpconn libcons \
> libhurd-slab \
> libbpf \
> +   libpciclient
>  
>  # Hurd programs
>  prog-subdirs = auth proc exec term \
> @@ -45,6 +46,7 @@ prog-subdirs = auth proc exec term \
>  init \
>  devnode \
>  eth-multiplexer \
> +pci_arbiter
>  
>  ifeq ($(HAVE_SUN_RPC),yes)
>  prog-subdirs += nfs nfsd
> diff --git a/hurd/hurd_types.defs b/hurd/hurd_types.defs
> index 4d7013c8..0e9b990e 100644
> --- a/hurd/hurd_types.defs
> +++ b/hurd/hurd_types.defs
> @@ -1,5 +1,5 @@
>  /* MiG type declarations for Hurd interfaces -*- C -*-
> -   Copyright (C) 1993,94,95,96,98,2001,02 Free Software Foundation, Inc.
> +   Copyright (C) 1993,94,95,96,98,2001,02,17 Free Software Foundation, Inc.
>  
>  This file is part of the GNU Hurd.
>  
> @@ -296,6 +296,23 @@ destructor: INTERRUPT_DESTRUCTOR
>  #endif
>  ;
>  
> +/* PCI arbiter */
> +type pci_t = mach_port_copy_send_t
> +#ifdef PCI_INTRAN
> +intran: PCI_INTRAN
> +intranpayload: PCI_INTRAN_PAYLOAD
> +#else
> +#ifdef HURD_DEFAULT_PAYLOAD_TO_PORT
> +intranpayload: pci_t HURD_DEFAULT_PAYLOAD_TO_PORT
> +#endif
> +#endif
> +#ifdef PCI_OUTTRAN
> +outtran: PCI_OUTTRAN
> +#endif
> +#ifdef PCI_DESTRUCTOR
> +destructor: PCI_DESTRUCTOR
> +#endif
> +;
>  
>  type proccoll_t = mach_port_copy_send_t;
>  
> diff --git a/hurd/hurd_types.h b/hurd/hurd_types.h
> index 2960a294..8b1092a5 100644
> --- a/hurd/hurd_types.h
> +++ b/hurd/hurd_types.h
> @@ -50,6 +50,7 @@ typedef mach_port_t exec_startup_t;
>  typedef mach_port_t interrupt_t;
>  typedef mach_port_t proccoll_t;
>  typedef mach_port_t ctty_t;
> +typedef mach_port_t pci_t;
>  
>  #include/* Defines `error_t'.  */
>  
> diff --git a/hurd/paths.h b/hurd/paths.h
> index e1b00e90..1484b43e 100644
> --- a/hurd/paths.h
> +++ b/hurd/paths.h
> @@ -30,6 +30,7 @@ the Free Software Foundation, 675 Mass Ave, Cambridge, MA 
> 02139, USA.  */
>  #define _SERVERS_PROC_SERVERS "proc"
>  #define _SERVERS_PASSWORD_SERVERS "password"
>  #define _SERVERS_DEFPAGER_SERVERS "default-pager"
> +#define _SERVERS_PCI _SERVERS "pci"
>  
>  /* Directory 

Re: [PATCH] eth-multiplxer: Implement ds_device_close()

[Justus Winter]

Hi :)

"Joan Lledó"  writes:

> Hello,
>
> One difference between pfinet and lwip translators is that pfinet doesn't 
> delete interfaces (or I haven't found how to do it) while lwip does. For 
> instance, if one starts pfinet with:
>
> settrans -fga /servers/socket/2 /hurd/pfinet -i /dev/eth1 -a 192.168.123.178 
> -m 255.255.255.0 -g 192.168.123.1
>
> and then runs:
>
> fsysopts /servers/socket/2 --interface=/dev/eth2 --address=192.168.124.178 
> --netmask=255.255.255.0 --gateway=192.168.124.1
>
> the result is this:
>
> root@hurd:/home/jlledom# fsysopts /servers/socket/2
> /hurd/pfinet --interface=/dev/eth1 --address=192.168.123.178 
> --netmask=255.255.255.0 --address6=fc00:123::5054:ff:feb6:1ab3/64 
> --address6=fe80::5254:b6:1ab3/10 --address6=fe80::5054:ff:feb6:1ab3/10 
> --gateway6=fe80::5054:ff:fef6:d496 --interface=/dev/eth2 
> --address=192.168.124.178 --netmask=255.255.255.0 --gateway=192.168.124.1 
> --address6=fe80::5254:4b:ad11/10 --address6=fe80::5054:ff:fe4b:ad11/10
>
> fsysopts added a new interface. But doing the same with lwip leads to this:
>
> root@hurd:/home/jlledom# fsysopts /servers/socket/2
> /hurd/lwip --interface=/dev/eth2 --address=192.168.124.178 
> --netmask=255.255.255.0 --gateway=192.168.124.1 
> --address6=FE80::5054:FF:FE4B:AD11/64 
> --address6=FC00:124::5054:FF:FE4B:AD11/64
>
> The stack is reset and reconfigured, and only the given interface is added. I 
> implemented this by simply removing all interfaces anytime somebody calls 
> fsysopts, and adding the new ones. If some interface already exists, it's 
> deleted and created again.

Afaik using multiple interfaces is not something that many people have
done, so either behavior looks okay to me.

> This behaviour generated a problem when working with
> eth-multiplexer. Adding a new interface calls ds_device_open in
> eth-multiplexer and that returns a port name, whereas deleting the
> interface deallocates the port name in lwip, but nothing happens in
> eth-multiplexer since ds_device_close is unimplemented,

ds_device_close is not unimplemented, it merely does nothing.  But
indeed, nothing happens when the only sender deallocates its port
because of the way eth-multiplexer handles the device objects (it puts
them into a linked list with a hard reference, preventing the proper
deallocation of the port).  I'll try to fix that.

> when lwip
> tries to create the same interface again, eth-multiplexer returns the
> same port name,

That should be fine though.

> and trying to use it leads to a MIG_SERVER_DIED
> error.

Please confirm that eth-multiplexer does not die.

MIG_SERVER_DIED is generated by the mig-generated client stubs when the
replies message id does not matche the expected id, but
MACH_NOTIFY_SEND_ONCE.  This message is generated by the kernel if a
send-once right is destroyed.  This usually happens when the server
dies, but could in theory happen when the server explicitly destroys the
reply port, but tbh I couldn't see why or where that should happen.


Cheers,
Justus

Re: [PATCH hurd] fixup use lwip_{CFLAGS,LIBS}

[Justus Winter]

Justus Winter <jus...@gnupg.org> writes:

> ---
>  lwip/Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lwip/Makefile b/lwip/Makefile
> index e9289b5ae..8d4f45549 100644
> --- a/lwip/Makefile
> +++ b/lwip/Makefile
> @@ -28,7 +28,7 @@ OBJS= $(patsubst %.S,%.o,$(patsubst 
> %.c,%.o,\
>   $(SRCS) $(IFSRCS) $(MIGSRCS)))
>  
>  HURDLIBS= trivfs fshelp ports ihash shouldbeinlibc iohelp
> -LDLIBS = -lpthread -llwip
> +LDLIBS = -lpthread -llwip $(liblwip_LIBS)

Eww, the -llwip is still too much >,<


signature.asc
Description: PGP signature

[PATCH hurd] fixup use lwip_{CFLAGS,LIBS}

[Justus Winter]

---
 lwip/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lwip/Makefile b/lwip/Makefile
index e9289b5ae..8d4f45549 100644
--- a/lwip/Makefile
+++ b/lwip/Makefile
@@ -28,7 +28,7 @@ OBJS  = $(patsubst %.S,%.o,$(patsubst %.c,%.o,\
$(SRCS) $(IFSRCS) $(MIGSRCS)))
 
 HURDLIBS= trivfs fshelp ports ihash shouldbeinlibc iohelp
-LDLIBS = -lpthread -llwip
+LDLIBS = -lpthread -llwip $(liblwip_LIBS)
 
 target = lwip
 
@@ -37,7 +37,7 @@ include ../Makeconf
 vpath %.c $(PORTDIR) \
$(PORTDIR)/netif
 
-CFLAGS += -I$(PORTDIR)/include -I$(includedir)/lwip
+CFLAGS += -I$(PORTDIR)/include $(liblwip_CFLAGS)
 
 CPPFLAGS += -imacros $(srcdir)/config.h
 MIGCOMSFLAGS += -prefix lwip_
-- 
2.14.1

Re: libpager

[Justus Winter]

"Brent W. Baccala"  writes:

>> Are you aware of the pager rework in [0]?  I'm considering to review and
>> merge these changes.  It may make sense to try to come up with a common
>> API.  What is the state of your library, is it a drop-in replacement?
>>
>> 0: https://git.sceen.net/hurd/hurd.git/log/?h=mplaneta/gsoc12/review_v1
>
>
> I was not aware of this link, exactly.  Richard Braun mentioned a GSoC 12
> project a week ago, but I thought that it was a gnumach modification.
> Glancing at this git repository, I see that it's all translator stuff.

Not merely, see e.g.:

https://git.sceen.net/hurd/hurd.git/commit/?h=mplaneta/gsoc12/review_v1=2ca8719606eeca99c425b3282017a6412b49213a

> I'll study it.  Is there corresponding gnumach work?

Yes:

https://git.sceen.net/hurd/gnumach.git/log/?h=mplaneta/gsoc12/review_v1


Justus


signature.asc
Description: PGP signature

Re: [GSoC 2017] Point-to-point

[Justus Winter]

Joan Lledó  writes:

> The last item in my initial TODO list was implementing the --peer
> option available in pfinet. This week I've been working on it and it's
> now done, so I can say the LwIP translator is now able to replace
> pfinet.

And indeed, I'm using lwip on my development box right now:

root@hurdbox ~ # fsysopts /servers/socket/2
/hurd/lwip --interface=/dev/eth0m/0 --address=192.168.122.246 
--netmask=255.255.255.0 --gateway=192.168.122.1 
--address6=FE80::5054:28FF:FE44:31B6/64

This is excellent work, thank you very much.  So what remains to be done
until we can merge this is to address the scalability issues.  I'll
paste the discussion from
http://richtlijn.be/~larstiq/hurd/hurd-2017-07-25
here so that we can discuss this further:

11:59:43< teythoon> i see two issues with the lwip codebase, and we need to 
talk about how we address them in a way compatible with the goals of the 
upstream project
12:01:11< teythoon> jlledom: issue 1/ are the static arrays for sockets etc
12:01:21< teythoon> we need to be able to dynamically allocate such objects
12:01:25< jlledom> OK
12:01:37< teythoon> but we need to introduce it so that it is acceptable 
upstream
12:01:47< teythoon> so it must not degrade the performance on embedded systems
12:02:10< teythoon> maybe we can introduce an abstraction with two backends
12:02:13< teythoon> of some kind
12:02:27< teythoon> macros or functions that can be inlined
12:02:47< teythoon> but it should not add any overhead
12:02:54< teythoon> not in code size, not in memory
12:03:23< jlledom> I'm gonna eat, bbl
12:03:40< teythoon> you seem to be working good with the lwip community, i'd 
suggest to raise the issue somewhere, maybe the list
12:03:42< teythoon> ok
12:03:45< teythoon> i'll keep writing
12:04:31< teythoon> point 2/ is performance
12:04:35< teythoon> or related to performance
12:04:50< teythoon> it is not that important though, but worth keeping an eye on
12:05:10< teythoon> i did some experiments on tcp throughput using iperf
12:06:04< teythoon> for me, pfinet was doing double the throughput lwip did
12:06:22< teythoon> now that may also be due to how i compiled lwip
12:06:34< teythoon> i haven't done better tests
12:06:37< teythoon> sorry
12:06:50< teythoon> but i am worried about the one global lock that lwip uses
12:07:08< teythoon> that on embedded systems is implemented by disabling 
interrupts
12:07:30< teythoon> which is very fast and on singleprocessor systems very cheap
12:08:18< teythoon> but on posix platforms (or in the code in your repo) lwip 
is replacing that with one badly implemented recursive lock on top of a pthread 
mutex
12:08:30< teythoon> and that is a very heavyweight primitive in comparison
12:09:16< teythoon> there, it is possible with some work to introduce a better 
abstraction
12:12:32< teythoon> currently, there are two macros, SYS_ARCH_PROTECT(lev) and 
SYS_ARCH_UNPROTECT(lev)
12:12:58< teythoon> now, lev is a local variable that stores processor flags
12:13:14< teythoon> this is how the disabling of interrupts is made recursive
12:14:05< teythoon> the old state is saved in lev when locking, the interrupts 
are disabled, and the saved state restored on unlocking
12:15:15< teythoon> i'd suggest to replace this with per-resource locks on 
posix systems
12:15:48< teythoon> for that, a per-resource variable must be defined (in 
addition to the per-critical section local variables)
12:16:13< teythoon> then, both must be passed to SYS_ARCH_PROTECT, e.g. 
SYS_ARCH_PROTECT(lock,lev)
12:16:34< teythoon> now, on embedded system we can leverage 'lev' to implement 
the former scheme
12:16:47< teythoon> and on posix systems, we can just use 
pthread_mutex_lock(lock)
12:18:09< teythoon> using posix mutexes with attribute PTHREAD_MUTEX_RECURSIVE
12:19:53< teythoon> (big picture: i'm worried about contention on that lock)
12:20:24< teythoon> so we need to introduce a more fine-grained locking system
12:20:32< teythoon> again without imposing any overhead on small systems
12:20:47< teythoon> this should be done using macros by extending 
SYS_ARCH_PROTECT and friends

Now I realize that you said the performance issue may be due to
insufficient TCP tuning, but I'm still worried about contention.  But
problem 1/ is a killer, we cannot arbitrarily restrict the number of
sockets like that.


Cheers,
Justus


signature.asc
Description: PGP signature

Re: libpager

[Justus Winter]

"Brent W. Baccala"  writes:

> Hi -
>
> I've learned some interesting things about libpager that I'd like to share
> with the list.

Cool :)

> I've found two bugs in the existing code that were "fixed" by the new
> demuxer that sequentially services requests on each pager object.
>
> For example, the data return code sets a flag PAGINGOUT in the pagemap
> before it starts calling pager_write_page (which could be slow; writing to
> a hard drive, say).  Future data returns check the PAGINGOUT flag and wait
> on a condition variable if it's set.  The problem is that if multiple
> threads start waiting on that, pthreads doesn't guarantee what order they
> will run in when the conditional variable is signaled, so the data writes
> can get reordered.  If three data returns come in 1, 2, 3, (maybe because
> pager_sync is called three times), number 1 starts writing, but if it
> doesn't finish quick enough, 2 and 3 can get reordered.
>
> Except that they can't.  The new demux code queues the second and third
> writes.  They don't process until the first one is done.  The pager object
> is essentially locked until the pager_write_page() completes.
>
> I went so far as to write a test case to exercise the bug!  Just good
> coding practice - develop tests for your known bugs first.  Then I ran it,
> and it couldn't reproduce the bug!  Only after thinking about the code more
> did I understand why.
>
> I know the demuxer code was rewritten to avoid thread storms,
> [...]
>  Does anybody remember what characterized the thread
> storms?  What conditions triggered them?  What kind of pager operations
> were being done?

Yes.  I rewrote the demuxing part of libpager.  There were some issues
besides the thread storms (see below), I don't remember all the details,
but libpager was multithreaded but ordered the requests using the
sequence numbers, adding huge overhead (sleeping threads).

The thread storm issue was specific to ext2fs.  The issue here was that
there are two kinds of pagers, the disk pager and the file pager.  Now
one paging request to the file pager resulted in at least one request to
the disk pager, sometimes more (e.g. for superblock updates).  Combined
with threads sleeping b/c of the seqno ordering, this lead to a huge
number of threads being created, and if one tried to use a bounded
number of threads, this lead to deadlocks.  The key insight was to use
separate thread pools for each kind of pager, currently with one worker
thread each, but that is configurable.

> but it's obviously got some issues

What kind of issues?

> and could become a performance bottleneck at some point.  There's no
> good reason to block all access to page 100 while a disk operation
> completes on page 1.

Let's wait until it actually becomes a bottleneck...

> I'm not looking to re-write it right now, but I'm curious.

Are you aware of the pager rework in [0]?  I'm considering to review and
merge these changes.  It may make sense to try to come up with a common
API.  What is the state of your library, is it a drop-in replacement?

0: https://git.sceen.net/hurd/hurd.git/log/?h=mplaneta/gsoc12/review_v1

Cheers,
Justus


signature.asc
Description: PGP signature

Re: PulseAudio support

[Justus Winter]

David Michael  writes:

> Hi,
>
> I just pushed an update to my Hurd distro build scripts for Fedora 26
> at https://github.com/dm0-/gnuxc which also includes a translator for
> /dev/audio.  I've uploaded the translator source separately if anyone
> else is interested:
>
> https://github.com/dm0-/hurd-rump-audio
>
> The README has details, but the summary is that the translator
> pretends to be a Solaris audio device and acts as a rump kernel client
> converting I/O to the NetBSD audio device.  Since PulseAudio has a
> Solaris module, this allows audio playback from any programs that use
> PulseAudio without patching them or modifying their build processes.
> (That includes IceCat/Firefox, so e.g. web video sites are fully
> working with this.)

Oh wow, nice work :)

I should continue my work on GNU Machs userspace device driver interface
so that we can consider including those kind of drivers in Hurd proper.

Maybe we can include it in the Debian package in the mean time.


Justus


signature.asc
Description: PGP signature

Re: Filtering Ethernet multicasr frames

[Justus Winter]

Joan Lledó  writes:

> Hello,
>
> finally I've managed to fix the problem with IPv6 in my LwIP
> translator, and the problem was that the driver was discarding some
> Ethernet multicast frames.
>
> I've fixed it by enabling the IFF_ALLMULTI flag through
> device_set_status(), that is, now there is no filtering and all
> multicast frames reach the stack.

Good!

> It is interesting to read the link in [1], that divides the drivers
> into three classes depending on their capacity to filter multicast
> frames. Now we're in the first class, not filtering at all, but I'd
> like to know if the device interface provides a way to manage a list
> of allowed addresses, like in the third class. Is that possible?

Well, I don't know if the Linux drivers in our dde driver are able to do
that, but I don't think we should spent any more time on them.

It is possible to use bpf filters so that only interesting packets are
sent to the lwip stack, even though they are still processed by the
driver though.


Justus


signature.asc
Description: PGP signature

Re: [GSoC 2017] Support for fsysopts and multiple interfaces

[Justus Winter]

"Brent W. Baccala"  writes:

> Joan -
>
> Thank you for your work on this.  I haven't commented on it until now,
> partly because of some email problems, and partly because I haven't been
> working on Hurd for the last six months or so.
>
> On Mon, Jun 26, 2017 at 7:28 AM, Joan Lledó 
> wrote:
>
>> I've advanced in several fronts during the last two weeks, like the
>> initial configuration of the stack from the command line, by using
>> libargp, or reading and writing a new configuration in run time,
>> through fsysopts. The aim is to support exactly the same options
>> pfinet does, so we'll be able to replace pfinet by the LwIP translator
>> transparently for the rest of the system.
>>
>
> That's good!  As far as I know, using command line style options is the
> only way to change configuration in the current pfinet translator.

And ioctls.

> Leaving that for backwards compatibility is fine, but I think that we also
> want to have more of an API to interface with software.

Let's not get ahead of ourself.


Justus


signature.asc
Description: PGP signature

Re: Lightly tested patch to make fakeroot obey O_NOFOLLOW

[Justus Winter]

Justus Winter <jus...@gnupg.org> writes:

> I haven't had a chance to build packages with it yet, but it makes my
> minimal test case happy.

With this fix, dkpg builds without problems.  I will commit the fix.

Justus


signature.asc
Description: PGP signature

[PATCH hurd] trans/fakeroot: Obey O_NOFOLLOW.

[Justus Winter]

* trans/fakeroot.c (netfs_S_dir_lookup): Do not follow symlinks if the
client used O_NOFOLLOW.
---
 trans/fakeroot.c | 60 +++-
 1 file changed, 59 insertions(+), 1 deletion(-)

diff --git a/trans/fakeroot.c b/trans/fakeroot.c
index f3f5e43d6..df47b00fe 100644
--- a/trans/fakeroot.c
+++ b/trans/fakeroot.c
@@ -302,16 +302,74 @@ netfs_S_dir_lookup (struct protid *diruser,
 
   dnp = diruser->po->np;
 
+  /* See glibc's lookup-retry.c about O_NOFOLLOW.  */
+  if (flags & O_NOFOLLOW)
+flags |= O_NOTRANS;
+
   mach_port_t dir = netfs_node_netnode (dnp)->file;
  redo_lookup:
   err = dir_lookup (dir, filename,
-   flags & (O_NOLINK|O_RDWR|O_EXEC|O_CREAT|O_EXCL|O_NONBLOCK),
+   flags & (O_NOFOLLOW|O_NOTRANS|O_NOLINK
+|O_RDWR|O_EXEC|O_CREAT|O_EXCL|O_NONBLOCK),
real_from_fake_mode (mode), do_retry, retry_name, );
   if (dir != netfs_node_netnode (dnp)->file)
 mach_port_deallocate (mach_task_self (), dir);
   if (err)
 return err;
 
+  /* See glibc's lookup-retry.c about O_NOFOLLOW.  */
+  if (flags & O_NOFOLLOW
+  && (*do_retry == FS_RETRY_NORMAL && *retry_name == 0))
+{
+  /* In Linux, O_NOFOLLOW means to reject symlinks.  If we
+did an O_NOLINK lookup above and io_stat here to check
+for S_IFLNK, a translator like firmlink could easily
+spoof this check by not showing S_IFLNK, but in fact
+redirecting the lookup to some other name
+(i.e. opening the very same holes a symlink would).
+
+Instead we do an O_NOTRANS lookup above, and stat the
+underlying node: if it has a translator set, and its
+owner is not root (st_uid 0) then we reject it.
+Since the motivation for this feature is security, and
+that security presumes we trust the containing
+directory, this check approximates the security of
+refusing symlinks while accepting mount points.
+Note that we actually permit something Linux doesn't:
+we follow root-owned symlinks; if that is deemed
+undesireable, we can add a final check for that
+one exception to our general translator-based rule.  */
+  struct stat st;
+  err = io_stat (file, );
+  if (!err
+ && (st.st_mode & (S_IPTRANS|S_IATRANS)))
+   {
+ if (st.st_uid != 0)
+   err = ENOENT;
+ else if (st.st_mode & S_IPTRANS)
+   {
+ char buf[1024];   /* XXX */
+ char *trans = buf;
+ size_t translen = sizeof buf;
+ err = file_get_translator (file,
+, );
+ if (!err
+ && translen > sizeof _HURD_SYMLINK
+ && !memcmp (trans,
+ _HURD_SYMLINK, sizeof _HURD_SYMLINK))
+ err = ENOENT;
+
+ if (trans != buf)
+   vm_deallocate (mach_task_self (), (vm_address_t) trans, 
translen);
+   }
+   }
+  if (err)
+   {
+ mach_port_deallocate (mach_task_self (), file);
+ return err;
+   }
+}
+
   switch (*do_retry)
 {
 case FS_RETRY_REAUTH:
-- 
2.11.0

Lightly tested patch to make fakeroot obey O_NOFOLLOW

[Justus Winter]

I haven't had a chance to build packages with it yet, but it makes my
minimal test case happy.

Justus

Re: [PATCH hurd] trans: Fix parallel build race around MIG-generated headers

[Justus Winter]

David Michael  writes:

> This causes random.d to be generated which makes random.o depend on
> mach_debug_U.h, ensuring it exists before random.c is compiled.
>
> * trans/Makefile: Add random.c to SRCS when gcrypt is used.

Merged, thanks!

Justus


signature.asc
Description: PGP signature

Re: [GSoC 2017] Work done so far

[Justus Winter]

Hi :)

Joan Lledó  writes:

> It's useful to make a review of the work done in the last months and
> list some of the problems arisen during this period.

Thank you for your nice writeups, they are appreciated :)

> The process of writing the sockets and I/O operations have been quite
> straightforward. Most of actions performed by pfinet's operations are
> already implemented by LwIP, this includes managing the state of the
> sockets and concurrency, so many operations in the LwIP translator
> only check for RPC credentials, call the proper function in LwIP's
> sockets API and return errno. Connect[1] operation is a good example.
> As you may see, some operations like recv and connect itself have
> needed some additional changes to meet the requirements of Glibc, but
> in general, problems have come later.

That is how it should be.  Consider writing a function that maps LwIP
error codes to the appropriate Hurd ones if possible.

> One of the major issues I had was related with the get_openmodes[2]
> I/O operation. The implementation in pfinet[3] returns O_WRITE if our
> local socket hasn't sent the FIN message, and O_READ if the peer
> hasn't sent it. The operation also returns O_NONBLOCK if that flag is
> enabled on the local socket. In LwIP, only the O_NONBLOCK flag was
> supported by lwip_fcntl()[4], so I had to make some changes in that
> function in order to support the other two flags. I wrote a patch[5]
> that was rejected as it was based on some misconceptions and wasn't
> polished, but finally managed to fix it and was accepted to be part of
> the next LwIP release, 2.0.3.

Congratulations :)

> And that's all until today. From the list of tasks I included in my
> proposal[6], the following are still pending:
>
> - Add support for IPv6
> - Implement other interfaces' operations if needed.
> - Implement support for more than one Ethernet interface.
> - Add support for command-line parameters.
> - Add support for fsysopts.

I'd suggest doing the last two first.  That should be easy actually,
just lift the code from pfinet, and stub out what isn't straight forward
to do.  Once we understand roughly the same flags, we can use lwip as a
drop-in replacement for pfinet and see where things break.

> The prototype is working and is able to connect to the Internet. But
> when one tests it seriously many errors arise, so it's still far from
> being stable and there's still a lot to polish.

I tried it and it worked fine.  Thanks for your work so far :)


Cheers,
Justus


signature.asc
Description: PGP signature

Re: proc_task2proc prototype change

[Justus Winter]

David Michael <fedora@gmail.com> writes:

> On Mon, Jun 5, 2017 at 7:40 AM, Justus Winter <jus...@gnupg.org> wrote:
>> Samuel Thibault <samuel.thiba...@gnu.org> writes:
>>
>>> Hello,
>>>
>>> Justus Winter, on mar. 09 mai 2017 10:25:31 +0200, wrote:
>>>> It is possible to do so using either some preprocessor trickery,
>>>> or some other means of transformation (awk) on the protocol
>>>> specification, or by manually rewriting and committing a specialized rpc
>>>> definition.  Every one of these techniques is used in the Hurd, and I
>>>> despise them all ;)
>>>>
>>>> Maybe we can fix MIG so that we can get server-side-polymorphic right
>>>> parameters.
>>>
>>> Well, at least we need some solution for now, otherwise we get stuck :)
>>>
>>> AIUI, the least-ugly solution is to define a new type, which in the
>>> server is defined to mach_port_poly_t and in the client is defined to
>>> mach_port_t?
>>
>> As usual, the gods were way ahead of us, and introduced a way to specify
>> a different type for the client and server side, and there is even
>> already exactly the type that we need.  Quoting from the fine "Mach 3
>> Server Writers Guide":
>>
>>> The other major use of transmission type changes is to specify a
>>> parameter (most likely a port type) that is polymorphic on the
>>> sender’s side (user side for in parameters, server side for out
>>> parameters, both for inout parameters) but of a known type on the
>>> receiver’s side. That is, the sender will specify a port and a port
>>> type, but the result of sending this port will be to generate a known
>>> port type. The following built-in MIG types have such a specification:
>>>
>>> [1]type MACH_MSG_TYPE_PORT_RECEIVE =   -1 | 16;
>>> [2]type MACH_MSG_TYPE_PORT_SEND =  -1 | 17;
>>> [3]type MACH_MSG_TYPE_PORT_SEND_ONCE = -1 | 18;
>>
>> With the corresponding Mach type mach_port_send_t.  I committed a fix.
>
> Thanks, I can confirm that fixed bootstrapping compilers.  The reply
> functions still have mach_port_poly_t, though, and they are used by
> GDB.  Is that correct?

Oh wow, a server for the reply part of a protocol.  Odd.  Well, I
decided not to revert the change to the reply part so that it is
consistent with how the reply of a server for the full protocol works.

> It will require something like the following to get GDB to compile due
> to the changed generated definitions.
> [...]
>  ILL_RPC (S_proc_getmsgport_reply,
>   mach_port_t reply_port, kern_return_t return_code,
> - mach_port_t msgports)
> + mach_port_t msgports, mach_msg_type_name_t msgportsPoly)

Well, seeing that ILL_RPC is for unused procedures, it would be much
easier to use MIGs "new" way of creating default server stubs that
return a fixed value.  This can be done by #defining MIG_EOPNOTSUPP to
some value while compiling the MIG-generated server stub.  Being a
simpleroutine it doesn't really matter to what value, but EOPNOTSUPP
seems to be a better choice than ILL_RPC's 0.

Justus


signature.asc
Description: PGP signature

Re: Hurd's fakeroot seems to break dpkg 1.18.24 testsuite

[Justus Winter]

Hi :)

Guillem Jover  writes:

> I finally got the chance to take a look at the dpkg FTBFS on Hurd (after
> recovering my VM! :), and it appears as if it was a fakeroot problem?

It is a bug in our fakeroot, it breaks O_NOFOLLOW:

teythoon@hurdbox ~ % cat onofollow.c
#include 
#include 
#include 
#include 

int
main (int argv, char **argc)
{
  if (open ("symlink", O_RDONLY|O_NOFOLLOW) < 0)
perror ("open");
  return 0;
}
teythoon@hurdbox ~ % ln -s /etc/debian_version symlink
teythoon@hurdbox ~ % ./onofollow
open: Too many levels of symbolic links
teythoon@hurdbox ~ % fakeroot ./onofollow
teythoon@hurdbox ~ %


Justus


signature.asc
Description: PGP signature

Re: proc_task2proc prototype change

[Justus Winter]

Samuel Thibault <samuel.thiba...@gnu.org> writes:

> Hello,
>
> Justus Winter, on mar. 09 mai 2017 10:25:31 +0200, wrote:
>> It is possible to do so using either some preprocessor trickery,
>> or some other means of transformation (awk) on the protocol
>> specification, or by manually rewriting and committing a specialized rpc
>> definition.  Every one of these techniques is used in the Hurd, and I
>> despise them all ;)
>> 
>> Maybe we can fix MIG so that we can get server-side-polymorphic right
>> parameters.
>
> Well, at least we need some solution for now, otherwise we get stuck :)
>
> AIUI, the least-ugly solution is to define a new type, which in the
> server is defined to mach_port_poly_t and in the client is defined to
> mach_port_t?

As usual, the gods were way ahead of us, and introduced a way to specify
a different type for the client and server side, and there is even
already exactly the type that we need.  Quoting from the fine "Mach 3
Server Writers Guide":

> The other major use of transmission type changes is to specify a
> parameter (most likely a port type) that is polymorphic on the
> sender’s side (user side for in parameters, server side for out
> parameters, both for inout parameters) but of a known type on the
> receiver’s side. That is, the sender will specify a port and a port
> type, but the result of sending this port will be to generate a known
> port type. The following built-in MIG types have such a specification:
>
> [1]type MACH_MSG_TYPE_PORT_RECEIVE =   -1 | 16;
> [2]type MACH_MSG_TYPE_PORT_SEND =  -1 | 17;
> [3]type MACH_MSG_TYPE_PORT_SEND_ONCE = -1 | 18;

With the corresponding Mach type mach_port_send_t.  I committed a fix.

Justus


signature.asc
Description: PGP signature

Re: proc_task2proc prototype change

[Justus Winter]

Samuel Thibault <samuel.thiba...@gnu.org> writes:

> Hello,
>
> Justus Winter, on mar. 09 mai 2017 10:25:31 +0200, wrote:
>> It is possible to do so using either some preprocessor trickery,
>> or some other means of transformation (awk) on the protocol
>> specification, or by manually rewriting and committing a specialized rpc
>> definition.  Every one of these techniques is used in the Hurd, and I
>> despise them all ;)
>> 
>> Maybe we can fix MIG so that we can get server-side-polymorphic right
>> parameters.
>
> Well, at least we need some solution for now, otherwise we get stuck :)
>
> AIUI, the least-ugly solution is to define a new type, which in the
> server is defined to mach_port_poly_t and in the client is defined to
> mach_port_t?

Sounds rather nice actually.

Justus



signature.asc
Description: PGP signature

Re: Revisited: Hurd on a cluster computer

[Justus Winter]

Samuel Thibault <samuel.thiba...@gnu.org> writes:

> Justus Winter, on dim. 21 mai 2017 15:39:07 +0200, wrote:
>> Samuel Thibault <samuel.thiba...@gnu.org> writes:
>> 
>> > Mark Morgan Lloyd, on sam. 20 mai 2017 12:18:01 +, wrote:
>> >> Finally, what is the Hurd portability situation? Way back I worked on a
>> >> microkernel in '386 protected mode that used segmentation heavily, am I
>> >> correct in assuming that that sort of thing is completely deprecated in 
>> >> the
>> >> interest of portability?
>> >
>> > Indeed.
>> 
>> Though there was some work, a reimplementation if memory serves, of the
>> netmsg servers.  The archive should have some infos on that.
>
> Well, that's unrelated to the question above, which was actually
> unrelated to the original thread, which you are answering now :)

Ah, segmentation... Indeed, I wasn't reading that carefully, I thought
is was related to the in-kernel forwarding of Mach messages across
networks...

Justus


signature.asc
Description: PGP signature

Re: Revisited: Hurd on a cluster computer

[Justus Winter]

Samuel Thibault  writes:

> Mark Morgan Lloyd, on sam. 20 mai 2017 12:18:01 +, wrote:
>> Finally, what is the Hurd portability situation? Way back I worked on a
>> microkernel in '386 protected mode that used segmentation heavily, am I
>> correct in assuming that that sort of thing is completely deprecated in the
>> interest of portability?
>
> Indeed.

Though there was some work, a reimplementation if memory serves, of the
netmsg servers.  The archive should have some infos on that.

Justus


signature.asc
Description: PGP signature

Re: proc_task2proc prototype change

[Justus Winter]

Samuel Thibault <samuel.thiba...@gnu.org> writes:

> Justus Winter, on dim. 07 mai 2017 17:42:24 +0200, wrote:
>> Can you paste the client code?
>
> kern_return_t __proc_task2proc
> (
> process_t process,
> mach_port_t task,
> mach_port_t *proc,
> mach_msg_type_name_t *procPoly

Interesting.  For a moment I thought that I missed that polymorphic can
also mean "not only a right, but maybe an integer", but I indeed used
the MIG type 'mach_port_poly_t', so it knows that it will be a right.  I
obviously wasn't aware that the client code will also get another
parameter.

This is really unfortunate, not only because I broke the libc build this
time (sorry), but because every use of the MIG type 'mach_port_t' aka
MACH_MSG_TYPE_COPY_SEND as an 'out' parameter severely limits what the
server code can do: It is not possible to move a send right when
returning from the rpc, making it impossible to interpose this rpc.  And
function interposing is at the heart of the Mach design.  Likewise for
'mach_port_make_send_t' and any other non-polymorphic right type.

To answer your previous question, how to do that only for the server
side: It is possible to do so using either some preprocessor trickery,
or some other means of transformation (awk) on the protocol
specification, or by manually rewriting and committing a specialized rpc
definition.  Every one of these techniques is used in the Hurd, and I
despise them all ;)

Maybe we can fix MIG so that we can get server-side-polymorphic right
parameters.  Because I see us doing what I've done for these three
procedures every time we want to interpose something, and interposing is
what Mach is about aiui.


Justus


signature.asc
Description: PGP signature

Re: proc_task2proc prototype change

[Justus Winter]

Samuel Thibault  writes:

> David Michael, on sam. 06 mai 2017 12:28:51 -0700, wrote:
>> The new generated functions all get an additional
>> "mach_msg_type_name_t *msgportPoly" parameter and none of the callers
>> were updated, so they all fail to compile with "too few arguments"
>> errors.
>
> Indeed, rebuilding glibc gives:
>
> hurdexec.c:72:29: error: too few arguments to function ‘__proc_task2proc’
>
> because client RPC stubs are built by glibc that went unnoticed.
>
> Do RPC-knowledgeable people know a way to have this change happen in the
> server only?  Perhaps just using a typedef which will not be the
> same in the server and the client?

Whoops.  That was me.  But tbh, since I only changed out parameters (and
obviously the *in* parameters of the async reply part), I don't see why
that should change the client stub (as I remarked in the commit
message).

What did I miss?  Can you paste the client code?  I still don't see why
the out parameter should possibly be polymorphic at the client side, and
cannot even imagine what the semantic could be...

Justus


signature.asc
Description: PGP signature

[bug #48438] Cannot replace /hurd/fifo with different passive translator

[Justus Winter]

Follow-up Comment #2, bug #48438 (project hurd):

That is not what the OP meant.  You are seeing this because you accessed the
node 'hop' and started the translator.  Therefore, you have to use some flag
to tell settrans how to deal with the active translator.

The OP correctly reported that the short-circuiting logic in libfshelp is
wrong, or at least somewhat naive.

___

Reply to this item at:

  

___
  Message sent via/by Savannah
  http://savannah.gnu.org/

[bug #48890] mach-defpager isn't properly marked important

[Justus Winter]

Update of bug #48890 (project hurd):

  Status:None => Works For Me   
 Open/Closed:Open => Closed 

___

Follow-up Comment #5:

Yes, this is correct.  This bug report is very dubious.  If this is a problem
in the Debian package, then it should be reported in the Debian bug tracker. 
I have never seen a problem with this, and I am the one who introduced the
whole notion of important processes that should be left alone by killall5.

___

Reply to this item at:

  

___
  Message sent via/by Savannah
  http://savannah.gnu.org/

Changing our thread model // resource accounting

[Justus Winter]

Hello :)

with the recent advancements in Subhurd(TM) technology, our lack of
accounting is becoming more and more apparent.  The challenge with
accounting is of course to attribute resources spent to the client
that actually asked some server for some work.

Also, with our current thread model and asynchronous IPC it is really
easy to DOS the server by sending requests, with the server having to
spawn a thread for each request.

One way of relating the servers work to the client is that the client
surrenders its scheduling context to the server, also known under the
way cooler names migrating threads(TM) or thread shuttles(TM).  Under
this model, the client is restricted to synchronous IPC, solving the
DOS as well.

Richard suggests another solution, thread binding.  This is my attempt
to imagine how this might work, with an eye on how to implement it
within our current IPC model and framework code.  Beat me gently with
a XOR-linked list...

Transparent thread binding within the Mach IPC
==

Thread_client GNU MachThread_server
  |  |  |
  |  |  task_require_bind   |
  |  |<-0---|
  |  |  |
  |   msg_0  |  thread_bind|msg_0   |
  |--1-->|--1'->| thread_create
  |  |  |--2->
  |  |  thread_bind |
  |  |<-3---|
  |  |  |
 /~//~//~/
  |  |  reply_0 |
  |<-|--4---|
  |  |  |
 /~//~//~/
  |   msg_n  |  |
  |--|--5-->|
  |  |  reply_n |
  |<-|--6---|
  |  |  |
 /~//~//~/
  |   thread_terminate   |  thread_unbind   |
  |--7-->|--8-->|
 |  |


0. The server signals that it wants to use thread binding by sending

   task_require_bind (mach_task_self (), port)

1. A client thread wants to send a message to the server.  Currently,
no server thread is bound to this client thread.  Mach detects that,
and modifies the message to indicate that a binding is requested.

2. If this is the last idle server thread, a new one is spawned.

3. The server thread acknowledges the binding somehow.  Maybe this
isn't even necessary.

4. The server thread answers.

5., 6. Client and server communicate as usual.

7. If the client thread terminates,

8. Mach unbinds the server thread.

[Maybe: Mach can opportunistically (8. without 7.) unbind threads to
free resources.]


This gives us a relation between client thread and server thread.
This is easily combined with some kind of per-task (thread?) resource
ledger, and bound threads using the ledger of the client task
(thread?).

It restricts the client to synchronous IPC (there is only one server
thread answering requests for one client thread), solving the DOS
issue.

It seems feasible to implement, requiring changes only to GNU Mach,
libports, rpctrace.

[*cough*, I thought that once before...]

It can be incrementally implemented on a per port/portset basis.  A
smooth upgrade path is possible (i.e. new servers on old kernel, old
servers on new kernel).


Wdyt? WdIm? Discuss!
Justus


signature.asc
Description: PGP signature

Making Subhurds great again!

[Justus Winter]

Hello :)

last year some commie bastard took away the privileges of our Subhurds!
Tearable!  And worse, the processes of our poor Subhurds were weird
looking as before, but no longer debuggable.  Sad.

But fear not, my friends, for I made Subhurds great again:

teythoon@hurdbox ~ % ps
  PID TT STAT TIME COMMAND
 1245 p0 S 0:00.42 boot -f eth0 /dev/sd1s1
teythoon@hurdbox ~ % pstree 1245
boot───init─┬─init───sh───startpar───sh───logsave───fsck───fsck.ext2
├─procfs─┬─magic
│└─mtab
├─startup─┬─auth
│ ├─ext2fs.static─┬─exec
│ │   ├─fifo
│ │   ├─null
│ │   ├─pflocal
│ │   ├─2*[storeio]
│ │   └─term
│ └─proc
├─2*[tmpfs]
└─tmpfs───fifo
teythoon@hurdbox ~ % gdb /hurd/auth --pid $(pgrep auth | tail -n 1)
[...]
(gdb) bt
#0  0x0108e9ac in mach_msg_trap ()
at 
/build/glibc-cU1xjk/glibc-2.24/build-tree/hurd-i386-libc/mach/mach_msg_trap.S:2
#1  0x0108f146 in __mach_msg (msg=0x2001cc0, option=2051, send_size=40,
rcv_size=8192, rcv_name=10, timeout=0, notify=0) at msg.c:110
#2  0x0108f754 in __mach_msg_server_timeout (demux=0x2003d6c, max_size=8192,
rcv_name=10, option=2048, timeout=0) at msgserver.c:150
#3  0x0103f6fd in thread_function (arg=0x1)
at ../../libports/manage-multithread.c:259
#4  0x0103f93f in ports_manage_port_operations_multithread (bucket=0x804eca8,
demuxer=0x80493f0 , thread_timeout=3, hook=0x0)
at ../../libports/manage-multithread.c:290
#5  0x080492ac in main (argc=, argv=)
at ../../auth/auth.c:543


SCNR.  Seriously, use Subhurds, they're great.  The best.  Use Subhurds.
Justus


signature.asc
Description: PGP signature

Re: [PATCH 2/2] Rewrite gsync so that it works with remote tasks v2

[Justus Winter]

Samuel Thibault  writes:

> Agustina Arzille, on lun. 06 mars 2017 15:24:16 -0300, wrote:
>> On 2017-03-04 19:44, Samuel Thibault wrote:
>> > (thus related with Brent's "multithread rpctrace to avoid deadlocks in
>> > the kernel" patch)
>> 
>> Could be. I'm a bit puzzled as to why rpctrace uses a single thread to trace
>> every RPC in a program.
>
> Probably for simplicity :)
>
> I guess you shouldn't try to workaround the issue, let's just fix
> rpctrace :)

Note that there is no real need for it to be multithreaded, because it
doesn't block anywhere, it just relays messages.  Also, it has to
serialize the messages it prints out anyway.  I'm not sure what Brent
was trying to fix though.

Justus


signature.asc
Description: PGP signature

Re: GSoC mentors

[Justus Winter]

Thomas Schwinge  writes:

> Who will be available this year as a mentor for GSoC?

Me.

Justus


signature.asc
Description: PGP signature

Re: drama

[Justus Winter]

Hi,

Richard Braun  writes:

> We're lucky enough to have almost no drama-related issue, thanks in no
> small part to the tiny number of contributors.
>
> But we have Svante Signell. His inability to decently work with others
> is known beyond the Hurd, so it should come as no surprise that I've
> always had a hard time putting up with him along the years.

I agree.  This has always been a problem, and it does not seem to
change or improve.  It is time to speak up.


Svante, it is really hard to interact with you, and it is certainly not
because of a lack of trying on our part.  It is puzzling to me, really.
Please try to find someone you trust, someone that is completely honest
with you and whose judgment you respect, and talk with her or him about
your interactions with the project.  Show this persons IRC logs and mail
conversations, but don't cherry pick.  Get a second opinion, a new
perspective.

Because the way you behave is having an detrimental effect.  It is a
drain on scarce resources.  It is certainly not helping.  Please get
help to improve the way you interact with other people, or just leave us
alone.


Failing that, I believe that we as a project need to discuss how to deal
with the problem.  I learned that there is a precedent of the Hurd
project kicking someone out, though I certainly consider that a last
resort (also, I have no idea how to implement such a measure).


Justus


signature.asc
Description: PGP signature

Re: [PATCH] adding /proc/pid/exe

[Justus Winter]

Samuel Thibault  writes:

> Hello,
>
> Samuel Thibault, on Mon 19 Dec 2016 14:38:25 +0100, wrote:
>> I'm notably wondering about the RPC:
>> should we be using string_t or data_t ?
>
> So, any opinion on this?

I recommend just using string_t for now.

Eventually, we need to salvage the string_t disaster (i.e. that string_t
is a fixed-size char array which arbitrarily limits values like
filenames and blows up message sizes).  The annoying thing is that MIG
does support variable-sized character arrays (iirc, the terminology
for the Mach types could be different), so the Hurd could have used it
in the first place.  We could a) simply change it and rebuild clients
and servers, or b) try to provide a smooth upgrade path.

Justus


signature.asc
Description: PGP signature

Re: Time for another round of releases

[Justus Winter]

Thomas Schwinge  writes:

>> but I'm actually glad we did have a chance to merge
>> some more stuff ;) So, anything else missing for Hurd 0.9?
>
> :-) Seems we're good to go; planning for tomorrow.

Cool!

> I'll then set the GNU Hurd 0.10 etc. release dates for 2017-06, so that
> we'll again have six months to get changes done.

Or 1.0... /me ducks

Cheers,
Justus


signature.asc
Description: PGP signature

Re: Time for another round of releases

[Justus Winter]

Thomas Schwinge  writes:

> Will it be OK to move the release date towards end of November?  (Yay,
> one more month for getting stuff finished for inclusion...)  ;'-\

Or two (mea culpa), but I'm actually glad we did have a chance to merge
some more stuff ;) So, anything else missing for Hurd 0.9?

Cheers,
Justus


signature.asc
Description: PGP signature

Re: gdb handling of Mach exceptions

[Justus Winter]

"Brent W. Baccala"  writes:

> On Wed, Nov 23, 2016 at 10:03 PM, Brent W. Baccala 
> wrote:
>
>>
>> Any comments?
>>
>
> Well, yes, actually.  :-)
>
> gdb's hurd target has a poorly documented command "set noninvasive".  I
> don't completely understand it, but...

gdb in noninvasive mode does not stop the target.  This is important if
you want to use it on core servers (like the root filesystem) that your
gdb process, your shell, etc.  uses.  Blocking such a server likely
causes a deadlock.

I don't know what else it does, or how it is implemented.

Justus


signature.asc
Description: PGP signature

[bug #17341] booting a sub-hurd as non-root user

[Justus Winter]

Update of bug #17341 (project hurd):

  Status:   Confirmed => Fixed  
 Open/Closed:Open => Closed 

___

Follow-up Comment #2:

Fixed in 8c0e65b6b2db9946153ed736e66d4e613875deb6.

___

Reply to this item at:

  

___
  Message sent via/by Savannah
  http://savannah.gnu.org/

Re: gsync/libihash hash function

[Justus Winter]

Hi,

"Brent W. Baccala"  writes:
> libihash has a similar issue.  It seems to use no hash function by default,

The main performance critical use of libihash was to map port names
(which are mostly consecutive small integers) to objects.  I spent some
time optimizing libihash for that use case, and using a hash function
other than computing the remainder using bit masks was considerably
slower.  Nowadays we use protected payloads to map ports to objects, and
libihash involved into a more general-purpose hash table implementation.

> but provides Murmur3 if you want it, which looks better than just shifting
> bits, but also slower.

This was added to hash non-integer keys.

> I've been looking at the CRC32 instruction, which was introduced ten years
> ago in SSE 4.2.  According to the Intel Software Developer's Manual, CRC32
> works like this:

Richard wrote a micro-benchmark, you can find it on darnassus.

> It seems a bit silly to agonize over performance when we've got so many
> other issues,

Not at all.  Performance is important, and requires doing many small
things right.


Cheers,
Justus


signature.asc
Description: PGP signature

Re: two more rpctrace patches

[Justus Winter]

Hi!

"Brent W. Baccala"  writes:
> I'm attaching two more patches to rpctrace that close bug 48863.

Cool.  Two nitpicks:  1/ Instead of attaching patches, why don't you use
git send-email.  That is easier for everyone.  2/ The summary line of
your patches is too long, try to keep it at ~60 chars or so, and we
require changelog-style descriptions of the changes.

Cheers,
Justus


signature.asc
Description: PGP signature

Re: minor bug in glibc

[Justus Winter]

"Brent W. Baccala"  writes:

> task337(pid1240)->mach_port_deallocate (pn{  0}) = 0xf ((os/kern) invalid
> name)

We talked about this briefly on the list, and I remember we agreed that
this should be a nop.  This needs to be documented and changed in GNU
Mach, of course.  Thoughts?


Justus


signature.asc
Description: PGP signature

Re: Mach "pipe" between parent and child

[Justus Winter]

"Brent W. Baccala"  writes:
> What is the best way to fork() a child and have a Mach receive right on the
> parent connected to a send right on the child?  Or vice versa?

There is no easy and obvious way that I know.  Basically, you need to
insert the send right into the other task, then communicate the name.

If you are looking to do that across fork/exec, take a look at the
startup server to see how to replace fork/exec with
task_create/file_exec.  If you do that, you can communicate the port
name using the argument vector.  We do that e.g. to give the bootstrap
filesystem the privileged kernel ports.

One other idea that comes to mind is to install the send right as one of
the special ports (e.g. the bootstrap port), and hope that it is copied
during fork(2).  You need to read the fork implementation to see what it
does.


Cheers,
Justus


signature.asc
Description: PGP signature

Re: rpctrace design

[Justus Winter]

Hey,

I don't have time to look into that in detail now, but

"Brent W. Baccala"  writes:
> I read on the website's hurd/debugging/rpctrace page that somebody (zhenga)
> had come with a new version of rpctrace.  Do we have a copy of it around
> somewhere?

We merged his rpctrace work.  His contributions made multi-task tracing
possible.


Justus


signature.asc
Description: PGP signature

Re: commit 341f43d: boot: Ignore EINTR.

[Justus Winter]

Hi :)

"Brent W. Baccala"  writes:
> What are the symptoms of this bug?

Start a Subhurd, stop the boot program using e.g. gdb, resume it, select
got interrupted, boot dies.

> I've been seeing sporadic behavior where my boot hangs right after "exec"
> prints in the "Hurd server bootstrap" line.  Is this patch related to that?

No.  boot used to be used to boot the Hurd, but the elders moved the
bootscript parser into the kernel, and nowadays we only use it to boot
Subhurds.


Cheers,
Justus


signature.asc
Description: PGP signature

[PATCH gnumach] i386: Use discontiguous page directories when using PAE.

[Justus Winter]

Previously, we used contiguous page directories four pages in length
when using PAE.  To prevent physical memory fragmentation, we need to
use virtual memory for objects spanning multiple pages.  Virtual
kernel memory, however, is a scarce commodity.

* i386/intel/pmap.h (lin2pdenum): Never include the page directory pointer 
table index.
(lin2pdenum_cont): New macro which does include said index.
(struct pmap): Remove the directory base pointer when using PAE.
* i386/intel/pmap.c (pmap_pde): Fix lookup.
(pmap_pte): Fix check for uninitialized pmap.
(pmap_bootstrap): Do not store the page directory base if PAE.
(pmap_init): Reduce size of page directories to one page, use
direct-mapped memory.
(pmap_create): Allocate four page directories per pmap.
(pmap_destroy): Adapt to the discontinuous directories.
(pmap_collect): Likewise.
* i386/i386/xen.h (hyp_mmu_update_la): Adapt code manipulating the
kernels page directory.
* i386/i386at/model_dep.c (i386at_init): Likewise.
---
 i386/i386/xen.h |   2 +-
 i386/i386at/model_dep.c |  16 +++---
 i386/intel/pmap.c   | 129 +---
 i386/intel/pmap.h   |  14 --
 4 files changed, 118 insertions(+), 43 deletions(-)

diff --git a/i386/i386/xen.h b/i386/i386/xen.h
index b434dda..debf278 100644
--- a/i386/i386/xen.h
+++ b/i386/i386/xen.h
@@ -169,7 +169,7 @@ MACH_INLINE int hyp_mmu_update_pte(pt_entry_t pte, 
pt_entry_t val)
 #define HYP_BATCH_MMU_UPDATES 256
 
 #define hyp_mmu_update_la(la, val) hyp_mmu_update_pte( \
-   (kernel_pmap->dirbase[lin2pdenum((vm_offset_t)(la))] & INTEL_PTE_PFN) \
+   (kernel_page_dir[lin2pdenum_cont((vm_offset_t)(la))] & INTEL_PTE_PFN) \
+ ptenum((vm_offset_t)(la)) * sizeof(pt_entry_t), val)
 #endif
 
diff --git a/i386/i386at/model_dep.c b/i386/i386at/model_dep.c
index 239f63f..aa6c2c1 100644
--- a/i386/i386at/model_dep.c
+++ b/i386/i386at/model_dep.c
@@ -430,14 +430,14 @@ i386at_init(void)
delta = (vm_offset_t)(-delta);
nb_direct = delta >> PDESHIFT;
for (i = 0; i < nb_direct; i++)
-   kernel_page_dir[lin2pdenum(INIT_VM_MIN_KERNEL_ADDRESS) + i] =
-   kernel_page_dir[lin2pdenum(LINEAR_MIN_KERNEL_ADDRESS) + 
i];
+   kernel_page_dir[lin2pdenum_cont(INIT_VM_MIN_KERNEL_ADDRESS) + 
i] =
+   
kernel_page_dir[lin2pdenum_cont(LINEAR_MIN_KERNEL_ADDRESS) + i];
 #endif
/* We need BIOS memory mapped at 0xc & co for Linux drivers */
 #ifdef LINUX_DEV
 #if VM_MIN_KERNEL_ADDRESS != 0
-   kernel_page_dir[lin2pdenum(LINEAR_MIN_KERNEL_ADDRESS - 
VM_MIN_KERNEL_ADDRESS)] =
-   kernel_page_dir[lin2pdenum(LINEAR_MIN_KERNEL_ADDRESS)];
+   kernel_page_dir[lin2pdenum_cont(LINEAR_MIN_KERNEL_ADDRESS - 
VM_MIN_KERNEL_ADDRESS)] =
+   kernel_page_dir[lin2pdenum_cont(LINEAR_MIN_KERNEL_ADDRESS)];
 #endif
 #endif
 
@@ -489,21 +489,21 @@ i386at_init(void)
for (i = 0 ; i < nb_direct; i++) {
 #ifdef MACH_XEN
 #ifdef MACH_PSEUDO_PHYS
-   if 
(!hyp_mmu_update_pte(kv_to_ma(_page_dir[lin2pdenum(VM_MIN_KERNEL_ADDRESS)
 + i]), 0))
+   if 
(!hyp_mmu_update_pte(kv_to_ma(_page_dir[lin2pdenum_cont(VM_MIN_KERNEL_ADDRESS)
 + i]), 0))
 #else  /* MACH_PSEUDO_PHYS */
if (hyp_do_update_va_mapping(VM_MIN_KERNEL_ADDRESS + i * 
INTEL_PGBYTES, 0, UVMF_INVLPG | UVMF_ALL))
 #endif /* MACH_PSEUDO_PHYS */
printf("couldn't unmap frame %d\n", i);
 #else  /* MACH_XEN */
-   kernel_page_dir[lin2pdenum(INIT_VM_MIN_KERNEL_ADDRESS) + i] = 0;
+   kernel_page_dir[lin2pdenum_cont(INIT_VM_MIN_KERNEL_ADDRESS) + 
i] = 0;
 #endif /* MACH_XEN */
}
 #endif
/* Keep BIOS memory mapped */
 #ifdef LINUX_DEV
 #if VM_MIN_KERNEL_ADDRESS != 0
-   kernel_page_dir[lin2pdenum(LINEAR_MIN_KERNEL_ADDRESS - 
VM_MIN_KERNEL_ADDRESS)] =
-   kernel_page_dir[lin2pdenum(LINEAR_MIN_KERNEL_ADDRESS)];
+   kernel_page_dir[lin2pdenum_cont(LINEAR_MIN_KERNEL_ADDRESS - 
VM_MIN_KERNEL_ADDRESS)] =
+   kernel_page_dir[lin2pdenum_cont(LINEAR_MIN_KERNEL_ADDRESS)];
 #endif
 #endif
 
diff --git a/i386/intel/pmap.c b/i386/intel/pmap.c
index b143dd7..ab983ed 100644
--- a/i386/intel/pmap.c
+++ b/i386/intel/pmap.c
@@ -421,9 +421,15 @@ static pmap_mapwindow_t mapwindows[PMAP_NMAPWINDOWS];
 static inline pt_entry_t *
 pmap_pde(const pmap_t pmap, vm_offset_t addr)
 {
+   pt_entry_t *page_dir;
if (pmap == kernel_pmap)
addr = kvtolin(addr);
-   return >dirbase[lin2pdenum(addr)];
+#if PAE
+   page_dir = (pt_entry_t *) ptetokv(pmap->pdpbase[lin2pdpnum(addr)]);
+#else
+   page_dir = pmap->dirbase;
+#endif
+   return _dir[lin2pdenum(addr)];
 }
 
 /*
@@ -439,8 +445,13 @@ pmap_pte(const pmap_t pmap, vm_offset_t addr)
pt_entry_t  *ptp;
pt_entry_t  pte;
 
+#if PAE
+   if (pmap->pdpbase == 0)
+   return(PT_ENTRY_NULL);
+#else
  

Re: Time for another round of releases

[Justus Winter]

Thomas Schwinge <tho...@codesourcery.com> writes:
> On Sun, 2 Oct 2016 18:54:05 +0200, Justus Winter <jus...@gnupg.org> wrote:
>> it is October, therefore, it is time for a new set of releases :)
>
> Will it be OK to move the release date towards end of November?

Sure.

> I still need to better document/automate what needs to be done (in
> particular for updating the web pages).

That would be helpful, yes.

> (Yay, one more month for getting stuff finished for inclusion...)
> ;'-\

:)


Cheers,
Justus


signature.asc
Description: PGP signature

[bug #48930] crash server crashes after --core-file-name is changed with fsysopts

[Justus Winter]

Update of bug #48930 (project hurd):

 Open/Closed:Open => Closed 


___

Reply to this item at:

  

___
  Message sent via/by Savannah
  http://savannah.gnu.org/

Time for another round of releases

[Justus Winter]

Hello,

it is October, therefore, it is time for a new set of releases :)

I'll be going over the changes and update the NEWS files as usual, but
feel free to beat me to that.  Also, if anyone has some pet patches or
fixes that would be nice to include, feel free to speak up or send
patches.

Personally, I'd love to include Shengyu Zhang's work on xattrs, so I'm
going to see whether I can weed out the remaining issues in time.  Also,
I have a cleanup patch for Mach.

I'm afraid there haven't been any commits for MIG.  Does anyone have a
patch for it?  If not, are we going to make an release anyway, to keep
the version numbers in sync with GNU Mach?


Cheers,
Justus


signature.asc
Description: PGP signature

panic: pmap_page_protect removing a wired page

[Justus Winter]

Hello,

we spoke briefly in #hurd about a problem with wired memory I
encountered during my work on the malleable syscall interface.  I
managed to create a more minimal test case.  The attached program
crashes stock Mach kernels as packaged by Debian.

Cheers,
Justus



signature.asc
Description: PGP signature
#define _GNU_SOURCE
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

/* Verbatim copy, s/VM_INHERIT_NONE/VM_INHERIT_SHARE/ */
error_t
maptime_map (int use_mach_dev, char *dev_name,
	 volatile struct mapped_time_value **mtime)
{
  error_t err;
  mach_port_t memobj;

  if (use_mach_dev)
{
  device_t device;  
  mach_port_t device_master;

  err = get_privileged_ports (0, _master);
  if (err)
	return err;

  err = device_open (device_master, 0, dev_name ?: "time", );
  mach_port_deallocate (mach_task_self (), device_master);
  if (err)
	return err;

  err = device_map (device, VM_PROT_READ, 0, sizeof *mtime, , 0);

  /* Deallocate the device port.  The mapping is independent of
	 this port.  */
  mach_port_deallocate (mach_task_self (), device);
}
  else
{
  mach_port_t wr_memobj;
  file_t node = file_name_lookup (dev_name ?: "/dev/time", O_RDONLY, 0);

  if (node == MACH_PORT_NULL)
	return errno;

  err = io_map (node, , _memobj);
  if (!err && wr_memobj != MACH_PORT_NULL)
	mach_port_deallocate (mach_task_self (), wr_memobj);

  mach_port_deallocate (mach_task_self (), node);
}

  if (! err)
{
  *mtime = 0;
  err =
	vm_map (mach_task_self (), (vm_address_t *)mtime, sizeof *mtime, 0, 1,
		memobj, 0, 0, VM_PROT_READ, VM_PROT_READ, VM_INHERIT_SHARE);
  mach_port_deallocate (mach_task_self (), memobj);
}

  return err;
}

int
main ()
{
  error_t err;
  volatile struct mapped_time_value *mtime;
  err = maptime_map (0, NULL, );
  assert_perror (err);
  fprintf (stderr, "%d\n", fork ());
  return 0;
}

root@debian:~# ./pmap-assertion
681
panic: pmap_page_protect removing a wired page
Debugger invoked: panic
Kernel Breakpoint trap, eip 0xc1020314
Stopped at  Debugger+0x13:  int $3
Debugger(c10dfbec,0,f5cb8e2c,0,f9a3fbe0)+0x13
panic(c10e2380,f48439a0,f5cb8e4c,c101b47f,399c3)+0x79
pmap_page_protect(399d3000,0,f42db0b8,1,f40dfab8)+0x217
vm_object_pmap_remove(f9a3fbe0,0,1000,f5cb8ee0)+0x46
vm_map_entry_delete(f9a43510,f4843108,f5cb8f00,f5cb8f50,c1052466)+0x105
vm_map_delete(f9a43510,0,c000,f8c0c000,f54c2a90)+0x100
vm_map_deallocate.part.5(f5f80248,c1146920,f5cb8f6c,c102a73e,f9a43510)+0x1e
vm_map_deallocate(f9a43510,f5f80248,803,f54c2a90,f54c2a90)+0x25
task_deallocate(f5f80248,f54c2a90,f5cb8f9c,c102b7e5)+0x4e
thread_deallocate(f54c2a90,1,803,c102bcdd,1)+0x197
reaper_thread_continue(f99d0ce8,f99d2c00,f5cb8ec0,f5cb8ef8,f5f80248)+0x33
> user space <

Re: [PATCH gnumach] RFC: Create a malleable syscall interface.

[Justus Winter]

"Brent W. Baccala" <cos...@freesoft.org> writes:

> On Thu, Sep 22, 2016 at 12:21 PM, Justus Winter <jus...@gnupg.org> wrote:
>
>>
>> % ~/build/machometer/machometer
>> N: 33554432 (1<<25), qlimit: 5
>> mach_print:   5s75us171.363354ns5835553.391 (1/s)
>>nullmsg:   6s33us188.648701ns5300858.136 (1/s)
>>   producer:  27s65us824.034214ns1213541.844 (1/s)
>>   consumer:  27s65us824.034214ns1213541.844 (1/s)
>>
>>
> Where can I find "machometer"?

http://darnassus.sceen.net/cgit/teythoon/machometer.git/

[I had a lot of stuff there, then something terrible happened to
darnassus, and I did not re-add everything yet.]

Justus


signature.asc
Description: PGP signature

[PATCH gnumach] RFC: Create a malleable syscall interface.

[Justus Winter]

From: Justus Winter <4win...@informatik.uni-hamburg.de>

Currently, GNU Mach uses trap gates on i386 which are known to be slow
compared to modern alternatives like SYSENTER/SYSEXIT from Intel or
SYSCALL/SYSRET from AMD.  To make use of these instructions, we need
to detect at runtime whether the CPU supports these instructions and
to communicate this to userspace.

While we could select the appropriate mechanism purely in userspace,
doing so in the kernel and then providing code to userspace to make
use of it has an additional advantage: We abstract the actual syscall
mechanism away, making it malleable without changing the userspace
interface.

This change implements the following interface.

1. There exists a platform-dependent way to map a device into the
userspace task.  On i386, the platform-dependent way is to use the
current trapgate-based mechanism to communicate with the kernel.

On the Hurd, we provide a storeio translator for unprivileged tasks.
This is exactly the same way we handle Mach's mapped time source:

 % settrans -c /dev/syscall /hurd/storeio syscall

2. This device can be mapped into processes and presents the following
content:

 - A table of offsets into the page describing how to find the
   functions.

 long[] = -- table of offsets into this chunk.
   {
 long (*syscall) (long number, ...),-- unix syscall function
 long (*syscall_trampoline) (...),  -- expects number in %eax
 NULL   -- terminator
   };

   - The first function is a traditional syscall function as described
 in unistd.h.

   - The second is a variant that expects the system call number in
 %eax instead.  This allows one to use small trampolines to set the
 system call number, avoiding the need to rearrange the arguments on
 the stack.

   - The list is terminated, so that we can add more functions in the
 future.

   - XXX: Might be nice to use NULL as 'not available', and ~0 as
 terminator.

 - The actual functions as position independent code.

Discussion.

At this point we need to agree on the kernel interface.  I have
patches for glibc, but they are far from pretty.  Some preliminary
results:

Trapgate:

% ~/build/machometer/machometer
N: 33554432 (1<<25), qlimit: 5
mach_print:   5s75us171.363354ns5835553.391 (1/s)
   nullmsg:   6s33us188.648701ns5300858.136 (1/s)
  producer:  27s65us824.034214ns1213541.844 (1/s)
  consumer:  27s65us824.034214ns1213541.844 (1/s)

(mach_print uses a hard-coded trapgate mechanism, nullmsg is calling
mach_msg without any operation, producer/consumer are two threads in
the same task that exchange small, simple messages using mach_msg.)

Sysenter:

% LD_LIBRARY_PATH=$(pwd) ~/build/machometer/machometer
N: 33554432 (1<<25), qlimit: 5
mach_print:   5s74us171.065331ns5845719.861 (1/s)
   nullmsg:   3s24us96.559525ns 10356306.173 (1/s)
  producer:  24s41us727.474689ns1374618.271 (1/s)
  consumer:  24s41us727.474689ns1374618.271 (1/s)

This uses the patched glibc.  The mach_print is unchanged because of
the hard-coded mechanism, nullmsg and producer/consumer both improve
by ~95ns on my system, suggesting that this is the benefit of fewer
checks when traversing rings.  This *suggests* a ~10% improvement of
raw IPC performance for simple messages, ymmv, synthetic benchmark,
yadayadayada.

* i386/Makefrag.am (libkernel_a_SOURCES): Add new files.
* i386/i386/gdt.c (gdt_init): Add new descriptors for
SYSENTER/SYSEXIT.
* i386/i386/gdt.h ({KERNEL_ENTER,USER_EXIT}_{CS,DS}): New descriptors.
* i386/i386/i386asm.sym: Make size of the i386_saved_state structure
available to assembler bits.
* i386/i386/locore.S (_return_from_kernel): Check for magic value and
branch to sysenter epilogue.
(user_trapgate_syscall{,_trampoline}): Userspace stubs for the trap
gate mechanism.
(user_sysenter_syscall{,_trampoline}): Likewise for SYSENTER/SYSEXIT.
(sysenter_entry): Entry point for SYSENTER/SYSEXIT.
* i386/i386/pcb.c (pcb_init, thread_setstatus): Use new descriptors.
* i386/i386/syscall.c: New file.
* i386/i386/syscall.h: Likewise.
* i386/i386/tss.h (struct i386_tss): Add small stack for sysenter
entry.  This is only used very briefly.
* i386/i386at/conf.c: Add new 'syscall' device.
* i386/i386at/model_dep.c (machine_init): Initialize syscall module.
---
 i386/Makefrag.am|   2 +
 i386/i386/gdt.c |  17 +++
 i386/i386/gdt.h |   7 +-
 i386/i386/i386asm.sym   |   1 +
 i386/i386/locore.S  | 287 
 i386/i386/pcb.c |  24 ++--
 i386/i386/syscall.c | 103 +
 i386/i386/syscall.h |   7 ++
 i386/i386/tss.h |   1 +
 i386/i386at/conf.c  |   8 ++
 i386/i386at/model_dep.c |   2 +
 11 files changed, 446 insertions(+), 13 deletions(-)
 create mode 100644 i386/i386/sysc

[bug #49056] sending mach_port_kernel_object to non-task object crashes mach

[Justus Winter]

Update of bug #49056 (project hurd):

 Open/Closed:Open => Closed 

___

Follow-up Comment #1:

Thanks for the report, fixed in 25d5ceed.

I must ask though, in the time it has taken you to file this report and write
a reproducer, you could have fixed this by your self like 25 times in a row. 
Why didn't you?

___

Reply to this item at:

  

___
  Message sent via/by Savannah
  http://savannah.gnu.org/

Re: problems with a subhurd

[Justus Winter]

Hey :)

"Brent W. Baccala"  writes:
> Our current setup is that PID 5 (ext2fs) runs first, then starts PID 2
> (startup), which starts PID 1 (init).  Weird.  The cleanest solution, of
> course, would be to have proc actually respect these parenting
> relationships, then SIGCHLD and waitpid() would work normally.

Yes, weird indeed.  /hurd/proc assigns pids as the tasks are discovered,
and I hacked it so that we can make sysvinit pid 1.  sysvinit (posix?)
expects that.

> 3. Booting the subhurd, then running "halt -f" from its shell crashes the
> parent Hurd.  Here's what the subhurd displays:

I recommend against shutting down subhurds.  Privileged subhurds
(privileged as in the do have the privileged host control port) can
power down the system.  Also, the subhurd can see tasks from the parent
hurd, and interact with them.

If you like, you can have a look at my unprivileged subhurd branch here:

http://darnassus.sceen.net/cgit/teythoon/hurd.git/log/?h=unprivileged-subhurds-2016-09-04

Cheers,
Justus


signature.asc
Description: PGP signature

Re: Fwd: Hurd shutdown problems

[Justus Winter]

Hello,

"Brent W. Baccala"  writes:

> Further progress trying to track this down:
>
> I don't have to shutdown the system to have problems.  "swapoff /dev/hd0s5"
> is enough to cause problems, once enough swap is in use.  After a failed
> swapoff, I have an extra 98 storeio processes running!

:(

So we are seeing different problems.  I sometimes see the shutdown hang
way before swapoff is called.

Nevertheless, I have finished my little utility that you can use to make
a shell survive the shutdown process:

http://darnassus.sceen.net/~teythoon/bless

> I don't have to swapoff to have "symptoms".  The kernel debugger normally
> shows symbolic names, i.e:
>
> Stopped  at  machine_idle+0xe:   leave
> machine_idle(0,81a2c630,3806f64,0,9b448b38)+0xe
> idle_thread_continue(9fcbdde0,81028b50,9c0c7fe4,0,9c3d5548)+0x2a
>
> Once I've got enough swap in use, though, it stops doing this.  Now I see:
>
> Stopped   at  0x81be: leave
> 0x81be(0,0,9fcc5990,0,9fb90b30)
> 0x810293fa(9fcbdde0,81028b50,99526fe4,0,9c3d5548)

Uh :( that is not good.  That sounds like a swap-related corruption in
the kernel.

> When I see a kernel page fault, it's always in strcmp()

strcmp is used in the elf symbol lookup code, so that might explain the
fault.

> I can't task_terminate the auth server, as this typically does nothing once
> I've started having symptoms, but I can kill the auth server from the
> command line (just "kill 7") and that triggers a reboot that leaves the
> disk in a clean state.

Well, once the symbol lookup mechanism is fried, you likely cannot
!task_terminate anything anymore, since this relies on that mechanism.

> I'm just learning Hurd.  Any ideas?

Keep at it, the Hurd is an interesting system to learn from.  But you
might want to start with a simpler problem.


Cheers,
Justus


signature.asc
Description: PGP signature

Re: [PATCH] [hurd] pflocal/socket.c: Support MSG_DONTWAIT in pflocal send/recv

[Justus Winter]

Richard Braun <rbr...@sceen.net> writes:

> On Mon, Aug 08, 2016 at 04:54:47PM +0200, Justus Winter wrote:
>> Richard Braun <rbr...@sceen.net> writes:
>> > Why not start the translator from the remapped environment too ?
>> 
>> No reason, but this has to be implemented.  I started working on a
>> library for writing such chrooting translators, then got side-tracked by
>> the complexity of the dir_lookup operations.  Currently, remap has a
>> very naive lookup function, fakeroot's is better, but still not
>> sufficient.  I made some patches towards unifying and refactoring the
>> logic used in libdiskfs and libnetfs, but these functions are still huge
>> :/
>
> No, i mean, here, in such a specific case, if the parent translator is
> itself running from the remap env, it should used the custom pflocal
> instance, right ?

No, that doesn't help, because binding a unix socket involves setting a
passive translator, and that is still started by the filesystem
"outside" the chrooted environment:

teythoon@hurdbox /tmp % touch 1
teythoon@hurdbox /tmp % remap /servers/socket/1 /tmp/1 -- /bin/bash
bash: cannot make pipe for command substitution: (ipc/mig) bad request message 
ID
teythoon@hurdbox:/tmp$ exit
/bin/settrans: fsys_goaway: (ipc/mig) server died

(eh, also it is tricky to set up, cannot use bash right away)

teythoon@hurdbox /tmp % remap /servers/socket/1 /tmp/1 -- /bin/sh
$ settrans -a 1 /hurd/pflocal
teythoon@hurdbox:/tmp$ python3
Python 3.5.2+ (default, Aug  5 2016, 08:07:14) 
[GCC 6.1.1 20160705] on gnu0
Type "help", "copyright", "credits" or "license" for more information.
>>> import socket
>>> s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
>>> s.bind('/tmp/test.sock')
Traceback (most recent call last):
  File "", line 1, in 
OSError: [Errno 1073741873] Cannot assign requested address
>>> 
teythoon@hurdbox:/tmp$ showtrans test.sock
/hurd/ifsock

I firmly believe that the way to proceed is to teach such chrooting
translators to detect that a node has a passive translator record, and
instead of letting the filesystem start it, it must start the translator
on its own.  Not only gives this much stronger isolation, it is also
necessary for correctness.

Justus


signature.asc
Description: PGP signature

Re: [PATCH] [hurd] pflocal/socket.c: Support MSG_DONTWAIT in pflocal send/recv

[Justus Winter]

Richard Braun <rbr...@sceen.net> writes:

> On Mon, Aug 08, 2016 at 12:55:24PM +0200, Justus Winter wrote:
>> Right, I can see how this is a problem.  The thing is, remap doesn't
>> quite do the job: 1/ it fails to remap relative paths, 2/ if one sets a
>> translator record on a node, and that translator is then started by the
>> filesystem, it is started "outside" of the remap environment.  I belive
>> 2/ is what happens here.
>
> Why not start the translator from the remapped environment too ?

No reason, but this has to be implemented.  I started working on a
library for writing such chrooting translators, then got side-tracked by
the complexity of the dir_lookup operations.  Currently, remap has a
very naive lookup function, fakeroot's is better, but still not
sufficient.  I made some patches towards unifying and refactoring the
logic used in libdiskfs and libnetfs, but these functions are still huge
:/

Justus


signature.asc
Description: PGP signature

Re: [PATCH] [hurd] pflocal/socket.c: Support MSG_DONTWAIT in pflocal send/recv

[Justus Winter]

Christian Seiler  writes:

 Use the remap translator instead, which is one of the things the Hurd
 design allows you to do easily.

 See /bin/remap to easily set one.
>>>
>>> remap doesn't work at all here, programs then complain
>>> that they can't assign requested address when doing any
>>> socket operation.
>> 
>> Seems to work fine here:
>> 
>> teythoon@hurdbox ~ % cd /tmp
>> teythoon@hurdbox /tmp % settrans -ac 1 /hurd/pflocal
>> teythoon@hurdbox /tmp % remap /servers/socket/1 /tmp/1 -- /bin/bash -c 'echo 
>> huhu world | wc'
>>   1   2  11
>
> For pipes yes, for named sockets (which is what open-isns
> uses): no.
>
> $ cd /tmp
> $ settrans -ac 1 /hurd/pflocal
> $ remap /servers/socket/1 /tmp/1 -- python3
> Python 3.5.2+ (default, Aug  5 2016, 08:07:14) 
> [GCC 6.1.1 20160705] on gnu0
> Type "help", "copyright", "credits" or "license" for more information.
 import socket
 s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
 s.bind('/tmp/test.sock')
> Traceback (most recent call last):
>   File "", line 1, in 
> OSError: [Errno 1073741873] Cannot assign requested address
>
> (Same also from C programs, Python is just easier to test.)
>
> The same python code works if you run it without remap.

Right, I can see how this is a problem.  The thing is, remap doesn't
quite do the job: 1/ it fails to remap relative paths, 2/ if one sets a
translator record on a node, and that translator is then started by the
filesystem, it is started "outside" of the remap environment.  I belive
2/ is what happens here.

fakeroot has the same problem.  For me, lack of robust lightweight
virtualization this is the most pressing shortcoming of the Hurd, and I
did some work to address this.  Aiui remap/fakeroot must prevent the
filesystem from starting the translator and do it themself to make the
translation more correct.

> Anyway, not terribly important to me, rebooting did work fine
> anyway, and I now have a working patch for open-isns that will
> make it work on Hurd once my other patch against pflocal's
> socket.c is merged.

Cool!

Cheers,
Justus


signature.asc
Description: PGP signature

Re: [PATCH] [hurd] pflocal/socket.c: Support MSG_DONTWAIT in pflocal send/recv

[Justus Winter]

Christian Seiler  writes:

> (The following is not really important, rebooting does
> work, so it's not a showstopper.)
>
> On 08/07/2016 09:13 PM, Richard Braun wrote:
>> On Sun, Aug 07, 2016 at 08:44:56PM +0300, Esa Peuha wrote:
 PS: Is there any way to sanely restart /hurd/pflocal without
 rebooting?
>>>
>>> Yes, the commands to do that are
>>>
>>> settrans -ck /servers/socket/1
>>> settrans -ck /servers/socket/1 /hurd/pflocal
>
> FYI: that's really weird: the translater appears to be
> replaced on my system (up to date Debian sid), but from
> the response of programs, the old one still appears to
> be used.

Yes, that's what the -k is for, it keeps the old translator running.
Also, without specifying -a, settrans only stores the translator record,
which does not change.  -c creates the node, which already exists.  All
in all this was just bad advice.

>> Use the remap translator instead, which is one of the things the Hurd
>> design allows you to do easily.
>> 
>> See /bin/remap to easily set one.
>
> remap doesn't work at all here, programs then complain
> that they can't assign requested address when doing any
> socket operation.

Seems to work fine here:

teythoon@hurdbox ~ % cd /tmp
teythoon@hurdbox /tmp % settrans -ac 1 /hurd/pflocal
teythoon@hurdbox /tmp % remap /servers/socket/1 /tmp/1 -- /bin/bash -c 'echo 
huhu world | wc'
  1   2  11


Cheers,
Justus


signature.asc
Description: PGP signature

Re: Hurd shutdown problems

[Justus Winter]

"Brent W. Baccala" <cos...@freesoft.org> writes:

> On Sat, Aug 6, 2016 at 7:59 AM, Justus Winter <jus...@gnupg.org> wrote:
>
>>
>> To prevent filesystem damage, try the following.  Break into the kernel
>> debugger, and kill the auth server using:
>>
>> !task_terminate($task5)
>>
>> Then continue using "c", and /hurd/startup should cleanly shutdown the
>> system.
>>
>>
> The problem seems to be caused by a failure to swapoff the swap space.
> Since I've started paying attention to the swap space usage, I've always
> been able to cleanly shutdown if no swap is in use.  Once, when a small
> amount of swap was in use (7 MB), I was able to shutdown cleanly.  After a
> decent sized compile, however, with 100 MB or so of swap in use, I always
> get this:
>
> Deactivating swap...swapoff: /dev/hd0s5: 177152k swap space
> swapoff: /dev/hd0s5: (os/kern) failure
> failed.
> Unmounting weak filesystems...umount: /etc/mtab: Warning: duplicate entry
> for device /dev/hd0s1 (/dev/cons)
> done.
> mount: cannot remount /: Device or resource busy
> Will now halt.
>
> Now everything stops.

Interesting.  There is a utility in the Hurd tree called 'vmallocate'
that can be used to allocate and dirty large amounts of memory to
trigger such issues.  Unfortunately it isn't shipped with Debian iirc.

> What happens if I now try Justus's advice?
>
> Stoppedat  0x81be:leave
> Kernel Page fault trap, eip 0x81029b4e
> Caught Page fault (14),code = 0, pc = 81029b4e

Well, your system seems to be in a bad shape when entering the debugger,
a kernel fault occurred.  You cannot reasonably expect anything at this
point.

But yes, it fails from time to time, usually when it fails I see the
kernel rebooting as soon as I call the task_terminate function.  I guess
it is because one can break into the debugger when the system is at an
inconsistent state by chance.

Cheers,
Justus


signature.asc
Description: PGP signature

Re: Hurd shutdown problems

[Justus Winter]

Please use a MUA that does not break threads like this.

Esa Peuha  writes:

> Have you tried using halt-hurd instead of shutdown? As far as I can
> remember, halt-hurd has never caused file system corruption for me,
> but I'm pretty sure shutdown did way back when I was still trying
> to use it.

That is correct.  halt-hurd is basically halt -f, which is safe on the
Hurd, but skips the sysvinit shutdown.  However, we need to figure out
why this hangs every now and then.

Justus


signature.asc
Description: PGP signature

Re: Hurd shutdown problems

[Justus Winter]

Hi :)

"Brent W. Baccala"  writes:

> Aloha!
>
> Well, I think that I've resolved my boot hangs by removing the
> -no-kvm-irqchip flag, now I'm trying to resolve the occasional hang when
> the system shuts down.  The filesystem always gets corrupted if the system
> can't cleanly halt, so it's a real problem.

I do see the occasional hang when shutting down Debian/Hurd, but not at
the points you described.

To prevent filesystem damage, try the following.  Break into the kernel
debugger, and kill the auth server using:

!task_terminate($task5)

Then continue using "c", and /hurd/startup should cleanly shutdown the
system.

> Any idea what's causing this?  Any advice on how to debug it further?

Ideas yes, but none that is readily available.  1/ improve our subhurd
support and reproduce the shutdown problems in a subhurd, then debug
From the other hurd system.  2/ prevent sysvinit from killing your shell
by marking it as important using proc_mark_important, then investigate
using this shell.  Not sure if that would by itself interfere with the
shutdown, but it would probably work.

Cheers,
Justus


signature.asc
Description: PGP signature

Re: [bug #48685] update-grub triggers filesystem server panics

[Justus Winter]

"Brent W. Baccala" <cos...@freesoft.org> writes:

> On Tue, Aug 2, 2016 at 9:52 PM, Justus Winter <invalid.nore...@gnu.org>
> wrote:
>
>>
>> The isofs crash is fixed in 21ee0e79.  Please update your Hurd packages.
>>
>> The fact that you are seeing these messages is a (quite nice actually)
>> side-effect of 234568d0.  I guess that if you were doing this on a Linux
>> machine, you would see these in the kernel ring buffer.
>>
>>
> OK, I updated my Hurd system using the Debian ports repository.  The
> package is hurd-0.8.git20160522, and it includes 21ee0e79.
>
> Sure enough, the crash is gone, but I still get the error messages:

Good.

> ext2fs: /dev/hd0s2: panic: main: device too small for superblock (1024
> bytes)
> /hurd/fatfs: Invalid number of bytes per sector
> /hurd/iso9660fs: Could not find valid superblock
>
> Are you suggesting that these are to be expected?

Yes.

> Is this a bug in update-grub?

No.

> These messages seem awfully severe for a standard system operation.

And as I demonstrated, the very same kind of messages are generated on a
Linux machine.  The filesystem servers open the Mach console and write
the messages there.  You just happen to work on a console where those
messages are relayed.  If you were to do update-grub from an xterm, you
wouldn't see these.

Cheers,
Justus


signature.asc
Description: PGP signature

[bug #48685] update-grub triggers filesystem server panics

[Justus Winter]

Update of bug #48685 (project hurd):

  Status:None => Fixed  
 Open/Closed:Open => Closed 

___

Follow-up Comment #1:

The isofs crash is fixed in 21ee0e79.  Please update your Hurd packages.

The fact that you are seeing these messages is a (quite nice actually)
side-effect of 234568d0.  I guess that if you were doing this on a Linux
machine, you would see these in the kernel ring buffer.  And indeed, I just
tried:

[  667.337992] VFS: Can't find a Minix filesystem V1 | V2 | V3 on device
dm-6.
[  667.339712] hfsplus: unable to find HFS+ superblock
[  667.341322] qnx4: no qnx4 filesystem (no root dir).
[  667.342771] ufs: You didn't specify the type of your ufs filesystem

mount -t ufs -o
ufstype=sun|sunx86|44bsd|ufs2|5xbsd|old|hp|nextstep|nextstep-cd|openstep ...

>>>WARNING<<< Wrong ufstype may corrupt your filesystem, default is
ufstype=old
[  667.342935] ufs: ufs_fill_super(): bad magic number
[  667.344509] hfs: can't find a HFS filesystem on dev dm-6


___

Reply to this item at:

  

___
  Message sent via/by Savannah
  http://savannah.gnu.org/

Re: [PATCH] i386: Omit pmap workaround on i486 or later.

[Justus Winter]

Hi :)

Masanori Ogino  writes:
> How about this patch?
> Did I share this with you in a wrong way, or is it simply useless?
>
> 2016-06-18 22:04 GMT+09:00 Masanori Ogino :
>> As described in XXX comments, the workaround for memory mapping is
>> implemented for 80386 and it is unnecessary on i486 or later.  Thus, it
>> is safe to omit that if the kernel is built for the recent (1989~)
>> processors.

Sweet, thanks for the patch.  I included it in my builds, and it seems
good.  However, I did not understand the change in detail, could you
motivate it a little?  Or if others agree with the change, I'll merge
it.

Cheers,
Justus

[task #2561] Restrict hurd_ihash_add function

[Justus Winter]

Update of task #2561 (project hurd):

 Should Start On: => Mon 04 Jul 2016 12:00:00
AM GMT
   Should be Finished on: => Mon 04 Jul 2016 12:00:00
AM GMT
  Status:None => Need Info  
Percent Complete: Any => 0% 
 Open/Closed:Open => Closed 

___

Follow-up Comment #1:

Please re-open with more information.

___

Reply to this item at:

  

___
  Message sent via/by Savannah
  http://savannah.gnu.org/

Re: RFC: Runtime checking of port handling

[Justus Winter]

Hi,

Quoting Samuel Thibault (2016-06-04 17:32:06)
> Justus Winter, on Sat 04 Jun 2016 15:45:15 +0200, wrote:
> > tl;dr: Compiler-assisted runtime checking of port handling in
> > variables with automatic storage duration.  Do we want to go there?
> 
> That's interesting, but the proposed way would need annotating the whole
> source code, I'm afraid we'll make mistakes there.

Are you afraid that it is too much work, or that the process is
error-prone?

On the former point, this method can be applied on a per-function, and
even per-variable basis, so it can be introduced incrementally.  On
the latter, it makes all port handling errors fatal, while providing
us with meaningful error messages and stack traces.

> Passing a port to a function that will deallocate it will probably
> need another macro BTW.

I have provided Mach_port_move for that.

> Ideally we'd have a static analysis tool which knows about the mig rules
> for port references. Perhaps LeakSanitizer could be taught that? (we
> probably want to port it to Hurd, anyway, and it seems very portable)

Yes, static analysis tools are nicer (but come with their own set of
limitations).  Note that the *Sanitizers are also runtime tools.  In
any case, I doubt that we can create a tool that works entirely
without modifying / annotating existing code.

Justus

[PATCH hurd 2/2] libshouldbeinlibc: add safe port handling macros

[Justus Winter]

* libshouldbeinlibc/Makefile (SRCS, installhdrs): Add new file.
* libshouldbeinlibc/machx.h: New file.
---
 libshouldbeinlibc/Makefile |  2 ++
 libshouldbeinlibc/machx.h  | 73 ++
 trans/crash.c  | 29 +-
 3 files changed, 89 insertions(+), 15 deletions(-)
 create mode 100644 libshouldbeinlibc/machx.h

diff --git a/libshouldbeinlibc/Makefile b/libshouldbeinlibc/Makefile
index 04c085b..76076f2 100644
--- a/libshouldbeinlibc/Makefile
+++ b/libshouldbeinlibc/Makefile
@@ -30,11 +30,13 @@ SRCS = termsize.c timefmt.c exec-reauth.c maptime-funcs.c \
ugids-verify-auth.c nullauth.c \
refcount.c \
assert-backtrace.c \
+   machx.h \
 
 installhdrs = idvec.h timefmt.h maptime.h \
  wire.h portinfo.h portxlate.h cacheq.h ugids.h nullauth.h \
  refcount.h \
  assert-backtrace.h \
+ machx.h \
 
 installhdrsubdir = .
 
diff --git a/libshouldbeinlibc/machx.h b/libshouldbeinlibc/machx.h
new file mode 100644
index 000..6e727de
--- /dev/null
+++ b/libshouldbeinlibc/machx.h
@@ -0,0 +1,73 @@
+/* Safe right handling routines.
+
+   Copyright (C) 2016 Free Software Foundation, Inc.
+
+   This file is part of the GNU Hurd.
+
+   The GNU Hurd is free software; you can redistribute it and/or
+   modify it under the terms of the GNU General Public License as
+   published by the Free Software Foundation; either version 2, or (at
+   your option) any later version.
+
+   The GNU Hurd is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with the GNU Hurd.  If not, see .  */
+
+#ifndef __MACHX__
+#define __MACHX__
+
+#include 
+#include 
+#include 
+
+#define Mach_port_deallocate(TASK, NAME)\
+  ({\
+if (MACH_PORT_VALID (NAME)) \
+  { \
+error_t _Xerr = mach_port_deallocate ((TASK), (NAME));  \
+if (_Xerr)  \
+  { \
+char _Xmessage[1024];   \
+snprintf (_Xmessage, sizeof _Xmessage,  \
+  "error deallocating "#NAME": %s", strerror (_Xerr)); \
+__print_fail_backtrace (_Xmessage, __FILE__, __LINE__,  \
+__PRETTY_FUNCTION__);   \
+  } \
+NAME = MACH_PORT_NULL;  \
+  } \
+  })
+
+#define Mach_port_move(NAME)\
+  ({\
+mach_port_t _Xport = (NAME);\
+NAME = MACH_PORT_NULL;  \
+_Xport; \
+  })
+
+#define Mach_port_check(NAME)   \
+  void _Mach_port_check_##NAME(char *_unused[] __attribute__ ((unused))) \
+  { \
+  if (MACH_PORT_VALID (NAME))   \
+__print_fail_backtrace (#NAME " leaked",\
+__FILE__, __LINE__, "Port leak detector");  \
+  } \
+  char _Mach_port_check_x_##NAME[0] \
+  __attribute__ ((unused, cleanup (_Mach_port_check_##NAME)))
+
+#define Mach_server_success(ERR)   ((ERR) == 0 || (ERR) == MIG_NO_REPLY)
+
+#define Mach_server_port_check(ERR, NAME)   \
+  void _Mach_port_check_##NAME(char *_unused[] __attribute__ ((unused))) \
+  { \
+if (Mach_server_success (ERR) && MACH_PORT_VALID (NAME))\
+  __print_fail_backtrace (#NAME " leaked",  \
+  __FILE__, __LINE__, "Port leak detector"); \
+  } \
+  char _Mach_port_check_x_##NAME[0] \
+  __attribute__ ((unused, cleanup (_Mach_port_check_##NAME)))
+
+#endif /* __MACHX__ */
diff --git a/trans/crash.c b/trans/crash.c
index 0fe9b24..d911da3 100644
--- a/trans/crash.c
+++ b/trans/crash.c
@@ -21,6 +21,7 @@
along with the GNU Hurd; see the 

[PATCH hurd 1/2] xxx print fail

[Justus Winter]

---
 libshouldbeinlibc/assert-backtrace.c | 8 
 libshouldbeinlibc/assert-backtrace.h | 6 ++
 2 files changed, 14 insertions(+)

diff --git a/libshouldbeinlibc/assert-backtrace.c 
b/libshouldbeinlibc/assert-backtrace.c
index ca23c8d..72a49e9 100644
--- a/libshouldbeinlibc/assert-backtrace.c
+++ b/libshouldbeinlibc/assert-backtrace.c
@@ -76,4 +76,12 @@ __assert_perror_fail_backtrace (int errnum,
 
 }
 
+void
+__print_fail_backtrace (const char *message, const char *file,
+unsigned int line, const char *function)
+{
+  __assert_fail_base_backtrace ("%s: %s:%u: %s: %s.\n",
+   message, file, line, function);
+}
+
 #endif /* ! defined NDEBUG */
diff --git a/libshouldbeinlibc/assert-backtrace.h 
b/libshouldbeinlibc/assert-backtrace.h
index c54b810..b36e5b2 100644
--- a/libshouldbeinlibc/assert-backtrace.h
+++ b/libshouldbeinlibc/assert-backtrace.h
@@ -42,6 +42,12 @@ void __assert_perror_fail_backtrace (int errnum,
 const char *function)
   __attribute__ ((noreturn, unused));
 
+/* Likewise, but prints the given MESSAGE.  */
+void
+__print_fail_backtrace (const char *message, const char *file,
+unsigned int line, const char *function)
+  __attribute__ ((noreturn, unused));
+
 #define assert_backtrace(expr) \
   ((expr)  \
? (void) 0  \
-- 
2.1.4

RFC: Runtime checking of port handling

[Justus Winter]

Hello :)

tl;dr: Compiler-assisted runtime checking of port handling in
variables with automatic storage duration.  Do we want to go there?

Long version:

I recently found a pretty bad resource leak in our crash server and
pondered how to find such issues proactively.  The main idea is to 1/
clear variables if the right is transferred, and to 2/ check if it is
indeed cleared when the variable goes out of scope.

1/ can be idiomatically achieved using variants of
mach_port_deallocate that do error checking and clear the variable
holding the port name.

2/ can be done in a robust way using the gcc variable attribute
'cleanup' that executes a function when the variable goes out of
scope.

The follow up patch demonstrates the use of this in the crash servers
'S_crash_dump_task' server function.  Using it I already found a bug,
my initial attempt of fixing the resource leak in crash was
incomplete, and in fact leaking 'ctty_id' when suspending the crashing
task.

I believe this approach offers strong guarantees: It produces no false
negatives (if used correctly...), with acceptable manual work applying
it to existing code, and minimal runtime overhead (which can be
reduced to zero for NDEBUG builds).  Printing meaningful error
messages with backtraces will help both fixing bugs and deploying this
mechanism.

Do we want to do this, or is this too clever for its own good?

Cheers,
Justus

Re: [PATCH] libhurdutil: New library containing utils to be used by Guix.

[Justus Winter]

Hi :)

Quoting manolis...@gmail.com (2016-05-25 17:05:57)
> From: Manolis Ragkousis 
> 
> * libhurdutil/hurdutil.h: New file.
> * libhurdutil/settrans.c: New file.
> * libhurdutil/Makefile: New file.
> * utils/Makefile (HURDLIBS, settrans): Use the new library.
> * utils/settrans.c: Update to use the new library.
> * Makefile: (lib-subdirs): Add library.
> ---
>  Makefile   |   2 +-
>  libhurdutil/Makefile   |  28 +++
>  libhurdutil/hurdutil.h |  78 
>  libhurdutil/settrans.c | 377 +++
>  utils/Makefile |   4 +-
>  utils/settrans.c   | 475 
> +++--
>  6 files changed, 594 insertions(+), 370 deletions(-)
>  create mode 100644 libhurdutil/Makefile
>  create mode 100644 libhurdutil/hurdutil.h
>  create mode 100644 libhurdutil/settrans.c
> 
> diff --git a/Makefile b/Makefile
> index d48baaa..e712767 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -29,7 +29,7 @@ include ./Makeconf
>  lib-subdirs = libshouldbeinlibc libihash libiohelp libports libthreads \
>   libpager libfshelp libdiskfs libtrivfs libps \
>   libnetfs libpipe libstore libhurdbugaddr libftpconn libcons \
> - libhurd-slab
> + libhurd-slab libhurdutil
>  
>  # Hurd programs
>  prog-subdirs = auth proc exec term \
> diff --git a/libhurdutil/Makefile b/libhurdutil/Makefile
> new file mode 100644
> index 000..2e0e642
> --- /dev/null
> +++ b/libhurdutil/Makefile
> @@ -0,0 +1,28 @@
> +#   Copyright (C) 2016 Free Software Foundation, Inc.
> +#
> +#   This file is part of the GNU Hurd.
> +#
> +#   The GNU Hurd is free software; you can redistribute it and/or
> +#   modify it under the terms of the GNU General Public License as
> +#   published by the Free Software Foundation; either version 2, or (at
> +#   your option) any later version.
> +#
> +#   The GNU Hurd is distributed in the hope that it will be useful, but
> +#   WITHOUT ANY WARRANTY; without even the implied warranty of
> +#   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +#   General Public License for more details.
> +#
> +#   You should have received a copy of the GNU General Public License
> +#   along with this program; if not, write to the Free Software
> +#   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.

This is an old license template, the current one doesn't list a
snailmail address, but some URL.

> +
> +dir := libhurdutil
> +makemode := library
> +
> +libname := libhurdutil
> +SRCS = settrans.c
> +installhdrs = hurdutil.h
> +
> +OBJS = $(SRCS:.c=.o)
> +
> +include ../Makeconf
> diff --git a/libhurdutil/hurdutil.h b/libhurdutil/hurdutil.h
> new file mode 100644
> index 000..5786cff
> --- /dev/null
> +++ b/libhurdutil/hurdutil.h
> @@ -0,0 +1,78 @@
> +/* hurdutil.h - Hurd utils interface.
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +   Written by Manolis Fragkiskos Ragkousis .
> +
> +   This file is part of the GNU Hurd.
> +
> +   The GNU Hurd is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU General Public License as
> +   published by the Free Software Foundation; either version 2, or (at
> +   your option) any later version.
> +
> +   The GNU Hurd is distributed in the hope that it will be useful, but
> +   WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with the GNU Hurd; if not, write to the Free Software
> +   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.  */
> +
> +#ifndef _HURD_UTIL_H
> +#define _HURD_UTIL_H
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +

As Guillem noted, we might want to change the way options are passed
into the library.  Let's see...

> +struct settrans_flags
> +{
> +  /* The name of the node we're putting the translator on. */
> +  char *node_name;
> +
> +  /* Flags to pass to file_set_translator.  */
> +  int lookup_flags;
> +  int goaway_flags;

These two contain ORed flags.

> +  /* Various option flags.  */
> +  int passive;
> +  int active;
> +  int keep_active;
> +  int pause;
> +  int kill_active;
> +  int orphan;
> +  int start;
> +  int stack;
> +  int excl;
> +  int timeout;

These are boolean, and the idiomatic way would be to OR some constants
containing powers of two together.  Or use a bitfield, that might be
more modern, I don't know.

> +  char *pid_file;
> +  char *underlying_node_name;
> +  int underlying_lookup_flags;
> +  char **chroot_command;
> +  char *chroot_chdir;
> +
> +  /* The translator's arg vector, in '\0' separated format.  */
> +  char *argz;
> +  size_t argz_len;
> +};

So only using bitflags is not enough, we have plenty of other kind of
options.  Maybe we 

Re: [PATCH] libhurdutil: New library containing utils to be used by Guix.

[Justus Winter]

Quoting manolis...@gmail.com (2016-05-25 17:05:56)
> 
> I am resending the patch from 
> https://lists.gnu.org/archive/html/bug-hurd/2016-05/msg00040.html modified to 
> apply on master. 

git send-email --in-reply-to=MSGIDGOESHERE is your friend ;)

Justus

Re: [PATCH] drop the deprecated malloc/free hooks in hurd/mach-defpager

[Justus Winter]

Quoting Justus Winter (2016-05-18 13:27:13)
> > Backing up one step: what is actually being done here in mach-defpager?
> > Again from just a quick look, is the idea to use wired memory for all of
> > mach-defpager's process memory?

Yes, wiring the memory is required to avoid deadlocks [0].

0: 
http://darnassus.sceen.net/~rbraun/hurd_related_papers/mach/moving_the_default_memory_manager_out_of_the_mach_kernel.pdf

> > So, for example, if now some mach-defpager code calls calloc, it will now
> > get non-wired "glibc" memory instead of wired "kalloc" memory.  The
> > problem here is not mach-defpager's own code (which we can easily
> > verify/control), but the problems is the libraries it links against,
> > which currently are: libihash, libpthread, glibc itself, and any
> 
> And indeed libihash uses calloc.
> 
> > dependencies these may have.  Can we be sure these are not internally
> > using calloc, for example?
> > 
> > Now, this is not a new problem that you introduced ;-) -- previously we
> > also didn't provide malloc's memalign and realloc hooks, and realloc is
> > used quite commonly, I suppose?
> > 
> > If my theories are correct, do we need to use a different mechanism to
> > get the whole mach-defpager process into wired memory?
> 
> As Richard said in #hurd, implement mlockall and MCL_FUTURE and just
> use the default allocator.
> 
> I'd suggest reverting that change, or is the removal of that
> deprecated interface imminent?

Wdyt?

Cheers,
Justus

Re: [PATCH] drop the deprecated malloc/free hooks in hurd/mach-defpager

[Justus Winter]

Hey :)

Quoting Thomas Schwinge (2016-05-16 18:49:47)
> Hi!
> 
> On Tue, 29 Dec 2015 23:09:54 +0100, Flavio Cruz  wrote:
> > mach-defpager: Drop the deprecated malloc/free hooks.
> 
> (After approval by Samuel, this became commit
> 8c49801c8f7e3f800cabedf8fca8ccec3cf35a22.)
> 
> > * mach-defpager/kalloc.c: Define malloc and free directly since those are 
> > weak
> > symbols.
> 
> The idea here is to just change the mechanism (override glibc's weak
> symbols instead of using glibc's deprecated malloc hooks), while not
> changing any semantics, right?
> 
> From a quick look, I have some doubts about this generally (so, not only
> related to your change).
> 
> > --- a/mach-defpager/kalloc.c
> > +++ b/mach-defpager/kalloc.c
> 
> > -static void *
> > -malloc_hook (size_t size, const void *caller)
> > +void *
> > +malloc (size_t size)
> >  {
> >return (void *) kalloc ((vm_size_t) size);
> >  }
> >  
> > -static void
> > -free_hook (void *ptr, const void *caller)
> > +void
> > +free (void *ptr)
> >  {
> >/* Just ignore harmless attempts at cleanliness.  */
> >/* panic("free not implemented"); */

Uh, we actually don't free memory here...

> So, here we now only provide the malloc and free symbols.  What happens
> if, for example, calloc, realloc, posix_memalign et al. are called?
> 
> For example, looking at [glibc]/malloc/malloc.c:__libc_calloc (weak alias
> for calloc).  Before your change, it called mach-defpager's malloc_hook,
> and then zeroed that region.  After your change, it is implemented using
> the real glibc calloc -- which actually does change the semantics in an
> unintended way?
> 
> Backing up one step: what is actually being done here in mach-defpager?
> Again from just a quick look, is the idea to use wired memory for all of
> mach-defpager's process memory?
> 
> So, for example, if now some mach-defpager code calls calloc, it will now
> get non-wired "glibc" memory instead of wired "kalloc" memory.  The
> problem here is not mach-defpager's own code (which we can easily
> verify/control), but the problems is the libraries it links against,
> which currently are: libihash, libpthread, glibc itself, and any

And indeed libihash uses calloc.

> dependencies these may have.  Can we be sure these are not internally
> using calloc, for example?
> 
> Now, this is not a new problem that you introduced ;-) -- previously we
> also didn't provide malloc's memalign and realloc hooks, and realloc is
> used quite commonly, I suppose?
> 
> If my theories are correct, do we need to use a different mechanism to
> get the whole mach-defpager process into wired memory?

As Richard said in #hurd, implement mlockall and MCL_FUTURE and just
use the default allocator.

I'd suggest reverting that change, or is the removal of that
deprecated interface imminent?

Cheers,
Justus

Re: ext2fs crash

[Justus Winter]

Hi,

Quoting Thomas Schwinge (2016-05-13 23:02:08)
> Hi!
> 
> On Mon, 09 May 2016 16:11:52 +0200, Justus Winter 
> <4win...@informatik.uni-hamburg.de> wrote:
> > Quoting Thomas Schwinge (2016-04-27 22:53:33)
> > > [...]
> > > I suppose we want to fix (or, at least understand) that problem before
> > > making the releases?
> > 
> > No.  Let's take what we have and release it.  Even though some people
> > are having trouble with ext2fs, for most people it is working really
> > well (e.g. the build daemons).
> 
> So, the ext2fs issues (bogus port deallocations, crash) are something to
> look into (later, I agree), [...]

I believe I can shed some light on the former problem:

$ settrans -ac hi /hurd/hello ; cat hi ; pkill hello ; rm hi
Hello, world!
(kernel complaining about ext2fs doing bogus port operations)

The problem is that libfshelp doesn't handle translators dying right.
I added code for the recursive mtab translator to do that, assuming
that the existing code copes with translators going away, but it does
not.  This merely needs to be implemented.

Justus

Re: ext2fs crash

[Justus Winter]

Hi :)

Quoting Thomas Schwinge (2016-04-27 22:53:33)
> Before preparing the next set of releases, I wanted to sort-of
> sanity-check what we currently got (at least in terms of Debian
> packages), so yesterday dist-upgraded my Debian GNU/Hurd system to the
> latest packages, and started a GCC bootstrap build and testsuite run.
> That one didn't complete successfully...
> 
> I can confirm that the
> 
> issue appears to be fixed -- yay, thanks!

:)

> [...]
> I suppose we want to fix (or, at least understand) that problem before
> making the releases?

No.  Let's take what we have and release it.  Even though some people
are having trouble with ext2fs, for most people it is working really
well (e.g. the build daemons).

Do you need help with the announcements?

Justus

Re: Minor bug introduced by commit b04364b

[Justus Winter]

Quoting Esa Peuha (2016-05-06 18:39:11)
> Most changes in commit b04364b seem to be fine, but this one isn't:
> 
> @@ -405,7 +401,7 @@ run (const char *server, mach_port_t *ports, task_t *task)
>  }
> 
>if (verbose)
> -fprintf (stderr, "started %s\n", prog);
> +fprintf (stderr, stderr, "started %s\n", prog);

Ewww :( fixed, thanks :)

Justus

Re: [PATCH] Simple testsuite for MIG.

[Justus Winter]

Quoting Flavio Cruz (2016-04-19 01:48:50)
> This includes a set of valid and invalid definition files that MIG will
> try to process. For valid definitions, GCC will compile the stubs to
> check if valid C code was generated.

Merged, many thanks :)

Justus

Re: [PATCH] Use uint32_t instead of unsigned32_t.

[Justus Winter]

Quoting Flávio Cruz (2016-04-05 01:05:43)
> On 5 April 2016 at 00:59, Justus Winter <4win...@informatik.uni-hamburg.de>
> wrote:
> 
> > Quoting Justus Winter (2016-04-05 00:46:11)
> > > Quoting Flávio Cruz (2016-04-05 00:39:48)
> > > > > This broke your test suite:
> > > > Should work with the new patch. Thanks for the report.
> > >
> > > It does indeed.
> >
> > Furthermore, I was able to build a seemingly working Hurd package
> > using the fixed MIG, but I see these kind of warnings that never
> > appeared before (and neither have I seen the polymorphic type being
> > used):
> >
> > fsServer.c: In function ‘_Xfile_getcontrol’:
> > fsServer.c:1086:22: warning: large integer implicitly truncated to
> > unsigned type [-Woverflow]
> >/* msgt_name = */  MACH_MSG_TYPE_POLYMORPHIC,
> >   ^
> 
> 
> Mig was using the polymorphic type before but was outputting '-1' instead
> of MACH_MSG_TYPE_POLYMORPHIC.
> The other patch to change the definition of mach_msg_type_name_t to
> unsigned char will fix this. I'm preparing an updated version.

Oh good :)

Justus

Re: [PATCH] Use uint32_t instead of unsigned32_t.

[Justus Winter]

Quoting Justus Winter (2016-04-05 00:46:11)
> Quoting Flávio Cruz (2016-04-05 00:39:48)
> > > This broke your test suite:
> > Should work with the new patch. Thanks for the report.
> 
> It does indeed.

Furthermore, I was able to build a seemingly working Hurd package
using the fixed MIG, but I see these kind of warnings that never
appeared before (and neither have I seen the polymorphic type being
used):

fsServer.c: In function ‘_Xfile_getcontrol’:
fsServer.c:1086:22: warning: large integer implicitly truncated to unsigned 
type [-Woverflow]
   /* msgt_name = */  MACH_MSG_TYPE_POLYMORPHIC,
  ^

Justus

Re: [PATCH] Use uint32_t instead of unsigned32_t.

[Justus Winter]

Quoting Flávio Cruz (2016-04-05 00:39:48)
> > This broke your test suite:
> Should work with the new patch. Thanks for the report.

It does indeed.

Thanks,
Justus

Re: [PATCH] Use posix types for mig definitions.

[Justus Winter]

Quoting Flavio Cruz (2016-03-15 10:49:51)
> * hurd/hurd_types.defs: Use posix types.
> * libfshelp/start-translator-long.c: Use uint32_t instead of
> unsigned32_t.

This broke the Debian/Hurd package build.

Justus

Re: [PATCH] Use uint32_t instead of unsigned32_t.

[Justus Winter]

Quoting Flavio Cruz (2016-03-20 22:01:10)
> * Makefile.am: Use -ffreestanding and remove -nostdinc since we are including 
> stdint.h.
> * i386/i386/xen.h: Use uint64_t.
> * i386/include/mach/i386/machine_types.defs: Use uint32_t and int32_t.
> * i386/include/mach/i386/vm_types.h: Remove definitions of int*, uint*,
> unsigned* and signed* types.

This broke the xen port.

Justus

Re: [PATCH] Use uint32_t instead of unsigned32_t.

[Justus Winter]

Quoting Flavio Cruz (2016-03-15 10:48:48)
> * utils.c: Generate code using uint32_t.
> ---
>  utils.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/utils.c b/utils.c
> index 4c2a87b..ccde966 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -77,7 +77,7 @@ WriteBogusDefines(FILE *file)
>  
>  fprintf(file, "#define BAD_TYPECHECK(type, check) mig_unlikely (({\\\n");
>  fprintf(file,
> -   "  union { mach_msg_type_t t; unsigned32_t w; } _t, _c;\\\n");
> +   "  union { mach_msg_type_t t; uint32_t w; } _t, _c;\\\n");

This broke your test suite:

FAIL: case
==

Generating stubs for case...
Compiling stubs for case...
case-server.c: In function ‘_Xfactorial’:
case-server.c:35:30: error: unknown type name ‘uint32_t’
union { mach_msg_type_t t; uint32_t w; } _t, _c;\
^
case-server.c:23:47: note: in definition of macro ‘mig_unlikely’
#define mig_unlikely(X) __builtin_expect (!! (X), 0)
^
case-server.c:93:6: note: in expansion of macro ‘BAD_TYPECHECK’
if (BAD_TYPECHECK(>nType, ))
^
ERROR: Could not compile stubs for case
FAIL case.defs (exit status: 1)

(likewise directions.defs)

Justus

>  fprintf(file,
> "  _t.t = *(type); _c.t = *(check);_t.w != _c.w; }))\n");
>  }
> -- 
> 2.6.4
> 
> 

Re: [PATCH] Turn mach_msg_type_{name,size}_t into unsigned chars.

[Justus Winter]

Quoting Samuel Thibault (2016-04-04 09:16:17)
> Hello,
> 
> Flavio Cruz, on Sun 03 Apr 2016 16:12:48 +0200, wrote:
> > mach_msg_type_name_t and mach_msg_type_size_t are used as 8 bit fields in
> > mach_msg_type_t struct, therefore they should be treated as unsigned char's 
> > to
> > avoid warnings when using macros such as MACH_MSG_TYPE_POLYMORPIC.
> 
> Mmm, looking quickly at the mach code shows that there is a "long"
> variant of these fields, see for instance ipc_kmsg_clean_body() which
> uses a mach_msg_type_long_t

I don't see a problem with that for mach_msg_type_name_t.  The long
form is used if e.g. an array is transmitted with a size that cannot
be expressed with mach_msg_type_t, but there cannot be at type that
doesn't fit in 8 bits afaics.

If we restrict mach_msg_type_size_t to 8 bits, we limit the size a
single object in an array can have even for the long form.

(Obligatory rant: Why is the size of an element expressed in bits even
though the code everywhere in Mach assumes it to be a multiple of
eight and pads it accordingly?  Something to fix for Mach5...)

Justus

Re: [PATCH] Simple testsuite for MIG.

[Justus Winter]

I like it a lot.  May I suggest the following robustness fix?  Seems
like the empty files are lost somewhere in my ci/package building
setup.  Also I think we want copyright notices at least in the
framework files, not sure about the test vectors.  Thougths?

Justus

[PATCH mig] Robustness fix.

[Justus Winter]

---
 tests/includes/all.h| 1 +
 tests/includes/server.h | 1 +
 tests/includes/user.h   | 1 +
 3 files changed, 3 insertions(+)

diff --git a/tests/includes/all.h b/tests/includes/all.h
index e69de29..6e25b02 100644
--- a/tests/includes/all.h
+++ b/tests/includes/all.h
@@ -0,0 +1 @@
+/* Empty.  */
diff --git a/tests/includes/server.h b/tests/includes/server.h
index e69de29..6e25b02 100644
--- a/tests/includes/server.h
+++ b/tests/includes/server.h
@@ -0,0 +1 @@
+/* Empty.  */
diff --git a/tests/includes/user.h b/tests/includes/user.h
index e69de29..6e25b02 100644
--- a/tests/includes/user.h
+++ b/tests/includes/user.h
@@ -0,0 +1 @@
+/* Empty.  */
-- 
2.1.4

Re: [PATCH] Simple testsuite for MIG.

[Justus Winter]

Quoting Flavio Cruz (2016-03-25 17:19:06)
> This includes a set of valid and invalid definition files that MIG will
> try to process. For valid definitions, GCC will compile the stubs to
> check if valid C code was generated.
> 
> * configure.ac: Add new test Makefiles.
> * Makefile.am: Add SUBDIRS.
> * tests/Makeconf.am: Automake definitions shared by all test subdirectories.
> * tests/test_lib.sh: Library of functions shared by all test drivers.
> * tests/good/run_good_test.sh: Script to run valid definition files.
> * tests/good/Makefile.am: New file.
> * tests/bad/run_bad_test.sh: Script to run invalid definition files.
> * tests/bad/Makefile.am: New file.
> * tests/generate-only/run_generate_only.sh: Script to run valid
> definition files that should be generated only.
> * tests/generate-only/Makefile.am: New file.
> * tests/includes/*.h: Test header files that are included by test stubs.
> * tests/good/*.defs: Valid definition files where generated stubs can
> be compiled.
> * tests/generate_only/*.defs: Valid definition files that can be
> generated but no compilation should be attempted.
> * tests/bad/*.defs: Definition files with some mistakes that should be
> recognized by MIG.
> ---
> 
> Hey,
> 
> On Wed, Mar 23, 2016 at 11:17:03AM +0100, Justus Winter wrote:
> > Quoting Flavio Cruz (2016-03-23 01:31:13)
> > > This includes a set of valid and invalid definition files that MIG will
> > > try to process. For valid definitions, GCC will compile the stubs to
> > > check if valid C code was generated.
> > 
> > Love the idea, but 1/ this breaks out of tree (vpath) builds, 2/ I
> > believe the traditional GNU make target for tests is 'check', 3/ the
> > test suite fails on Linux b/c of missing mach/mig_support.h, finally
> > 4/ automake has a test driver that might be useful.
> 
> Thanks for the comments! I think I have addressed all of them. Let me
> know if there's anything else missing.

Awesome!  Unfortunately:


   GNU MIG 1.6: tests/good/test-suite.log
   

# TOTAL: 7
# PASS:  6
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0

.. contents:: :depth: 2

FAIL: import


Generating stubs for import...
Compiling stubs for import...
import-server.c:42:20: fatal error: server.h: No such file or directory
compilation terminated.
ERROR: Could not compile stubs for import
FAIL import.defs (exit status: 1)

Justus

Re: [GSoC] Porting GuixSD to GNU Hurd draft

[Justus Winter]

Hi,

Quoting Manolis Ragkousis (2016-03-24 14:55:31)
> On 03/24/16 15:22, Ludovic Courtès wrote:
> >>> The main question is whether you should implement build isolation in
> >>> guix-daemon, in which case that would leave little time for the GuixSD
> >>> parts.  I think I would rather let you focus on the GuixSD stuff as you
> >>> wrote, but I’d like to hear what the Hurd folks think.
> >>
> >> I consider isolation more important.
> > 
> > OK.
> 
> Isolation first it is then.
> 
> > So, Manolis, what about reframing the agenda such that porting
> > guix-daemon to GNU/Hurd comes first (I’d consider it roughly half of the
> > programming effort), followed by GuixSD stuff?
> > 
> 
> Current objectives then:
> 1) Achieve build isolation in the daemon on the Hurd.
> 2) Modify Guix so it can produce a working image, while isolating any
> cases of Linux assumptions.
> 3) Boot to GuixSD

Well, I didn't ment to stomp over your priorities like that, it is just
that I personally consider it more important and from my point of view
it should be easier to achieve.

Justus

Re: [GSoC] Porting GuixSD to GNU Hurd draft

[Justus Winter]

Quoting Manolis Ragkousis (2016-03-24 12:18:25)
> >>> The project consists of four main stages
> >>>
> >>> 1. Modify Guix so it will be able to create and mount the file-system 
> >>> needed to boot into a system with Hurd at its core. 
> >>> 2. Modify Guix so it can produce a working image, while isolating any 
> >>> cases of Linux assumptions.
> >>> 3. Successfully boot into one such system using GNU Shepherd with pid 1.
> >>> 4. Modify the new Guix system to take advantage of Hurd specific 
> >>> mechanisms.
> > 
> > For me, 4. is the most important bit, so we can build packages in
> > isolation.
> 
> I think our priority should be to first get GuixSD working and then
> concentrate to achieving isolation.  From what I understand none of the
> two is trivial but in the long run the ability to spawn GNU/Hurd system
> vms on the fly will make it easier to work on it.

Otoh if we could properly build packages, we could provide the
substitutes, so people could try Guix on the Hurd without going
through the 12h+ bootstrap procedure.

> Currently I am working on familiarizing myself with both how Hurd and
> GuixSD booting works. I am already using GuixSD as a system and I am
> going though the source code so I can better understand what is going on.

Here is an overview of the early server bootstrap in the Hurd.  It is
slightly outdated, but still the best description that I know of:

http://teythoon.cryptobitch.de/posts/bootstrapping-the-hurd/

> >> For debugging purposes, it would be very helpful to say the least to
> >> have a working ‘guix system vm’: it would allow you to test your changes
> >> very quickly, without rebooting and so on.
> >>
> >> This in itself requires some thought: currently (guix system vm) relies
> >> on QEMU’s ‘-kernel’ option to boot the kernel Linux.  For GNU/Hurd, we
> >> instead need to boot a complete image with GRUB, like ‘guix system
> >> vm-image’ does.  You’ll have to investigate how to port this.
> > 
> > qemu can boot multiboot operating systems.
> 
> Justus is correct here. I found this
> https://www.gnu.org/software/hurd/hurd/running/qemu.html#index9h1 which
> explains how to make qemu start with gnumach.  This way guix system vm
> can work, just with the proper modifications.

Hmmm, so one still needs a filesystem, right?  That's going to be a
bit tricky too, since whatever tool you use for that purpose, it
surely does not support creating hurdish passive translator records.
Without passive translator records things get indeed more interesting.
I have a patch for some tool for creating ext2 filesystems that could
help, or we create all the passive translator records on first boot
similar to how Samuels Debian/Hurd live cds deal with that.  Or I
finish my bootshell work that can boot from filesystems without
passive translator records.

Justus

Re: [GSoC] Porting GuixSD to GNU Hurd draft

[Justus Winter]

Hi,

Quoting Ludovic Courtès (2016-03-23 14:40:38)
> > 2. The Project
> >
> > The project consists of four main stages
> >
> > 1. Modify Guix so it will be able to create and mount the file-system 
> > needed to boot into a system with Hurd at its core. 
> > 2. Modify Guix so it can produce a working image, while isolating any cases 
> > of Linux assumptions.
> > 3. Successfully boot into one such system using GNU Shepherd with pid 1.
> > 4. Modify the new Guix system to take advantage of Hurd specific mechanisms.

For me, 4. is the most important bit, so we can build packages in
isolation.

> > Currently the tools Guix uses to interact with the filesystem exist inside 
> > the (guix build syscalls) module. 
> > This module provides bindings to libc's syscall wrappers, which are only 
> > available on a GNU/Linux system. 
> > In order to offer the same functionality on a GNU/Hurd system we must first 
> > write Guile bindings for the 
> > relevant Hurd functions, like 'file_set_translator' in hurd/fs.defs
> > for example.
> 
> Note that technically the ‘file_set_translator’ function is in libc:
> 
> --8<---cut here---start->8---
> scheme@(guile-user)> (dynamic-func "file_set_translator" (dynamic-link))
> $1 = #
> --8<---cut here---end--->8---
> 
> > This module will be called (guix build hurd). This allows us to
> > re-implement the functionality of the 'settrans' command, as described
> > here[1], in Scheme and use that in place of 'mount()', where
> > applicable.

Right.  In fact, what settrans (or mount) does isn't that hard to
reproduce, though I wouldn't mind moving the c implementation to
libhurdutil or the like and binding to that.

> I think it’s important to think about:
> 
>   1. How functionality equivalent to linux-{initrd,boot} will be
>  implemented on the Hurd.
> 
>  In my experience the equivalent thing is simpler on the Hurd,
>  esp. if relying on passive translators (see daemons/runsystem.sh in
>  the Hurd), though here we’ll probably explicitly activate
>  translators at boot time.

Currently, there is not really a reason to have an initrd-like
solution on the Hurd.  Yes, one has to start the default pager
explicitly, but that's about it.

> > Finally, once GuixSD is successfully ported, we can cater to the last stage 
> > of taking advantage of Hurd specific 
> > mechanisms. 
> > This includes but is not limited to:
> > 1)Replacing (gnu build linux-container) with (gnu build subhurd) when on a 
> > GNU/Hurd system. Subhurds can offer 
> > isolation similar to Linux containers as described here[3].
> 
> This is really super optional IMO.  This module is only used by ‘guix
> environment --container’, which is an optional feature.

Yes, subhurds are more on the experimental side imho.

> > 2)Modify the guix-daemon to run without root privileges by utilizing the 
> > Hurd architecture. The daemon needs root 
> > privileges in order to setup chrooted environments for the builds.  In the 
> > Hurd this can be done by setting up a 
> > translator as described here[4]. 
> 
> This is more important, and already non-trivial work.  If libc provided
> ‘mount’ with support for MS_BIND (which I think it should), it would
> help a bit, but there’s still the problem of the other Linux name spaces
> that are used (the CLONE_NEW* clone(2) flags.)
> 
> Thus it may make more sense to write this functionality in guix-daemon
> using directly the Hurd interfaces.  Separate PID name spaces, UID name
> spaces, mount name spaces, etc. are fairly natural on the Hurd: it’s
> “just” a matter of giving the child process ports to separate proc,
> auth, etc. translators.
> 
> In itself this is also a bit of work.  I wonder what the Hurd folks
> think about priorities here?

I'd go for specializing guix-daemon over trying too hard to implement
Linux-compatible interfaces in the libc.  I consider the
filesystem-isolation part easy, UID isolation relatively easy, PID
isolation is a bit tricky.  Currently one cannot simply start another
proc server and expect it to work as it needs privileged kernel
interfaces.  I created an RPC to allow nested proc servers for
unprivileged subhurds.  That should do the trick, it is merely a
matter of making it actually work I guess.

> > 3)Guix uses symlink trees for user profiles. Instead we can use stowfs[5]. 
> > Stowfs creates a traditional Unix directory 
> > structure from all the files in the individual package directories.
> 
> Fun but optional.  ;-)

Agreed.

> > May 2 - May 22
> > Create (gnu build hurd-boot) and (gnu build hurd-initrd).
> > Start working on describing the GNU/Hurd system in (gnu system).
> 
> I know Debian at some point added initrd support to GNU Mach for some
> reason, but fundamentally, the whole point of multiboot is to provide a
> solution more flexible than initrds.  So, hopefully, no initrds here.
> Since there’s no initrd, there’s also no ‘switch_root’ dance 

Re: Dangerous use of alloca

[Justus Winter]

Hi,

Quoting Agustina Arzille (2016-03-16 02:44:28)
> As promised, here's the patch to avoid the aforementioned alloca bugs.
> It's a bit of an overkill, imo, but it does support ridiculously long
> command-line arguments =D

It is way too complicated.  We don't need to be efficient here.

Also, your patch got mangled by your MUA.  Please use git send-email
to submit patches.  It also contains a spurious whitespace change at
the end.

Thanks,
Justus

Re: [PATCH] Automatically generate parser.h.

[Justus Winter]

Quoting Flavio Cruz (2016-03-14 21:22:34)
> * Makefile.am: Set AM_YFLAGS to -d to generate parser.h. Add parser.h to
> CLEANFILES and include parser.h as a dependency of lexxer.c.
> * lexxer.l: Declare yyerror here.
> * parser.h: Remove file since yacc will generate the same content
> automatically.

Merged, thanks!

Justus

Re: Dangerous use of alloca

[Justus Winter]

Quoting Samuel Thibault (2016-03-15 18:30:16)
> Justus Winter, on Tue 15 Mar 2016 18:10:18 +0100, wrote:
> > Quoting Samuel Thibault (2016-03-15 17:31:53)
> > > Justus Winter, on Tue 15 Mar 2016 14:39:00 +0100, wrote:
> > > > Quoting Agustina Arzille (2016-03-15 14:11:27)
> > > > > Hello, everyone.
> > > > > 
> > > > > I was browsing some gnumach source files, and noticed some dangerous 
> > > > > use
> > > > > of the gcc builtin 'alloca'. In the file kern/bootstrap.c, lines 212 
> > > > > and
> > > > > 228, we have the following statement:
> > > > > 
> > > > > memcpy (alloca (len), ...)
> > > > 
> > > > Good catch.  However, alloca is dangerous even if used correctly, and
> > > > is best be avoided.
> > > 
> > > What do you mean by "dangerous"?  I don't think we'll ever get a command
> > > line that does not fit into the stack.
> > 
> > I don't see why not actually, unless grub limits this somehow O:-)
> 
> It doesn't indeed, so it is definitely possible; I just don't see
> anybody typing a 4096-characters command line :)

Not typing of course, but I'm pretty sure the grub scripting language
is turing complete, and I can actually picture a use case: Serialize
the grub environment and pass it on to the boot shell which can make
further use of it.

Justus

Re: Dangerous use of alloca

[Justus Winter]

Quoting Samuel Thibault (2016-03-15 17:31:53)
> Justus Winter, on Tue 15 Mar 2016 14:39:00 +0100, wrote:
> > Quoting Agustina Arzille (2016-03-15 14:11:27)
> > > Hello, everyone.
> > > 
> > > I was browsing some gnumach source files, and noticed some dangerous use
> > > of the gcc builtin 'alloca'. In the file kern/bootstrap.c, lines 212 and
> > > 228, we have the following statement:
> > > 
> > > memcpy (alloca (len), ...)
> > 
> > Good catch.  However, alloca is dangerous even if used correctly, and
> > is best be avoided.
> 
> What do you mean by "dangerous"?  I don't think we'll ever get a command
> line that does not fit into the stack.

I don't see why not actually, unless grub limits this somehow O:-)

Justus

Re: Dangerous use of alloca

[Justus Winter]

Quoting Agustina Arzille (2016-03-15 15:44:13)
> > Good catch.  However, alloca is dangerous even if used correctly, and
> > is best be avoided.  Would you be so kind to prepare a patch to that
> > end?
> > 
> > Thanks,
> > Justus
> 
> Sure. I will cook up a patch when I get home. Right now, the 
> alternatives
> to using alloca that I can think of are either:
> 1) A global buffer with a fixed size, or
> 2) 'kalloc' and friends.
> 
> Obviously, (2) would be the preferred option, but I'm not sure if it's
> completely available during bootstrap. What do you guys think?

The bootscript parser creates userspace tasks.  At that point the
kernel is fully initialized, feel free to use kalloc.

Re: Dangerous use of alloca

[Justus Winter]

Hi,

Quoting Agustina Arzille (2016-03-15 14:11:27)
> Hello, everyone.
> 
> I was browsing some gnumach source files, and noticed some dangerous use
> of the gcc builtin 'alloca'. In the file kern/bootstrap.c, lines 212 and
> 228, we have the following statement:
> 
> memcpy (alloca (len), ...)

Good catch.  However, alloca is dangerous even if used correctly, and
is best be avoided.  Would you be so kind to prepare a patch to that
end?

Thanks,
Justus

Re: fakeroot-hurd doesn't work with scripts

[Justus Winter]

Quoting Samuel Thibault (2016-03-07 20:49:01)
> Justus Winter, on Mon 07 Mar 2016 20:30:37 +0100, wrote:
> > teythoon@hurdbox ~ % for x in sh py pl; do cat test.$x ; fakeroot ./test.$x 
> > ; done
> 
> I should have mentioned it: the issue is only for scripts coming from
> PATH. When you specify the path, the interpreter will know how to open
> the file.

Uh, my bad, this is easy to fix.

Justus

Re: fakeroot-hurd doesn't work with scripts

[Justus Winter]

Hi,

Quoting Samuel Thibault (2016-03-04 18:39:21)
> We have an issue between fakeroot and scripts (from the borgbackup build
> log):
> 
> > fakeroot dh_auto_test
> > Can't open perl script "dh_auto_test": No such file or directory
> 
> I guess that's due to the way fakeauth starts the program, I haven't
> looked further.

I cannot reproduce this:

teythoon@hurdbox ~ % for x in sh py pl; do cat test.$x ; fakeroot ./test.$x ; 
done
#!/bin/sh
echo $0
ids
./test.sh
effective uids: 0(root)
effective gids: 0(root)
available uids: 0(root) 0(root)
available gids: 0(root) 0(root)
#!/usr/bin/env python
import sys
print(sys.argv[0])
./test.py
#!/usr/bin/env perl

print "Hello from perl";
system("ids");
Hello from perleffective uids: 0(root)
effective gids: 0(root)
available uids: 0(root) 0(root)
available gids: 0(root) 0(root)

Justus

Re: refcounts assertion

[Justus Winter]

Quoting Samuel Thibault (2016-03-07 01:40:01)
> Hello,
> 
> Some kind of refcount assertion failure we have been having is "refcount
> detected use-after-free!" from the call to refcounts_ref inside
> _ports_bucket_class_iterate, called for instance by the periodic sync.
> I'm thinking: what about this scenario:
> 
> - thread A calls diskfs_nput()
> - that calls refcounts_deref_weak(), which gets both hard and weak
>   counts to 0
> - thread B calls diskfs_sync_everything, which calls
>   ports_bucket_iterate, which calls _ports_bucket_class_iterate
> - _ports_bucket_class_iterate iterates over nodes, and calls
>   refcounts_ref for the node deref-ed above. It then traps on the
>   assertion.
> 
> So the situation is that diskfs_nput is releasing a node while
> sync_everything is trying to sync it.

That hypothesis should be easy enough to test.  It's a good bet
though, the iteration code is fishy enough as it is.

I have a patch that revamps it so that we avoid the copying.  The
thing with the patch is (iirc) that it requires the called function to
be idempotent, which afaics every function used with the
_ports_bucket_class_iterate were.  Or maybe the patch was for the node
iteration function, I don't remember and cannot check currently.

> Just using refcounts_unsafe_ref is most probably not a solution since
> the node is on its way out, the function called by the iterator will
> most probably get it all wrong on a node being destroyed.
> 
> Another way would be as attached: just skip nodes which didn't have any
> reference.  I however don't know libports enough to say whether that's
> correct behavior for libports or not:

That looks like a brown-paper fix that could easily use-after-free.
refcounts_unsafe_ref can be safely used in libports because of the
quiescent thread thing.

> AIUI callers of refcounts_deref* are supposed to check for 0,0 and
> are then responsible for destroying the node?

Yes.

> 
> Samuel