On 2023-02-13 18:33:34 +0900, Kyotaro Horiguchi wrote: > At Mon, 13 Feb 2023 10:15:03 +0530, Bharath Rupireddy > <bharath.rupireddyforpostg...@gmail.com> wrote in > > On Sun, Feb 12, 2023 at 11:01 PM Andres Freund <and...@anarazel.de> wrote: > > > > > > On 2023-02-12 19:59:00 +0530, Bharath Rupireddy wrote: > > > > /* Prepare to write out a lot of copies of our zero buffer at > > > > once. */ > > > > for (i = 0; i < lengthof(iov); ++i) > > > > { > > > > - iov[i].iov_base = zbuffer.data; > > > > + iov[i].iov_base = (void *) (unconstify(PGAlignedBlock *, > > > > &zbuffer)->data); > > > > iov[i].iov_len = zbuffer_sz; > > > > } > > > > > > Another thing: I think we should either avoid iterating over all the IOVs > > > if > > > we don't need them, or, even better, initialize the array as a constant, > > > once. > > FWIW, I tried to use the "{[start .. end] = {}}" trick (GNU extension? > [*1]) for constant array initialization, but individual members don't > accept assigning a const value, thus I did deconstify as the follows. > > > static const struct iovec iov[PG_IOV_MAX] = > > {[0 ... PG_IOV_MAX - 1] = > > { > > .iov_base = (void *)&zbuffer.data, > > .iov_len = BLCKSZ > > } > > }; > > I didn't checked the actual mapping, but if I tried an assignment > "iov[0].iov_base = NULL", it failed as "assignment of member > ‘iov_base’ in read-only object", so is it successfully placed in a > read-only segment? > > Later code assigns iov[0].iov_len thus we need to provide a separate > iov non-const variable, or can we use pwrite instead there? (I didn't > find pg_pwrite_with_retry(), though)
Given that we need to do that, and given that we already need to loop to handle writes that are longer than PG_IOV_MAX * BLCKSZ, it's probably not worth avoiding iov initialization. But I think it's worth limiting the initialization to blocks. I'd also try to combine the first pg_writev_* with the second one. Greetings, Andres Freund