Page MenuHomeFreeBSD

Simplify devfs_fsync().
ClosedPublic

Authored by trasz on Feb 16 2017, 11:02 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, May 8, 3:59 PM
Unknown Object (File)
Wed, May 8, 2:55 PM
Unknown Object (File)
Mar 17 2024, 11:36 AM
Unknown Object (File)
Dec 20 2023, 7:39 AM
Unknown Object (File)
Sep 15 2023, 11:07 AM
Unknown Object (File)
Sep 13 2023, 5:29 PM
Unknown Object (File)
Sep 7 2023, 8:13 PM
Unknown Object (File)
Sep 7 2023, 8:13 PM
Subscribers

Details

Summary

Simplify devfs_fsync(), by removing it. This might also be a minor
optimization, as vn_isdisk() needs to lock a global mutex.

Tested by: pho

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

trasz retitled this revision from to Simplify devfs_fsync()..
trasz updated this object.
trasz edited the test plan for this revision. (Show Details)

If doing this, you can simply point vop_fsync member of devfs_specops to vfs_stdfsync, without a need to create the trivial local wrapper.

That said, I believe that the printouts done by the current variant are useful, and the panic is justified until UFS is fixed.

The panic was changed into printf already, as it triggered every time, even when UFS or msdosfs could survive just fine. The warning - I kind of agree, it could be useful, but I don't think it's worth complicating the code just for that.

Also, good point with the vector. I'll fix that.

trasz edited edge metadata.

Remove the pointless wrapper.

trasz updated this object.
trasz updated this object.
imp edited edge metadata.

vop_stdfsync() handles these cases just fine, though the synchronous waiting code looks a bit odd since it only looks at the last bp in the bufobj's dirty chain for the error code, which seems like it would be wrong.
eg

TAILQ_FOREACH(bp, &bo->bo_dirty.bv_hd, b_bobufs)
        if ((error = bp->b_error) == 0)
                continue;

should be

TAILQ_FOREACH(bp, &bo->bo_dirty.bv_hd, b_bobufs)
        if ((error = bp->b_error) != 0)
                break;

but that's I guess unrelated.

This revision is now accepted and ready to land.Feb 19 2017, 5:45 PM
This revision was automatically updated to reflect the committed changes.