posix: use allocation classes

[igchor]

This change is 

[GBuella]

Reviewed 1 of 29 files at r1. Review status: 1 of 29 files reviewed at latest revision, 1 unresolved discussion, some commit checks broke.

---

src/libpmemfile-posix/blocks.c, line 61 at r1 (raw file):

struct pobj_alloc_class_desc pmemfile_posix_block =
  { 0, 1000, POBJ_HEADER_NONE, CONST_BLOCK_ID};

If I interpret it right, this is the new way to allow us to set a fixed block size, right? Just an idea, this could be a bit simpler I think:

The data_blocks array can be a null terminated array (e.g. the last element has unit_size==0) If a user sets fix block size using an environment variable, then we just need to update the first element of this array, and make the second element be the terminator.

So we would need to look at only the data_blocks array, instead of checking pmemfile_posix_block each time. Something like:

for (struct pobj_alloc_class_desc *desc = data_blocks; desc->unit_size !=0; ++desc) {
   ...
}

---

Comments from Reviewable

[igchor]

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

---

src/libpmemfile-posix/blocks.c, line 61 at r1 (raw file):

Previously, GBuella (Gabor Buella) wrote…

If I interpret it right, this is the new way to allow us to set a fixed block size, right? Just an idea, this could be a bit simpler I think: The `data_blocks` array can be a null terminated array (e.g. the last element has unit_size==0) If a user sets fix block size using an environment variable, then we just need to update the first element of this array, and make the second element be the terminator. So we would need to look at only the `data_blocks` array, instead of checking pmemfile_posix_block each time. Something like: ``` for (struct pobj_alloc_class_desc *desc = data_blocks; desc->unit_size !=0; ++desc) { ... } ```

Done.

---

Comments from Reviewable

[marcinslusarz]

First and second patch should be merged into one because at each commit it should be possible to build and run tests. (It's crucial for the ability to git bisect bugs)

Third and fourth should be merged into one or reordered - it's not possible to build pmemfile on Travis without the fourth patch.

[marcinslusarz]

Reviewed 21 of 29 files at r1, 6 of 9 files at r2. Review status: 27 of 34 files reviewed at latest revision, 12 unresolved discussions, some commit checks failed.

---

src/libpmemfile-posix/blocks.c, line 58 at r3 (raw file):

void
set_block_size_const(size_t size)

just set_block_size

---

src/libpmemfile-posix/blocks.h, line 48 at r3 (raw file):

/*
 * block_alignment value is always equal to smallest

the smallest

---

src/libpmemfile-posix/blocks.h, line 50 at r3 (raw file):

 * block_alignment value is always equal to smallest
 * block size, so it's either MIN_BLOCK_SIZE or
 * pmemfile_posix_block size if PMEMFILE_BLOCK_SIZE

s/size//

---

src/libpmemfile-posix/blocks.h, line 79 at r3 (raw file):

int initialize_alloc_classes(PMEMobjpool *pop);

too many empty lines

---

src/libpmemfile-posix/data.c, line 513 at r3 (raw file):

          struct alloc_class_info info =
              data_block_info(size, MAX_BLOCK_SIZE);

Shouldn't second argument depend on "over"?

---

src/libpmemfile-posix/data.c, line 543 at r3 (raw file):

          struct alloc_class_info info =
              data_block_info(size, MAX_BLOCK_SIZE);

ditto

---

src/libpmemfile-posix/data.c, line 575 at r3 (raw file):

          /* Are all those bytes needed? */
          if (hole_count > size)
              hole_count = size;

Why did you remove this?

---

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

  if (inode_is_regular_file(inode))
      inode->file_data.blocks.length =

inode->file_data.blocks.version?

---

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

      + 4  /* used */ \
      + 8 /* padding */\
      + sizeof(PMEMmutex) \

Please keep this list in the same order as fields in the structure.

---

src/libpmemfile-posix/pool.c, line 172 at r3 (raw file):

  }

  if (initialize_alloc_classes(pfp->pop)) {

can we do it in initialize_super_block?

---

utils/docker/images/install-nvml.sh, line 37 at r3 (raw file):

#

git clone https://github.com/igchor/nvml.git

I think we should switch to pmemobj master, without this patch, but with smaller block size to not hit the memory corruption. Also I think there's a benefit in keeping the compatibility with pmemobj 1.3 - missing allocation classes support should not result in failing to open the pool.

---

Comments from Reviewable

[igchor]

Review status: 27 of 34 files reviewed at latest revision, 11 unresolved discussions, some commit checks failed.

---

src/libpmemfile-posix/data.c, line 513 at r3 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

Shouldn't second argument depend on "over"?

Second argument is the biggest allowed block size, if it's MAX_BLOCK_SIZE, then there is no limit and data_block_info() will return the smallest block bigger or equal to "size". Do you want to be able to request bigger block size (depending on over)?

---

src/libpmemfile-posix/data.c, line 575 at r3 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

Why did you remove this?

Because in any case, we pass "size" as first argument to data_block_info() - we don't try to fill the entire hole. E.g if size = 8KB and hole_count = 256KB: data_block_info(size, hole_count) will return block of size 16KB

---

Comments from Reviewable

[krzycz]

General comment - reviewing the code that lacks comments/function briefs is not the most pleasant things to do.

---

Reviewed 26 of 29 files at r1, 8 of 9 files at r2, 1 of 1 files at r3. Review status: all files reviewed at latest revision, 17 unresolved discussions, some commit checks failed.

---

src/libpmemfile-posix/blocks.c, line 80 at r3 (raw file):

struct alloc_class_info
metadata_block_info(void)

I don't get the idea of having a function that is a wrapper for another function that returns a copy of a static const structure, from which you only need a class_id (METADATA_ID) in most cases. Why don't you define METADATA_ID in block.h and use it everywhere?

---

src/libpmemfile-posix/blocks.c, line 106 at r3 (raw file):

{
  char query[30];
  struct pobj_alloc_class_desc desc = *ac;

Do you really need a local copy of a structure pointed by 'ac'?

---

src/libpmemfile-posix/dir.c, line 195 at r3 (raw file):

          dir->next = TX_XALLOC(struct pmemfile_dir, info.size,
              POBJ_XALLOC_ZERO |
              POBJ_CLASS_ID(info.class_id));

Why not just POBJ_CLASS_ID(METADATA_ID)?

---

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

  /* padding / unused */
    uint32_t padding_;

padding1/padding2 ?

---

src/libpmemfile-posix/pmemfile-posix.c, line 82 at r3 (raw file):

  size_t pmemfile_posix_block_size = 0;

  char *tmp = getenv("PMEMFILE_BLOCK_SIZE");

Why don't you use some meaningful names for these variables? tmp => blk_size_env tmpll => blk_size ...

---

utils/docker/images/install-nvml.sh, line 37 at r1 (raw file):

#

git clone https://github.com/igchor/nvml.git

igchor ???

---

Comments from Reviewable

[igchor]

Review status: 21 of 34 files reviewed at latest revision, 15 unresolved discussions.

---

src/libpmemfile-posix/blocks.c, line 58 at r3 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

just set_block_size

Done.

---

src/libpmemfile-posix/blocks.c, line 80 at r3 (raw file):

Previously, krzycz (Krzysztof Czurylo) wrote…

I don't get the idea of having a function that is a wrapper for another function that returns a **copy** of a static const structure, from which you only need a class_id (METADATA_ID) in most cases. Why don't you define METADATA_ID in block.h and use it everywhere?

Right, I changed that and now I just return const pointer. In new revision I don't use METADATA_ID directly to support nvml 1.3.

---

src/libpmemfile-posix/blocks.c, line 106 at r3 (raw file):

Previously, krzycz (Krzysztof Czurylo) wrote…

Do you really need a local copy of a structure pointed by 'ac'?

Right, that was unnecessary

---

src/libpmemfile-posix/blocks.h, line 48 at r3 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

the smallest

Done.

---

src/libpmemfile-posix/blocks.h, line 50 at r3 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

s/size//

?

---

src/libpmemfile-posix/blocks.h, line 79 at r3 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

too many empty lines

Done.

---

src/libpmemfile-posix/data.c, line 543 at r3 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

ditto

up

---

src/libpmemfile-posix/dir.c, line 195 at r3 (raw file):

Previously, krzycz (Krzysztof Czurylo) wrote…

Why not just `POBJ_CLASS_ID(METADATA_ID)`?

I still use metadata_block_info() to support nvml 1.3 (when allocation classes are not supported info->class_id will be 0)

---

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

Previously, marcinslusarz (Marcin Ślusarz) wrote…

inode->file_data.blocks.version?

Done.

---

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

Previously, krzycz (Krzysztof Czurylo) wrote…

padding1/padding2 ?

Done.

---

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

Previously, marcinslusarz (Marcin Ślusarz) wrote…

Please keep this list in the same order as fields in the structure.

Done.

---

src/libpmemfile-posix/pmemfile-posix.c, line 82 at r3 (raw file):

Previously, krzycz (Krzysztof Czurylo) wrote…

Why don't you use some meaningful names for these variables? tmp => blk_size_env tmpll => blk_size ...

Done.

---

src/libpmemfile-posix/pool.c, line 172 at r3 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

can we do it in initialize_super_block?

Done.

---

utils/docker/images/install-nvml.sh, line 37 at r1 (raw file):

Previously, krzycz (Krzysztof Czurylo) wrote…

igchor ???

This commit was just to run this pull request, intended to remove this before merge

---

utils/docker/images/install-nvml.sh, line 37 at r3 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

I think we should switch to pmemobj master, without this patch, but with smaller block size to not hit the memory corruption. Also I think there's a benefit in keeping the compatibility with pmemobj 1.3 - missing allocation classes support should not result in failing to open the pool.

Done.

---

Comments from Reviewable

[igchor]

Review status: 21 of 34 files reviewed at latest revision, 15 unresolved discussions.

---

src/libpmemfile-posix/blocks.h, line 50 at r3 (raw file):

Previously, igchor wrote…

?

Done.

---

Comments from Reviewable

[marcinslusarz]

Reviewed 9 of 13 files at r4. Review status: 30 of 34 files reviewed at latest revision, 13 unresolved discussions.

---

src/libpmemfile-posix/blocks.c, line 53 at r5 (raw file):

  { 256 * 1024,       16 },
  { 2 * 1024 * 1024,  8 },
  { 0 } // terminator

no C++ comments in C code please

---

src/libpmemfile-posix/blocks.c, line 100 at r5 (raw file):

}

#ifdef POBJ_CLASS_ID // if allocation classes are supported

no C++ comments in C code please

---

src/libpmemfile-posix/blocks.c, line 137 at r5 (raw file):

  int ret = set_alloc_class(pop, &metadata_block, METADATA_ID);
  if (ret)
      return;

I think the decision about whether error should be ignored or not is on the caller side.

---

src/libpmemfile-posix/blocks.c, line 145 at r5 (raw file):

      ret = set_alloc_class(pop, block, FIRST_BLOCK_ID + index);
      if (ret)
          return;

ditto

---

src/libpmemfile-posix/blocks.h, line 52 at r5 (raw file):

// block_alignment value is always equal to the smallest block size

no C++ comments in C code please

---

src/libpmemfile-posix/data.c, line 530 at r5 (raw file):

              count = (uint32_t)(first_offset - offset);

              const struct pmem_block_info *info =

indentation is off here

---

utils/docker/images/install-nvml.sh, line 41 at r5 (raw file):

# checkout commit which introduces allocation classes
git checkout 852860df2aed2e7ffb7f3aadc27be49d080ba81b

Please use recent commit id.

---

Comments from Reviewable

[GBuella]

Review status: 30 of 34 files reviewed at latest revision, 13 unresolved discussions.

---

src/libpmemfile-posix/blocks.c, line 53 at r5 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

no C++ comments in C code please

BTW, these are technically C comments, just sayin...

---

Comments from Reviewable

[igchor]

Review status: 30 of 34 files reviewed at latest revision, 13 unresolved discussions.

---

src/libpmemfile-posix/blocks.c, line 53 at r5 (raw file):

Previously, GBuella (Gabor Buella) wrote…

BTW, these are technically C comments, just sayin...

Done.

---

src/libpmemfile-posix/blocks.c, line 100 at r5 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

no C++ comments in C code please

Done.

---

src/libpmemfile-posix/blocks.c, line 137 at r5 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

I think the decision about whether error should be ignored or not is on the caller side.

Ok, but set_alloc_class might return -1 when allocation classes are not supported and I think this should not result in error. we just don't use allocation classes then.

---

src/libpmemfile-posix/blocks.c, line 145 at r5 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

ditto

up

---

src/libpmemfile-posix/blocks.h, line 52 at r5 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

no C++ comments in C code please

Done.

---

src/libpmemfile-posix/data.c, line 530 at r5 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

indentation is off here

Done.

---

utils/docker/images/install-nvml.sh, line 41 at r5 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

Please use recent commit id.

Done.

---

Comments from Reviewable

[marcinslusarz]

Review status: 30 of 34 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

src/libpmemfile-posix/blocks.c, line 137 at r5 (raw file):

Previously, igchor wrote…

Ok, but set_alloc_class might return -1 when allocation classes are not supported and I think this should not result in error. we just don't use allocation classes then.

Just ignore it on the caller side. When we'll stop supporting 1.3 we will fail pool opening if allocation classes initialization failed.

BTW, please add error message when set_alloc_class fails.

---

Comments from Reviewable

[marcinslusarz]

Reviewed 2 of 4 files at r6, 1 of 1 files at r7. Review status: 33 of 34 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

Comments from Reviewable

[marcinslusarz]

Please rebase this PR. I'd like to see updated code coverage report using configuration from master.

[krzycz]

Reviewed 9 of 13 files at r4, 3 of 4 files at r6, 1 of 1 files at r7. Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

Comments from Reviewable

[igchor]

Done.

---

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

---

src/libpmemfile-posix/blocks.c, line 137 at r5 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

Just ignore it on the caller side. When we'll stop supporting 1.3 we will fail pool opening if allocation classes initialization failed. BTW, please add error message when set_alloc_class fails.

Done.

---

Comments from Reviewable

[krzycz]

Reviewed 11 of 12 files at r8, 1 of 1 files at r9. Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from Reviewable

[marcinslusarz]

---

Reviewed 11 of 12 files at r8, 1 of 1 files at r9. Review status: all files reviewed at latest revision, all discussions resolved, some commit checks pending.

---

Comments from Reviewable

[codecov[bot]]

Codecov Report

Merging #319 into master will decrease coverage by 0.07%. The diff coverage is 70.54%.

@@            Coverage Diff             @@
##           master     #319      +/-   ##
==========================================
- Coverage   75.04%   74.97%   -0.08%     
==========================================
  Files          64       66       +2     
  Lines        7922     7987      +65     
  Branches     1589     1597       +8     
==========================================
+ Hits         5945     5988      +43     
- Misses       1479     1498      +19     
- Partials      498      501       +3

Flag	Coverage Δ
#sqlite_tests	45.37% <52.05%> (-0.17%)	:arrow_down:
#tests_antool	12.44% <0%> (-0.11%)	:arrow_down:
#tests_posix_multi_threaded	26.16% <53.42%> (+0.15%)	:arrow_up:
#tests_posix_single_threaded	56.09% <69.86%> (+0.12%)	:arrow_up:
#tests_preload	46.51% <48.63%> (+0.39%)	:arrow_up:

Impacted Files	Coverage Δ
src/libpmemfile-posix/fallocate.c	61.9% <ø> (ø)	:arrow_up:
src/libpmemfile-posix/utils.h	66.66% <ø> (-19.05%)	:arrow_down:
src/libpmemfile-posix/utils.c	71.66% <ø> (-1.78%)	:arrow_down:
src/libpmemfile-posix/data.c	93.22% <100%> (+3.61%)	:arrow_up:
src/libpmemfile-posix/dir.c	85.5% <100%> (-0.25%)	:arrow_down:
src/libpmemfile-posix/blocks.h	100% <100%> (ø)
src/libpmemfile-posix/stat.c	93.87% <100%> (ø)	:arrow_up:
src/libpmemfile-posix/inode.c	88.65% <100%> (+0.15%)	:arrow_up:
src/libpmemfile-posix/block_array.c	92.68% <100%> (+1.01%)	:arrow_up:
src/libpmemfile-posix/pmemfile-posix.c	46.66% <30%> (+0.23%)	:arrow_up:
... and 11 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 16679fb...4dbccab. Read the comment docs.

[sarahjelinek]

Reviewed 12 of 29 files at r1, 6 of 13 files at r4, 1 of 4 files at r6, 10 of 12 files at r8, 1 of 1 files at r9. Review status: all files reviewed at latest revision, 6 unresolved discussions, some commit checks failed.

---

src/libpmemfile-posix/dir.c, line 192 at r9 (raw file):

      if (!found && TOID_IS_NULL(dir->next)) {
          const struct pmem_block_info *info =
              metadata_block_info();

I don't think you need to split the line here. It won't be > 80 chars will it? IMO, when lines are split unnecessarily it is harder to read and it looks more like a conditional than an assignment.

---

src/libpmemfile-posix/pmemfile-posix.c, line 87 at r9 (raw file):

      unsigned long long blk_size = strtoull(env, &end, 0);
      if (env[0] == '\0' || blk_size == ULLONG_MAX ||
              end[0] != '\0' || blk_size < MIN_BLOCK_SIZE) {

If there is no value associated with the PMEMFiLE_BLOCK_SIZE then env == NULL. I don't see a need to check for env[0] == '\0' here again. Also, end will be null if the call succeeds so I think checking for != NULL is fine, rather than checking the first character.

---

src/libpmemfile-posix/pool.c, line 87 at r9 (raw file):

  error = initialize_alloc_classes(pfp->pop);
  if (error) {
      /* allocation classes will not be used - ignore error */

Should you set error = 0 in this case if you are ignoring the error? Because it will be set to non-zero and may result in a false failure below.

---

src/libpmemfile-posix/pool.c, line 366 at r9 (raw file):

  error = initialize_alloc_classes(pfp->pop);
  if (error) {
      /* allocation classes will not be used - ignore error */

Set error = 0 here?

---

src/libpmemfile-posix/stat.c, line 91 at r9 (raw file):

  } else if (inode_is_dir(inode)) {
      size_t sz = inode->size - sizeof(inode->file_data);
      blks = (pmemfile_blkcnt_t)(sz / 512);

If 512 is used in other places make it a #define.

---

tests/posix/rw/rw.cpp, line 408 at r9 (raw file):

  if (env_block_size == 0x4000)
      EXPECT_TRUE(test_pmemfile_stats_match(pfp, 2, 0, 203, 12800));

Why did block size change to hex?

---

Comments from Reviewable

[igchor]

Review status: 33 of 34 files reviewed at latest revision, 6 unresolved discussions.

---

src/libpmemfile-posix/dir.c, line 192 at r9 (raw file):

Previously, sarahjelinek (Sarah Jelinek) wrote…

I don't think you need to split the line here. It won't be > 80 chars will it? IMO, when lines are split unnecessarily it is harder to read and it looks more like a conditional than an assignment.

Actually it is > 80 chars, tabs are counted as 8 chars.

---

src/libpmemfile-posix/pmemfile-posix.c, line 87 at r9 (raw file):

Previously, sarahjelinek (Sarah Jelinek) wrote…

If there is no value associated with the PMEMFiLE_BLOCK_SIZE then env == NULL. I don't see a need to check for env[0] == '\0' here again. Also, end will be null if the call succeeds so I think checking for != NULL is fine, rather than checking the first character.

This code existed before my patch so I may be wrong, but I think this check is to handle case when env is an empty string (it's not NULL in that case)

---

src/libpmemfile-posix/pool.c, line 87 at r9 (raw file):

Previously, sarahjelinek (Sarah Jelinek) wrote…

Should you set error = 0 in this case if you are ignoring the error? Because it will be set to non-zero and may result in a false failure below.

Done.

---

src/libpmemfile-posix/pool.c, line 366 at r9 (raw file):

Previously, sarahjelinek (Sarah Jelinek) wrote…

Set error = 0 here?

Done.

---

src/libpmemfile-posix/stat.c, line 91 at r9 (raw file):

Previously, sarahjelinek (Sarah Jelinek) wrote…

If 512 is used in other places make it a #define.

It is not used anywhere else, it's specific for that function.

---

tests/posix/rw/rw.cpp, line 408 at r9 (raw file):

Previously, sarahjelinek (Sarah Jelinek) wrote…

Why did block size change to hex?

In some cases, block size was given in hex and in some other in decimal. I tried to change that so that all block sizes (in conditional statements at least) was using one convention (hex).

---

Comments from Reviewable

[krzycz]

Reviewed 1 of 1 files at r10. Review status: all files reviewed at latest revision, 6 unresolved discussions, some commit checks failed.

---

Comments from Reviewable

[GBuella]

Reviewed 1 of 13 files at r4, 1 of 12 files at r8. Review status: all files reviewed at latest revision, 6 unresolved discussions, some commit checks failed.

---

src/libpmemfile-posix/dir.c, line 192 at r9 (raw file):

Previously, igchor wrote…

Actually it is > 80 chars, tabs are counted as 8 chars.

Yes, reviewable show them as 4 chars, but cstyle counts them as 8. This would end at column 83.

---

src/libpmemfile-posix/pmemfile-posix.c, line 87 at r9 (raw file):

If there is no value associated with the PMEMFiLE_BLOCK_SIZE then env == NULL.

Apparently getenv can return a pointer to an empty string, so yes, env[0] can be a null character. In that case strtoull can just return zero, without indicating any error.

Actually, this check could be moved to the if above, as in: if(env && env[0] != '\0') {

I just tried some scenarios with strtoull in glibc:

https://gist.github.com/GBuella/0ae002a871187a8421596f628ac8ab63

One should look at blk_size == ULLONG_MAX, as that indicates too large, or to small numbers. One should look at *end, as that indicates wrong characters, e.g. if one sets PMEMFILE_BLOCK_SIZE=16K hoping for 16 kilobyte blocks.

---

Comments from Reviewable

[GBuella]

Review status: all files reviewed at latest revision, 6 unresolved discussions, some commit checks failed.

---

tests/posix/rw/rw.cpp, line 408 at r9 (raw file):

Previously, igchor wrote…

In some cases, block size was given in hex and in some other in decimal. I tried to change that so that all block sizes (in conditional statements at least) was using one convention (hex).

And it is a bit more readable as well. Especially if at some places we add a few special cases for other block sizes:

if (env_block_size == 0x1000) {
   ...
} else if (env_block_size == 0x6000) {
} else if (env_block_size == 0x11000) {
}

But since apparently we treat these values as related, we could add a name for this somewhere in the headers for tests, e.g.: constexpr size_t test_blocksize = 0x4000; /* used for some tests with fix block size */

---

Comments from Reviewable

[GBuella]

Review status: all files reviewed at latest revision, 7 unresolved discussions, some commit checks failed.

---

tests/posix/rw/rw.cpp, line 1210 at r10 (raw file):

  ASSERT_EQ(pmemfile_lseek(pfp, f, 1, PMEMFILE_SEEK_DATA), 16384);
  ASSERT_EQ(pmemfile_lseek(pfp, f, 0, PMEMFILE_SEEK_DATA), 16384);
  ASSERT_EQ(pmemfile_lseek(pfp, f, 16382, PMEMFILE_SEEK_DATA), 16384);

And this could be (4 * test_block_size) - 2

---

Comments from Reviewable

[marcinslusarz]

Reviewed 1 of 1 files at r10. Review status: all files reviewed at latest revision, 7 unresolved discussions, some commit checks failed.

---

Comments from Reviewable

[marcinslusarz]

I think the issues Gabor pointed out are minor and can be fixed post-merge. Igor is on vacation this week, so he won't be able to fix it soon. This patch will conflict with what I'm working on.

[marcinslusarz]

Minor fix for pmemobj 1.3 compatibility: https://github.com/marcinslusarz/pmemfile/commit/f6bb70a920d4af153980add0a56cda7dd12b759e

[marcinslusarz]

And one more fix: https://github.com/marcinslusarz/pmemfile/commit/81a45589255061fc59f9ca0e6d13e166f0685769

[sarahjelinek]

---

Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

---

Comments from Reviewable

[GBuella]

---

Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

---

Comments from Reviewable