posix: atime update improvements

[marcinslusarz]

Builds on top of #393

---

This change is 

[codecov[bot]]

Codecov Report

Merging #402 into master will increase coverage by 0.12%. The diff coverage is 90.19%.

@@            Coverage Diff             @@
##           master     #402      +/-   ##
==========================================
+ Coverage   77.09%   77.22%   +0.12%     
==========================================
  Files          67       67              
  Lines        8493     8526      +33     
  Branches     1698     1705       +7     
==========================================
+ Hits         6548     6584      +36     
+ Misses       1515     1512       -3     
  Partials      430      430

Flag	Coverage Δ
#ltp_tests	47.49% <62.74%> (+0.04%)	:arrow_up:
#sqlite_tests	45.99% <74.5%> (+0.06%)	:arrow_up:
#tests_antool	14.91% <0%> (-0.06%)	:arrow_down:
#tests_posix_multi_threaded	26.11% <45.09%> (+0.05%)	:arrow_up:
#tests_posix_single_threaded	54.74% <76.47%> (+0.1%)	:arrow_up:
#tests_preload	45.37% <72.54%> (+0.06%)	:arrow_up:

Impacted Files	Coverage Δ
src/libpmemfile-posix/inode.h	100% <100%> (ø)	:arrow_up:
src/libpmemfile-posix/stat.c	93.87% <100%> (ø)	:arrow_up:
src/libpmemfile-posix/read.c	95.13% <100%> (+0.54%)	:arrow_up:
src/libpmemfile-posix/inode.c	89.21% <100%> (+0.89%)	:arrow_up:
src/libpmemfile-posix/timestamps.c	91.3% <80%> (-0.31%)	:arrow_down:
src/libpmemfile-posix/write.c	86.38% <81.25%> (-0.19%)	:arrow_down:
src/libpmemfile/path_resolve.c	89.88% <0%> (-0.6%)	:arrow_down:
src/libpmemfile-posix/data.c	94.5% <0%> (-0.4%)	:arrow_down:
... and 2 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 44cebf7...5f73a3d. Read the comment docs.

[sarahjelinek]

Reviewed 15 of 34 files at r1, 16 of 19 files at r2. Review status: 31 of 34 files reviewed at latest revision, 4 unresolved discussions.

---

src/libpmemfile-posix/inode.c, line 811 at r2 (raw file):

  if (vinode->atime_dirty)
      /* we did it at suspend time */
      vinode->atime_dirty = false;

Why don't you clear the atime_dirty at suspend?

---

src/libpmemfile-posix/inode.h, line 242 at r2 (raw file):

static inline void
pmemfile_tx_time_set(struct pmemfile_time *time, struct pmemfile_time tm)

since the other functions are set_xxxtime() this one should be set_time()? Just for consistency? Unless you named this slightly different for a reason.

---

src/libpmemfile-posix/layout.h, line 145 at r2 (raw file):

#define PMEMFILE_INODE_SIZE METADATA_BLOCK_SIZE
#define PMEMFILE_IN_INODE_STORAGE \
  (sizeof(struct pmemfile_dir) + 2 * sizeof(struct pmemfile_dirent) + 8)

I am not sure I understand why you made this change?

---

Comments from Reviewable

[marcinslusarz]

Do you want me to rebase it to master to get rid of commits from #393 ?

---

Review status: 31 of 34 files reviewed at latest revision, 4 unresolved discussions.

---

src/libpmemfile-posix/inode.c, line 811 at r2 (raw file):

Previously, sarahjelinek (Sarah Jelinek) wrote…

Why don't you clear the atime_dirty at suspend?

Because suspend is in transaction - if it fails it means atime was not written back to medium.

---

src/libpmemfile-posix/inode.h, line 242 at r2 (raw file):

Previously, sarahjelinek (Sarah Jelinek) wrote…

since the other functions are set_xxxtime() this one should be set_time()? Just for consistency? Unless you named this slightly different for a reason.

It's part of #393 this PR builds on.

Other functions are named inode_tx_set_xtime because they operate on an inode, do it in transaction and they set xtime.

This function operates on pmemfile_time, in transaction and sets its value, so it should be either pmemfile_time_tx_set or time_tx_set. I added one commit to rename it to latter one.

---

src/libpmemfile-posix/layout.h, line 145 at r2 (raw file):

Previously, sarahjelinek (Sarah Jelinek) wrote…

I am not sure I understand why you made this change?

It's part of #393 (specifically commit 953ebd3efeea261c576544bdd6c94a5c3e308394).

I did it because I aligned the beginning of pmemfile_inode.file_data to cacheline boundary and wanted to use the remaining 8 bytes hole for short symlink information.

---

Comments from Reviewable

[sarahjelinek]

Yes. thanks.

---

Review status: 31 of 34 files reviewed at latest revision, 1 unresolved discussion.

---

Comments from Reviewable

[GBuella]

Reviewed 1 of 19 files at r2. Review status: 32 of 34 files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[sarahjelinek]

---

Review status: 5 of 6 files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable