Hi Alan,
Thanks for looking at this.
On 22/04/2019 16:52, Alan Bateman wrote:
> The calculation to compute the page aligned address and then the
> distance from there to the end of the region is tricky to get right. I
> think you have it right, I'm just wondering if we could leverage the
> existing mappingOffset/mappingAddress methods so that we have only one
> place to audit. For example, suppose mappingOffset is changed to take an
> index. I think this would reduce the new force method down to:
>
> long offset = mappingOffset(index);
> force0(fd, mappingAddress(offset), offset + length);
I like this idea but I'd like to make sure I am clear about exactly how
it ought to work. If you will indulge me I will work through this by
describing first the status quo (as I perceive it) and then what my
change attempts before trying to reconcile the two.
Currently:
There are 4 key locations of interest in the mapped byte range
[... map_base ... address ... address+limit ... address+capacity ...]
| |
+----- page size -----+
where:
- addresses to the left are numerically (i.e. as longs) lower than
those to the right
- map_base is aligned to page size -- it is obtained by rounding down
address to a page boundary
- consequently (address - map_base) is less than page size
n.b. the mmapped region may or may not commence at map_base but it will
always include map_base. Also, it will include bytes at all subsequent
addresses up to (address + capacity). Whether it includes or extends
beyond (address + capacity) is not really important.
mappingOffset() computes (address - map_base):
int ps = Bits.pageSize();
long offset = address % ps;
return (offset >= 0) ? offset : (ps + offset);
The jiggery-pokery with ? : works round the rather unhelpful behaviour
of mod wrt negative longs.
I think it could, equivalently, compute it directly as:
int ps = Bits.pageSize();
long map_base = (address & ~(ps - 1));
return address - map_base;
mappingAddress(mappingOffset) converts a previously retrieved
mappingOffset back to the map base:
return (address - mappingOffset)
== (address - (address - map_base))
== map_base
mappingLength(mappingOffset) computes ((address + capacity) - map_base)
i.e. the byte count from start of page map_base to the last
(potentially) /writeable/ buffer byte.
return capacity() + mappingOffset
== capacity + (address - map_base)
== (address + capacity) - map_base
arguably this method could just compute ((address + length) - map_base)
i.e. the byte count from map_base to the last /written/ byte -- but that
makes no real difference when performing a file-based flush.
The call to force0 is made as
force0(fd, mappingAddress(offset), mappingLength(offset))
which is equivalent to
force0(fd, map_base, capacity + (address - map_base))
My updated code:
When looking at an indexed location there are now 7 key locations of
interest in the mapped byte range
[... m_base ... a ... i_base ... a+i ... a+i+ln ... a+lim, a+cap ...]
| | | |
+- page size -+ +- page size -+
where:
- for brevity, map_base is abbreviated to m_base, address to a,
index_base to i_base, index to i, length to ln, limit to lim
and capacity to cap
- index is identifies the start of the region to be flushed
- length is the length of the region to be flushed
- (address + index) is the address of the byte at offset index
- index_base is the largest page aligned address below (address + idx)
i.e. it is obtained by rounding down the first byte of the flush
region to a page boundary
index_base is computed as follows
int ps = Bits.pageSize();
long index_base = ((address + index) & ~(ps - 1));
My code is calling
force0(fd, index_base, length + ((address + index) - i_base))
i.e. it the flushed range starts at the nearest page boundary below or
equal to (address + index) and includes the original length bytes plus
the intervening bytes between that page boundary and address.
When this is considered by analogy with the previous call then three
things stand out:
1) in place of map_base for the first argument we have index_base
2) in place of the extra included intervening bytes between map_base
and address we have the extra included intervening bytes between
index_base and address + index
3) in place of capacity (default) demarcated bytes to be flushed we
have length user demarcated bytes to be flushed
So, to retain those parallels detailed in 1, 2 and 3 we need
mappingOffset and mappingAddress to be overloaded and the originals
rerouted as follows
mappingOffset(index)
{
int ps = Bits.pageSize();
long index_address = address + index;
long index_base = (index_address & ~(ps - 1));
return index_address - index_base;
}
mappingOffset() => mappingOffset(0)
mappingAddress(mappingOffset, index)
{
long index_address = address + index;
return index_address - mappingOffset
== (index_address - (index_address - index_base))
== index_base
}
mappingAddress(mappingOffset) => mappingAddress(mappingOffset, 0)
mappingLength(mappingOffset, length)
{
return length + mappingOffset
}
mappingLength(mappingOffset) =>
mappingLength(mappingOffset, (long)capacity())
Does that look correct? If so I will prepare a new webrev accordingly.
> I don't expect any issues on Windows but I will test it to make sure.
Thank you. That would be very helpful.
regards,
Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
