Page MenuHomeFreeBSD

Use gbincore_unlocked for unprotected incore()
ClosedPublic

Authored by cem on Jul 23 2020, 7:29 PM.

Details

Diff Detail

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

Event Timeline

cem requested review of this revision.Jul 23 2020, 7:29 PM
cem created this revision.

Hmm. There is one use of incore() which modifies the returned buf, in indir_trunc(). The comment right after states that some lock is held, but I can't see what it is. How does the update to the dependency list get synchronized?

This revision is now accepted and ready to land.Jul 24 2020, 2:20 PM

Hmm. There is one use of incore() which modifies the returned buf, in indir_trunc(). The comment right after states that some lock is held, but I can't see what it is. How does the update to the dependency list get synchronized?

I think they are synchronized under the global ump mutex in UFS, but I don't know. I don't think this revision changes that, though.

In D25790#571313, @cem wrote:

Hmm. There is one use of incore() which modifies the returned buf, in indir_trunc(). The comment right after states that some lock is held, but I can't see what it is. How does the update to the dependency list get synchronized?

I think they are synchronized under the global ump mutex in UFS, but I don't know.

The per-mountpoint softdep lock is not held there (it is acquired a few lines below), and I don't think the mountpoint lock is either.

I don't think this revision changes that, though.

Indeed, just trying to understand if there is a bug here.

In D25790#571313, @cem wrote:

I think they are synchronized under the global ump mutex in UFS, but I don't know.

The per-mountpoint softdep lock is not held there (it is acquired a few lines below), and I don't think the mountpoint lock is either.

Right; it looked like (in my quick scan) that we intentionally dropped the lock to run indir_trunc(). But my guess was/is any synchronization needed on the datastructure is performed under that lock before it is dropped. I don't really know SU that well.

This revision was automatically updated to reflect the committed changes.