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

[Andreas Klinger]

Hi Mohammad,

Mohammad-Reza Nabipoor <mnabipoor@gnu.org> schrieb am Sa, 06. Jan 16:23:
> On Sat, Jan 06, 2024 at 03:40:15PM +0100, Mohammad-Reza Nabipoor wrote:
> > 
> > > +  if (align > 8)
> > > +    align = 8;
> > > +
> > > +  if (align > 1 && offset % align)
> > 
> > 
> > Why not returning IOD_EIOFF for "Invalid offset"? Or some other error?
> > If user is asking for an invalid range of memory, we have to return
> > an error, rather than trying to "fix" the problem.  It prevents mistakes.
> > 
> > Or we can handle the alignment like how some processors handle?
> >   For N byte read, you have to be aligned on N-byte boundaries.

This was also an idea I already implemented. It turned out that when using a
pickle there are reads on unaligned adresses. This happened in my case with jpeg
markers. To give a simplified example:

Pickle:
type MY_Tag = 
  struct
  {
    byte b;
    uint16 u;
  };

poke:
(poke) var m = MY_Tag @ 0x0#B

Will run into this error when trying to read 2 bytes at offset 1 which in turn
means someone is not able to use this struct in the pickle. Because of this
simply returning an error is not an option for my use case and I think there are
more use cases like this.

> Maybe we can relax alignment requirement using a flag.

The truth is that this is stronly dependent on the hardware architecture. We
run hardly into these kind of issues on x86 but we have to struggle with it on
arm every now and then. When someone is only accessing files and no memory
mapped io then this is also not an issue. So it depends on architecture features
and use case.

In this v2 patch I tried to implement a one fits all solution by setting the
alignment to 8 bytes. But this is of course not the optimum for all cases.

An option could be to introduce a new .set variable to set the requested
alignment. Then the user can decide if alignment matters and when yes which one
he or she wants to use. For example:

On arm 32 bit systems:
.set alignment 4

on arm 64 bit:
.set alignment 8

when alignment is no issue:
.set alignment 1

Default value could be sizeof(void *).

Comments?

Andreas

signature.asc
Description: PGP signature