On Tue, Nov 21, 2023 at 11:56:49AM +0100, Geert Uytterhoeven wrote: > Hi Kent, > > On Fri, 10 Nov 2023, Kent Overstreet wrote: > > Previosuly, the transaction commit path would have to add keys to the > > btree write buffer as a separate operation, requiring additional global > > synchronization. > > > > This patch introduces a new journal entry type, which indicates that the > > keys need to be copied into the btree write buffer prior to being > > written out. We switch the journal entry type back to > > JSET_ENTRY_btree_keys prior to write, so this is not an on disk format > > change. > > > > Flushing the btree write buffer may require pulling keys out of journal > > entries yet to be written, and quiescing outstanding journal > > reservations; we previously added journal->buf_lock for synchronization > > with the journal write path. > > > > We also can't put strict bounds on the number of keys in the journal > > destined for the write buffer, which means we might overflow the size of > > the preallocated buffer and have to reallocate - this introduces a > > potentially fatal memory allocation failure. This is something we'll > > have to watch for, if it becomes an issue in practice we can do > > additional mitigation. > > > > The transaction commit path no longer has to explicitly check if the > > write buffer is full and wait on flushing; this is another performance > > optimization. Instead, when the btree write buffer is close to full we > > change the journal watermark, so that only reservations for journal > > reclaim are allowed. > > > > Signed-off-by: Kent Overstreet <[email protected]> > > Thanks for your patch, which is the closest match to commit > 9e5a6c7797b240f1 ("bcachefs: btree write buffer now slurps keys from > journal") in next-20231121. > > > fs/bcachefs/btree_write_buffer.c | 507 ++++++++++++++++--------- > > The committed version has lots of changes, including: > > ++ for (struct wb_key_ref *n = i + 1; n < min(i + 4, > &darray_top(wb->sorted)); n++) > ++ prefetch(&wb->flushing.keys.data[n->idx]); > > which requires the inclusion if <linux/prefetch.h>, as reported by > kernel test robot <[email protected]> for arc[1], and by > [email protected] for m68k.
It's fixed in the current version of my for-next branch (noticed when I imported the code into -tools).
