<div dir="ltr"><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">Mr. Verma,<br><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">I am used to doing all of this strictly in DRAM, so my optimization background has differing overhead points.<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jun 22, 2015 at 2:25 PM, Verma, Vishal L <span dir="ltr"><<a href="mailto:vishal.l.verma@intel.com" target="_blank">vishal.l.verma@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Mon, 2015-06-22 at 09:50 -0700, Doug Dumitru wrote:<br> > I would like to comment on the BTT docs a bit. There are some design<br> > points you might want to consider.<br> ><br> </span>Hi Doug,<br> Thanks for the design review -<br> <span class="">><br> > First, real use cases will have no read/write collisions. If you<br> > think of a file system, the case of reading a block that is being<br> > written or writing a single block twice just don't happen because the<br> > data itself is non deterministic. The driver still needs to handle<br> > these cases, but optimizing it for this is not all that logical.<br> ><br> </span>Agreed - we did try to keep this in mind, and I still have a list of<br> experiments to try that might further reduce the impact of collision<br> handling on normal IO.<br> <span class="">><br> > Lets start at the BTT table. It would be useful if we could<br> > distinguish between a stable block and one that is getting updated<br> > now. An option is to encode the TRIM/ERROR bits as four "states"<br> > (stable, updating, trimmed, error) and just use the BTT entry as an<br> > index. This could probably point directly to the FLOG entry. The BTT<br> > table, at four bytes, has atomic updates without locks, so two threads<br> > can simultaneously update it to point to the FLOG table and then after<br> > the update, see if they won. If they did not win, they can wait for<br> > the first update to complete. The FLOG table could also have a<br> > parallel RAM based BTT2 table to store spinlocks or linked-lists to<br> > handle collisions. Then again, a simple spin or spin/sleep is<br> > probably good enough.<br> <br> </span>So first off, this made me realize the documentation was slightly dated<br> w.r.t. the use of the 'E' and 'Z' flags - we currently (and the code<br> reflects this) do use all four states from those two bits. (The next<br> posting of this patchset should reflect this change).<br> <br> Bit Description<br> 31 - 30 : Error and Zero flags - Used in the following way:<br> Bit Description<br> 31 30<br> -----------------------------------------------------------------------<br> 00 Initial state. Reads return zeroes; Premap = Postmap<br> 01 Zero state: Reads return zeroes<br> 10 Error state: Reads fail; Writes clear 'E' bit<br> 11 Normal Block – has valid postmap<br> <br> 29 - 0 : Mappings to internal 'postmap' blocks<br> <br> Apart from this, I didn't quite understand what you meant by:<br> > ... and just use the BTT entry as an index. This could probably point<br> <span class="">directly to the FLOG entry.<br> <br> </span>Care to elaborate with an example maybe?<br></blockquote><div><br><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small;display:inline">I am not sure how "committed" to binary region sizes here, but the first thing that sticks out is that all but the normal state don't really need an index entry. If you are willing to suffer some "non binary divide" operations, you can increase the efficiency of your region by using these 4 bytes as:<br><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small;display:inline">0 - Initial state<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small;display:inline">1 - Zero/trim/discard block, but why not just '0' above<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small;display:inline">2 - Error<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small;display:inline">3-F - Future flags<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small;display:inline">10-FFFF - index into FLOG table<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small;display:inline">10000-FFFFFFFF - index directly into RAM store<br><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">This pushes the region to just under 4B entries (2TB with 512 and 16TB with 4K). If you must stay binary, consider a single bit.<br><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">0 - Initial state<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">1 - Zero<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">2 - Error<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">10-FFFF - index into FLOW table<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">8FFFFFFFF - FFFFFFFF - index directly into RAM store<br></div><br><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small;display:inline">The FLOG table can have pre/post RAM store index (per your current design), so a read on a write has to do a double lookup, but that might be good anyway, and the double lookup is only on a collision anyway.<br><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small;display:inline">In terms of read overlaps, you could argue you can do this without a lock. If you read a BTT entry and it is 1000-FFFFFFFF, and you deterministically control how often a block gets re-used, it is safe to just read even if an update starts half-way through. If you want to be paranoid, setup an atomic for updates that the read can use to protect itself.<br><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small;display:inline">read update ATOMIC<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small;display:inline">read BTT<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small;display:inline">if BTT is not an index into the FLOG table then read data<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small;display:inline">read update ATOMIC again<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small;display:inline">If ATOMIC has not moved more than the guaranteed blocks over commit number, you know the read is valid data and the block has not been overwritten.<br><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small;display:inline">The write logic just increments the ATOMIC (and lets it wrap around) whenever a new block is used. As long as the avail block first-in first-out list is slightly bigger than the advertised device capacity, you can guarantee "N" updates before a read gets the rug pulled out from under it.<br><br></div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <span class="">><br> ><br> > The same works for readers. If you read a block, check the BTT table<br> > after you finish the read. If it is the same, your read was good. If<br> > it changed underneath you, or is pointing to a FLOG block, then you<br> > need to wait or re-read. Again, the real-world frequency of<br> > collisions is very low. This would let you eliminate the RTT table<br> > entirely.<br> <br> </span>There are a couple of issues I see with this -<br> 1. This could, theoretically, suffer from the A-B-A problem - a writer<br> could update the same mapping twice, ending up with an apparently<br> untouched map entry, but the data has gone through churn while the<br> reader was still reading it.<br> 2. Writing/reading the RTT is cheap as it is entirely in DRAM memory,<br> whereas another check on the map would mean NVDIMM IO, which could<br> potentially be slow enough to offset any benefit from getting rid of the<br> RTT.<br></blockquote><div><br><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small;display:inline">If the API read is expensive, then you are probably right. Consider an ATOMIC DRAM counter as described above. This requires you to keep a small number of blocks "spare" so that reads complete before they can get clobbered, but this lets reads run lock free.</div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br> One of the pending tests I plan to do is to make the RTT a hash map<br> instead of a list. i.e. Reader adds a refcount to rtt[postmap ABA], and<br> writer checks just RTT[free-block]. This keeps things the same for the<br> reader, but for writers, they are sapred from walking a list, at the<br> cost of false hash collisions. Obviously, with this path we'd have to<br> make the RTT a lot bigger than it currently is - and we'd need to figure<br> out if the increased memory footprint, and hash collisions outweigh the<br> cost of walking an in-memory list..<br></blockquote><div><br><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small;display:inline">If the BTT is a pointer to the FLOG, I think the RTT can go away entirely.</div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <span class="">><br> ><br> > One final optimization would be to keep the BTT table both in standard<br> > RAM as well as in NV RAM. If standard RAM is faster, then reads could<br> > lookup blocks without touching the NV driver. For 512G, this is 1B<br> > blocks or 4G of RAM. Then again, if the NV RAM is just as fast, this<br> > would not help. Perhaps an option.<br> <br> </span>We've thought about this, and while it could help, the added complexity<br> of keeping the two in sync may make it debatable - we'll have to<br> re-evaluate once NVDIMMs become more easily available :)<br></blockquote><div><br><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small;display:inline">It should just be a double update of the same four bytes to both places. Update the RAM last but only ACK the write after both are written and memory fenced. If a read comes along and gets the RAM image out of sync with the NV image, you are reading "during" a write so getting the previous data is still "correct"</div> <div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small;display:inline">as you started the read before the write ACKd.<br></div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="HOEnZb"><div class="h5">><br> ><br> > I have gotten into a lot of trouble optimizing for fio collisions when<br> > these collisions don't really impact real-workload performance. The<br> > code has to be "correct" in the collision case, but it does not really<br> > need to be fast.<br> ><br> ><br> ><br> > Doug Dumitru<br> ><br> > EasyCo LLC<br> ><br> ><br> ><br> ><br> ><br> > On Fri, Jun 19, 2015 at 9:50 AM, Verma, Vishal L<br> > <<a href="mailto:vishal.l.verma@intel.com">vishal.l.verma@intel.com</a>> wrote:<br> > On Fri, 2015-06-19 at 12:33 -0400, Mikulas Patocka wrote:<br> > > Hi<br> > ><br> > > I looked at the new the persistent memory block device<br> > driver<br> > > (drivers/block/pmem.c and arch/x86/kernel/pmem.c) and it<br> > seems that the<br> > > interface between them is incorrect.<br> > ><br> > > If I want to use persistent memory in another driver, for a<br> > different<br> > > purpose, how can I make sure that that drivers/block/pmem.c<br> > doesn't attach<br> > > to this piece of memory and export it? It seems not<br> > possible.<br> > > drivers/block/pmem.c attaches to everything without regard<br> > that there may<br> > > be other users of persistent memory.<br> > ><br> > > I think a correct solution would be to add a partition table<br> > at the<br> > > beginning of persistent memory area and this partition table<br> > would<br> > > describe which parts belong to which programs - so that<br> > different programs<br> > > could use persistent memory and not step over each other's<br> > data. Is there<br> > > some effort to standardize the partition table ongoing?<br> > ><br> > ><br> > > BTW. some journaling filesystems assume that 512-byte sector<br> > is written<br> > > atomically. drivers/block/pmem.c breaks this requirement.<br> > Persistent<br> > > memory only gurantees 8-byte atomic writes.<br> ><br> ><br> > Hi Mikulas,<br> ><br> > I can answer this part - The idea is that file systems that<br> > need sector<br> > atomicity will use the "Block Translation Table" (BTT) [1]. It<br> > would be<br> > a stacked block device on top of a pmem device (or partition),<br> > and file<br> > systems would be able to use it either for the entire space to<br> > get<br> > atomicity for all blocks, or if they want to use DAX, make two<br> > partitions, and enable the BTT only on one partition, and use<br> > it as the<br> > logdev.<br> ><br> > -Vishal<br> ><br> > [1]: <a href="https://lkml.org/lkml/2015/6/17/950" rel="noreferrer" target="_blank">https://lkml.org/lkml/2015/6/17/950</a><br> ><br> > ><br> > > Mikulas<br> > > _______________________________________________<br> > > Linux-nvdimm mailing list<br> > > <a href="mailto:Linux-nvdimm@lists.01.org">Linux-nvdimm@lists.01.org</a><br> > > <a href="https://lists.01.org/mailman/listinfo/linux-nvdimm" rel="noreferrer" target="_blank">https://lists.01.org/mailman/listinfo/linux-nvdimm</a><br> ><br> ><br> > --<br> > dm-devel mailing list<br> > <a href="mailto:dm-devel@redhat.com">dm-devel@redhat.com</a><br> > <a href="https://www.redhat.com/mailman/listinfo/dm-devel" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/dm-devel</a><br> ><br> ><br> ><br> ><br> > --<br> > Doug Dumitru<br> > EasyCo LLC<br> ><br> > _______________________________________________<br> > Linux-nvdimm mailing list<br> > <a href="mailto:Linux-nvdimm@lists.01.org">Linux-nvdimm@lists.01.org</a><br> > <a href="https://lists.01.org/mailman/listinfo/linux-nvdimm" rel="noreferrer" target="_blank">https://lists.01.org/mailman/listinfo/linux-nvdimm</a><br> <br> </div></div></blockquote></div><br><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">My comment on block sizes was only 512/4K, but the code should be easy to setup at anything binary.<br></div><br clear="all"><br>-- <br><div class="gmail_signature">Doug Dumitru<br>EasyCo LLC<br></div> </div></div>