Project

General

Profile

Bug #5382

pvn_getpages handles lengths <= PAGESIZE just fine

Added by Josef Sipek almost 5 years ago. Updated over 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
kernel
Start date:
2014-12-02
Due date:
% Done:

100%

Estimated time:
Difficulty:
Bite-size
Tags:
needs-triage

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.

History

#1

Updated by Gordon Ross almost 5 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".

#2

Updated by Josef Sipek almost 5 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.

#3

Updated by Electric Monk over 4 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>

Also available in: Atom PDF