how many times should read before partial write

[Finix1979]

I recently found that if I re-write a file by a small IO(< recordsize) whose primarcache is set to metadata, ZFS will read the record from disk two times in zfs_write routine. I understand RMW is necessary but it could be better if just read once.

The process is simple: (zfs 2.1.7 the newest version, ubuntu22.04)

1. zfs create p1/idx -o recordsize=8K -o primarycache=metadata
   dd if=/dev/urandom of=/p1/idx/a bs=8K count=1024
   zpool sync
2. re-write file
   dd if=/dev/urandom of=/p1/idx/a bs=32 count=1 conv=notrunc
3. another terminal trace arc_read function and I get
   1302504 1302504 dd arc_read
   b'arc_read+0x1 [zfs]'
   b'dbuf_read+0x103 [zfs]'
   b'dmu_tx_check_ioerr+0x6e [zfs]'
   b'dmu_tx_count_write+0x1af [zfs]'
   b'dmu_tx_hold_write_by_dnode+0x66 [zfs]'
   b'zfs_write+0x423 [zfs]'
   b'zpl_iter_write+0xf3 [zfs]'
   b'new_sync_write+0x111 [kernel]'
   b'vfs_write+0x1d5 [kernel]'
   b'ksys_write+0x67 [kernel]'
   b'__x64_sys_write+0x19 [kernel]'
   b'do_syscall_64+0x59 [kernel]'
   b'entry_SYSCALL_64_after_hwframe+0x61 [kernel]'

1302504 1302504 dd arc_read
b'arc_read+0x1 [zfs]'
b'dbuf_read+0x103 [zfs]'
b'dmu_buf_will_dirty_impl+0xeb [zfs]'
b'dmu_buf_will_dirty+0x16 [zfs]'
b'dmu_write_uio_dnode+0x85 [zfs]'
b'dmu_write_uio_dbuf+0x4f [zfs]'
b'zfs_write+0x8be [zfs]'
b'zpl_iter_write+0xf3 [zfs]'
b'new_sync_write+0x111 [kernel]'
b'vfs_write+0x1d5 [kernel]'
b'ksys_write+0x67 [kernel]'
b'__x64_sys_write+0x19 [kernel]'
b'do_syscall_64+0x59 [kernel]'
b'entry_SYSCALL_64_after_hwframe+0x61 [kernel]'

In terms of the trace, the record to be written is read by both dmu_tx_check_ioerr and dmu_buf_will_dirty. Because I set primarycache to metadata, the dbuf and its arc are destroyed after dmu_tx_check_ioerr. That's why dmu_buf_will_dirty could not find dbuf and create a new one.

Is there any possibility to reduce this double read even when metadata is set as primarycache? Thanks.

[amotin]

I think it should be fixable. My #14243 PR already improves short read and prefetch cases with primarycache=metadata. May be some alike logic could be applied for writes too. We could make those reads in dmu_tx_check_ioerr() to look like a prescient prefetch, which they really are, so they could be legally cached at very least by ARC.

But I think this problem should happen only once per block write in TXG. If you do another write to the same block while the first haven't completed, it should not cause any reads.

[Finix1979]

But I think this problem should happen only once per block write in TXG. If you do another write to the same block while the first haven't completed, it should not cause any reads.

@amotin Thank you. Yes, if another write happened in the same TXG, it could find existing dbuf and re-dirty it I think.

I run #14243 and could confirm it works for my case. The dmu_tx_check_ioerr will call dbuf_read with DB_RF_PARTIAL_FIRST flag and it then sets db_partial_read, which later gives dbuf_rele_and_unlock a "hint" to not destroy dbuf. In the near future, dmu_buf_will_dirty could then find the existing cached dbuf and reuse it without firing IO. Double read is solved.

BTW, given that a new type of dbuf(uncached) has a chance to hold ARC and stay in dbuf cache list, will you suggest increasing dbuf high water percentage(dbuf_cache_hiwater_pct) to avoid evicting? It seems that this kind of dbuf is short-time living, but I'm not quite sure.

Thank you and happy new year.

[amotin]

BTW, given that a new type of dbuf(uncached) has a chance to hold ARC and stay in dbuf cache list, will you suggest increasing dbuf high water percentage(dbuf_cache_hiwater_pct) to avoid evicting? It seems that this kind of dbuf is short-time living, but I'm not quite sure.

I am not sure I am getting your question. ARC for uncached state does not evict for at least a second after the last access. Reference held by dbuf cache may extend that further, since ARC can not free a block still having references. So increase of dbuf cache size will increase caching of uncached buffers, but I was not thinking about increased dbuf cache size. It is not designed to be big. My idea behind evicting uncacheable buffers as soon as they are read in full was to actually reduce eviction of buffers that have more rights to be cached in the limited dbuf cache space.

[amotin]

Thinking about size, with primarycache=metadata your ARC may stay pretty small. And since dbuf cache is 1/32 of ARC size, it may become extremely small. It may be work-arounded by explicitly setting arc_min a bit higher to help system better scale on boot until enough metadata read into ARC.

[amotin]

I think small addition I've made to #14243 should fix the issue. @Finix1979, you are welcome to test it. That PR should give you huge improvements on many fronts if you are running with primarycache=metadata.