Re: [PATCH v2] Add io space MMAP for memory mapping devices and files

[Andreas Klinger]

Hi Jose,

"Jose E. Marchesi" <jemarch@gnu.org> schrieb am Mo, 08. Jan 00:03:
> 
> > Hi again,
> >
> > Andreas Klinger <ak@it-klinger.de> schrieb am Sa, 06. Jan 20:42:
> >> Hi Jose,
> >> 
> >> "Jose E. Marchesi" <jemarch@gnu.org> schrieb am Sa, 06. Jan 20:14:
> >> > > I think that the approach of patch v2 could be optimized by a .set 
> >> > > alignment
> >> > > variable for those guys who are aware of such hardware constraints on 
> >> > > their
> >> > > architecture and want to optimize this. But I would set the default to 
> >> > > a value
> >> > > which works for all, which is the size of the address bus.
> >> > 
> >> > I think it is perfectly ok to have IOD specific configuration
> >> > parameters, sure.
> >> > 
> >> > Let's make it a Poke variable that the user can customize, plus a
> >> > setting in the registry so the .set dot-command works on it.  I suppose
> >> > it shall have the name mmap in it, like pk_mmap_alignment.  WDYT?
> >> 
> >> great. It makes sense to name it mmap as it only applies to mmap io spaces.
> > [...]
> >
> > After thinking about the problem again I came to a different solution which 
> > is
> > much simpler, doesn't need an additional .set variable and avoids the signal
> > handler as well as it is copying with the address bus size if possible, thus
> > avoiding performance bottlenecks. The read function is as follows:
> >
> > static int
> > ios_dev_mmap_pread (void *iod, void *buf, size_t count, ios_dev_off offset)
> > {
> >   struct ios_dev_mmap *dev_map = iod;
> >   int align = sizeof(void*);
> >   char *m = buf;
> >
> >   if (offset > dev_map->size || count > dev_map->size - offset)
> >     return IOD_EOF;
> >
> >   /* copy unaligned bytes */
> >   while (count && offset % align)
> >     {
> >       memcpy (m, dev_map->addr + offset, 1);
> >       count--;
> >       offset++;
> >       m++;
> >     }
> >
> >   /* copy with the address bus size */
> >   while (count >= align)
> >     {
> >       memcpy (m, dev_map->addr + offset, align);
> 
> This is still copying a byte at a time, isn't it?

There is no general answer to this question. It depends on what the C library
and the compiler are making out of it. Thus it's also dependent on the
architecture and the value of align. 

But yes I need to confess that when looking into the glibc for example this code
snippet could be optimized to allow larger amounts of data to be passed to the
memcpy function so that it's up to the C library and compiler to optimize it the
best.

> 
> >       count -= align;
> >       offset += align;
> >       m += align;
> >     }
> >
> >   /* copy remaining unaligned bytes */
> >   while (count)
> >     {
> >       memcpy (m, dev_map->addr + offset, 1);
> >       count--;
> >       offset++;
> >       m++;
> >     }
> >
> >   return IOD_OK;
> > }
> >
> > The write function is quite similar implemented in the same manner.
> >
> > WDYT?
> >
> > Andreas

Best regards,

Andreas

signature.asc
Description: PGP signature