Bug #5382
closed
pvn_getpages handles lengths <= PAGESIZE just fine
100%
Description
The pvn_getpages helper function handles the case of length being less than or equal to PAGESIZE just fine. Many filesystems don't realize that, and therefore they call it on lengths greater than one page calling the their getapage helper directly at other times.
It turns out calling pvn_getpages all the times is either the same speed or marginally faster - on top of making the code a bit more readable.
Updated by Gordon Ross over 8 years ago
Changing all file systems to replace this pattern:
if (len <= PAGESIZE)
err = tmp_getapage(vp, (u_offset_t)off, len, protp, pl, plsz,
seg, addr, rw, cr);
else
err = pvn_getpages(tmp_getapage, vp, (u_offset_t)off, len,
protp, pl, plsz, seg, addr, rw, cr);
with this pattern:
err = pvn_getpages(tmp_getapage, vp, (u_offset_t)off, len,
protp, pl, plsz, seg, addr, rw, cr);
is an awful lot of "code churn" for questionable benefit.
Unless there are good arguments presented on why this is a "good trade", I'm inclined to say this is "not a bug".
Updated by Josef Sipek over 8 years ago
It's a cleanup. Given the pretty small amount of documentation of the VFS, anyone dealing with any filesystem more or less has to read what other filesystems do. This sort of conditional indirection hurts readability and maintainability.
I think it is a pretty well accepted idea that "unnecessarily hard to read code" is bad. redmine gives me (basically) two options: bug or feature. Between the two, "bug" seems more appropriate.
Updated by Electric Monk over 8 years ago
- Status changed from New to Closed
- % Done changed from 80 to 100
git commit 06e6833ac1f55fa31b2fc68fa6af8abfc2974d0b
commit 06e6833ac1f55fa31b2fc68fa6af8abfc2974d0b
Author: Josef 'Jeff' Sipek <josef.sipek@nexenta.com>
Date: 2015-02-26T19:30:02.000Z
5382 pvn_getpages handles lengths <= PAGESIZE just fine
Reviewed by: Marcel Telka <marcel@telka.sk>
Approved by: Dan McDonald <danmcd@omniti.com>