Bug #2056

HAMMER does not update file access time after read

Added by marino almost 3 years ago. Updated over 1 year ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:dillon% Done:

0%

Category:-
Target version:-

Description

During the development of the upgrade of grep to version 2.7, it was observed
that the HAVE_WORKING_O_NOATIME value was defined as 0 for the i386 platform and
1 for the x86_64 platform. The i386 platform filesystem was UFS and the other
was configured with Hammer.

The grep configuration script uses standard gnu tests. The one used to derive
the O_NOATIME value is located at
http://leaf.dragonflybsd.org/~marino/fctnl_check.c

After some testing, swildner confirmed that Hammer doesn't update the file
access time after data (more than 0 bytes) is read from a file. He believes
that this behavior is not simply a convention, but covered by a standard. Since
Hammer isn't exhibiting the expected behavior, it should be considered a bug.

Moreover, every gnu configure script that is run on a system with Hammer will
produce the wrong value for O_NOATIME support, and thus may cause unexpected
behavior when the software compiles.

History

#1 Updated by swildner almost 3 years ago

It actually is in the standard, I just didn't find it directly:

http://pubs.opengroup.org/onlinepubs/9699919799/functions/read.html

"Upon successful completion, where nbyte is greater than 0, read() shall mark
for update the last data access timestamp of the file, and shall return the
number of bytes read."

S.

#2 Updated by alexh almost 3 years ago

I'll take a look at implementing this, it should be pretty straight forward.

Cheers,
Alex

#3 Updated by alexh almost 3 years ago

The code is in hammer_vop_read:

/*
* XXX only update the atime if we had to get the MP lock.
* XXX hack hack hack, fixme.
*/
if (got_fstoken) {
if ((ip->flags & HAMMER_INODE_RO) == 0 &&
(ip->hmp->mp->mnt_flag & MNT_NOATIME) == 0) {
ip->ino_data.atime = trans.time;
hammer_modify_inode(&trans, ip, HAMMER_INODE_ATIME);
}
hammer_done_transaction(&trans);
lwkt_reltoken(&hmp->fs_token);
}
return (error);

It seems that the atime is only updated sometimes. This is horribly inconsistent
behaviour. Matt needs to look at this one, too. I've not the slightest clue why
such a hackish behaviour.

Regards,
Alex

#4 Updated by dillon almost 3 years ago

I have committed a change to master to hopefully make atime updates more
consistent. The code that was in there was an attempt to optimize the
concurrency path for read()'s for the case where the buffer cache buffers are
already cached.

Please test.

-Matt

#5 Updated by marino over 1 year ago

  • Status changed from New to Closed

Both hammer and UFS give the same result given the provided test (as would be expected).

Also available in: Atom PDF