Page MenuHomeFreeBSD

ffs: do not read full direct blocks if they are going to be overwritten.
ClosedPublic

Authored by kib on Nov 24 2020, 5:28 PM.

Details

Summary

BA_CLRBUF specifies that existing context of the block will be completely overwritten by caller, so there is no reason to spend io fetching existing data. We do the same for indirect blocks.

Reported by: tmunro

Test Plan

Not tested.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

kib requested review of this revision.Nov 24 2020, 5:28 PM
kib retitled this revision from ffs: do not read direct blocks if they are going to be overwritten. to ffs: do not read full direct blocks if they are going to be overwritten..Nov 24 2020, 5:33 PM

Hmm. My filesystem has 32kb blocks, and this generates reads, according to dwatch -X io:

openat(AT_FDCWD,"file",O_RDWR|O_FSYNC|O_DIRECT,00) = 3 (0x3)
pwrite(3,"................................"...,32768,0x0) = 32768 (0x8000)
pwrite(3,"................................"...,32768,0x0) = 32768 (0x8000)
pwrite(3,"................................"...,32768,0x0) = 32768 (0x8000)

... while smaller write sizes don't. Isn't that backwards?

The sentiment is correct, but logic fixes noted are needed.

sys/ufs/ffs/ffs_balloc.c
175 ↗(On Diff #79937)

This is backwards. It should be "if (flags & BA_CLRBUF) != 0)".

BA_CLRBUF is set when not all of the buffer will be written so we need to clear it so that the unwritten part will be zero. Here where the block is already allocated, we need to read it when not all of it will be written and can getblk it when BA_CLRBUF is cleared because we will write all of it.

779 ↗(On Diff #79937)

Same comment here: This is backwards. It should be "if (flags & BA_CLRBUF) != 0)".

This revision now requires changes to proceed.Nov 25 2020, 4:39 AM
This revision is now accepted and ready to land.Nov 29 2020, 10:12 PM