fusefs: update atime on reads when using cached attributes
ClosedPublic
Actions

Authored by asomers on Nov 28 2021, 3:50 AM.

Details

Reviewers

Commits

rG4dd575e7d54d: fusefs: update atime on reads when using cached attributes
rG0bade34633f9: fusefs: update atime on reads when using cached attributes
rG91972cfcddf9: fusefs: update atime on reads when using cached attributes

Summary

When using cached attributes, whether or not the data cache is enabled,
fusefs must update a file's atime whenever it reads from it, so long as
it wasn't mounted with -o noatime. Update it in-kernel, and flush it to
the server on close or during the next setattr operation.

The downside is that close() will now frequently trigger a FUSE_SETATTR
upcall. But if you care about performance, you should be using
-o noatime anyway.

MFC after: 2 weeks

Test Plan

test cases added

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

asomers created this revision.Nov 28 2021, 3:50 AM

Herald added a subscriber: imp. · View Herald TranscriptNov 28 2021, 3:50 AM

asomers requested review of this revision.Nov 28 2021, 3:50 AM

Harbormaster completed remote builds in B43000: Diff 99093.Nov 28 2021, 3:50 AM

I will admit I don't like this. Does linux do the same?

In D33145#749045, @pfg wrote:

I will admit I don't like this. Does linux do the same?

It seems like Linux does something worse. On any read, it invalidates the atime field, so the next stat(2) call has to retrieve the file's status from the server rather than from cached attributes. For mtime and ctime, though, Linux updates those fields in-kernel just like FreeBSD already does.

pfg accepted this revision.Nov 29 2021, 12:24 AM

This revision is now accepted and ready to land.Nov 29 2021, 12:24 AM

Closed by commit rG91972cfcddf9: fusefs: update atime on reads when using cached attributes (authored by asomers). · Explain WhyNov 29 2021, 1:56 AM

This revision was automatically updated to reflect the committed changes.

asomers added a commit: rG91972cfcddf9: fusefs: update atime on reads when using cached attributes.

asomers added a commit: rG0bade34633f9: fusefs: update atime on reads when using cached attributes.Dec 14 2021, 10:24 PM

asomers added a commit: rG4dd575e7d54d: fusefs: update atime on reads when using cached attributes.Oct 19 2022, 3:20 AM

Revision Contents
Changeset List

Path

Size

sys/

fs/

fuse/

11 lines

3 lines

3 lines

12 lines

10 lines

tests/

sys/

fs/

fusefs/

1 line

1 line

3 lines

6 lines

296 lines

2 lines

2 lines

Diff 99126

View Options

sys/fs/fuse/fuse_internal.c

	.\" $FreeBSD$			.\" $FreeBSD$
	Set to not build and install the enhanced			Install the traditional FreeBSD
	.Xr bc 1			.Xr bc 1
				jhbUnsubmitted Not Done Inline Actions I would maybe avoid making other changes for this commit than just the "Set to" part. Losing the Xr sections for bc and dc seems rather odd. Granted, I would rewrite this description to be something like "Use the traditional FreeBSD versions of bc(1) and dc(1) rather than the <foo> versions." where <foo> is more descriptive than just "enhanced". jhb: I would maybe avoid making other changes for this commit than just the "Set to" part. Losing…
				emasteAuthorUnsubmitted Done Inline Actions Huh, I think this must have been an accident during a rebase, I did not intend to drop these. When I first did this it was a bit of sed with manual review/touch-ups. In can incorporate feedback before this gets committed and/or we can make further improvements like this one after commit. I am also not sure what <foo> would be off hand. emaste: Huh, I think this must have been an accident during a rebase, I did not intend to drop these.
	and			and
	.Xr dc 1			.Xr dc 1
	programs instead of the traditional FreeBSD versions.			programs instead of the enahnced versions.
				emasteAuthorUnsubmitted Done Inline Actions and of course I have a typo here. corrected locally. emaste: and of course I have a typo here. corrected locally.

View Options

sys/fs/fuse/fuse_io.c

	.\" $FreeBSD$			.\" $FreeBSD$
	Set to not build and install the enhanced			Install the traditional FreeBSD
	.Xr bc 1			.Xr bc 1
				jhbUnsubmitted Not Done Inline Actions I would maybe avoid making other changes for this commit than just the "Set to" part. Losing the Xr sections for bc and dc seems rather odd. Granted, I would rewrite this description to be something like "Use the traditional FreeBSD versions of bc(1) and dc(1) rather than the <foo> versions." where <foo> is more descriptive than just "enhanced". jhb: I would maybe avoid making other changes for this commit than just the "Set to" part. Losing…
				emasteAuthorUnsubmitted Done Inline Actions Huh, I think this must have been an accident during a rebase, I did not intend to drop these. When I first did this it was a bit of sed with manual review/touch-ups. In can incorporate feedback before this gets committed and/or we can make further improvements like this one after commit. I am also not sure what <foo> would be off hand. emaste: Huh, I think this must have been an accident during a rebase, I did not intend to drop these.
	and			and
	.Xr dc 1			.Xr dc 1
	programs instead of the traditional FreeBSD versions.			programs instead of the enahnced versions.
				emasteAuthorUnsubmitted Done Inline Actions and of course I have a typo here. corrected locally. emaste: and of course I have a typo here. corrected locally.

View Options

sys/fs/fuse/fuse_node.h

	.\" $FreeBSD$			.\" $FreeBSD$
	Set to not build and install the enhanced			Install the traditional FreeBSD
	.Xr bc 1			.Xr bc 1
				jhbUnsubmitted Not Done Inline Actions I would maybe avoid making other changes for this commit than just the "Set to" part. Losing the Xr sections for bc and dc seems rather odd. Granted, I would rewrite this description to be something like "Use the traditional FreeBSD versions of bc(1) and dc(1) rather than the <foo> versions." where <foo> is more descriptive than just "enhanced". jhb: I would maybe avoid making other changes for this commit than just the "Set to" part. Losing…
				emasteAuthorUnsubmitted Done Inline Actions Huh, I think this must have been an accident during a rebase, I did not intend to drop these. When I first did this it was a bit of sed with manual review/touch-ups. In can incorporate feedback before this gets committed and/or we can make further improvements like this one after commit. I am also not sure what <foo> would be off hand. emaste: Huh, I think this must have been an accident during a rebase, I did not intend to drop these.
	and			and
	.Xr dc 1			.Xr dc 1
	programs instead of the traditional FreeBSD versions.			programs instead of the enahnced versions.
				emasteAuthorUnsubmitted Done Inline Actions and of course I have a typo here. corrected locally. emaste: and of course I have a typo here. corrected locally.

View Options

sys/fs/fuse/fuse_node.c

	.\" $FreeBSD$			.\" $FreeBSD$
	Set to not build and install the enhanced			Install the traditional FreeBSD
	.Xr bc 1			.Xr bc 1
				jhbUnsubmitted Not Done Inline Actions I would maybe avoid making other changes for this commit than just the "Set to" part. Losing the Xr sections for bc and dc seems rather odd. Granted, I would rewrite this description to be something like "Use the traditional FreeBSD versions of bc(1) and dc(1) rather than the <foo> versions." where <foo> is more descriptive than just "enhanced". jhb: I would maybe avoid making other changes for this commit than just the "Set to" part. Losing…
				emasteAuthorUnsubmitted Done Inline Actions Huh, I think this must have been an accident during a rebase, I did not intend to drop these. When I first did this it was a bit of sed with manual review/touch-ups. In can incorporate feedback before this gets committed and/or we can make further improvements like this one after commit. I am also not sure what <foo> would be off hand. emaste: Huh, I think this must have been an accident during a rebase, I did not intend to drop these.
	and			and
	.Xr dc 1			.Xr dc 1
	programs instead of the traditional FreeBSD versions.			programs instead of the enahnced versions.
				emasteAuthorUnsubmitted Done Inline Actions and of course I have a typo here. corrected locally. emaste: and of course I have a typo here. corrected locally.