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.
Tags
None
Referenced Files
F80132744: D27353.diff
Thu, Mar 28, 8:37 AM
Unknown Object (File)
Feb 8 2024, 7:50 PM
Unknown Object (File)
Jan 30 2024, 7:20 AM
Unknown Object (File)
Jan 1 2024, 6:19 AM
Unknown Object (File)
Jan 1 2024, 4:17 AM
Unknown Object (File)
Dec 20 2023, 5:45 AM
Unknown Object (File)
Dec 11 2023, 2:18 AM
Unknown Object (File)
Oct 10 2023, 8:49 PM
Subscribers

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
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 35003

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

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

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