posix_fadvise v22

Lists: pgsql-hackers
From: Gregory Stark <stark(at)enterprisedb(dot)com>
To: Postgres <pgsql-hackers(at)postgresql(dot)org>
Subject: posix_fadvise v22
Date: 2008-12-09 14:57:38
Message-ID: 87oczliefx.fsf@oxford.xeocode.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


Here's an update to eliminate two small bitrot conflicts.

Attachment Content-Type Size
posix_fadvise_v22.diff.gz application/octet-stream 12.9 KB
From: ITAGAKI Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>
To: Gregory Stark <stark(at)enterprisedb(dot)com>
Cc: Postgres <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: posix_fadvise v22
Date: 2008-12-10 09:22:56
Message-ID: 20081210180009.EAE2.52131E4D@oss.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hello,

Gregory Stark <stark(at)enterprisedb(dot)com> wrote:
> Here's an update to eliminate two small bitrot conflicts.

I read your patch with interest, but found some trivial bad manners.

* LET_OS_MANAGE_FILESIZE is already obsoleted.
You don't have to cope with the option.

* Type mismatch in prefetch_pages
A variable prefetch_pages is defined as "unsigned" or "int"
in some places. Why don't you define it only once in a header
and include the header in source files?

* Assignment to prefetch_pages
What do "+0.99" means here?
[assign_io_concurrency()]
+ prefetch_pages = new_prefetch_pages+0.99;
You want to do as follows, right?
+ prefetch_pages = (int) ceil(new_prefetch_pages);

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center