[PATCH] windows: fix pread/pwrite

[PATCH] windows: fix pread/pwrite

[Vincent Fu]

The pread and pwrite functions for Windows posix emulation never actually seek
to the requested offset. Fix this so that the psync ioengine works correctly on
Windows.

Signed-off-by: Vincent Fu <vincent.fu@samsung.com>
---
 os/windows/posix.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/os/windows/posix.c b/os/windows/posix.c
index e3abf383..3e48c3ff 100644
--- a/os/windows/posix.c
+++ b/os/windows/posix.c
@@ -830,18 +830,24 @@ ssize_t pwrite(int fildes, const void *buf, size_t nbyte,
 		off_t offset)
 {
 	int64_t pos = _telli64(fildes);
-	ssize_t len = _write(fildes, buf, nbyte);
+	ssize_t len;
 
+	_lseeki64(fildes, offset, SEEK_SET);
+	len = _write(fildes, buf, nbyte);
 	_lseeki64(fildes, pos, SEEK_SET);
+
 	return len;
 }
 
 ssize_t pread(int fildes, void *buf, size_t nbyte, off_t offset)
 {
 	int64_t pos = _telli64(fildes);
-	ssize_t len = read(fildes, buf, nbyte);
+	ssize_t len;
 
+	_lseeki64(fildes, offset, SEEK_SET);
+	len = read(fildes, buf, nbyte);
 	_lseeki64(fildes, pos, SEEK_SET);
+
 	return len;
 }
 
-- 
2.47.2

Re: [PATCH] windows: fix pread/pwrite

[Jens Axboe]

On 5/8/25 11:14 AM, Vincent Fu wrote:
> The pread and pwrite functions for Windows posix emulation never actually seek
> to the requested offset. Fix this so that the psync ioengine works correctly on
> Windows.
> 
> Signed-off-by: Vincent Fu <vincent.fu@samsung.com>
> ---
>  os/windows/posix.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/os/windows/posix.c b/os/windows/posix.c
> index e3abf383..3e48c3ff 100644
> --- a/os/windows/posix.c
> +++ b/os/windows/posix.c
> @@ -830,18 +830,24 @@ ssize_t pwrite(int fildes, const void *buf, size_t nbyte,
>  		off_t offset)
>  {
>  	int64_t pos = _telli64(fildes);
> -	ssize_t len = _write(fildes, buf, nbyte);
> +	ssize_t len;
>  
> +	_lseeki64(fildes, offset, SEEK_SET);
> +	len = _write(fildes, buf, nbyte);
>  	_lseeki64(fildes, pos, SEEK_SET);
> +
>  	return len;
>  }
>  
>  ssize_t pread(int fildes, void *buf, size_t nbyte, off_t offset)
>  {
>  	int64_t pos = _telli64(fildes);
> -	ssize_t len = read(fildes, buf, nbyte);
> +	ssize_t len;
>  
> +	_lseeki64(fildes, offset, SEEK_SET);
> +	len = read(fildes, buf, nbyte);
>  	_lseeki64(fildes, pos, SEEK_SET);
> +
>  	return len;
>  }

lol, how broken is that... Does windows have any offset read/write
variants, or is lseek required?

-- 
Jens Axboe

Re: [PATCH] windows: fix pread/pwrite

[Vincent Fu]

On Thu, May 8, 2025 at 1:18 PM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 5/8/25 11:14 AM, Vincent Fu wrote:
> > The pread and pwrite functions for Windows posix emulation never actually seek
> > to the requested offset. Fix this so that the psync ioengine works correctly on
> > Windows.
> >
> > Signed-off-by: Vincent Fu <vincent.fu@samsung.com>
> > ---
> >  os/windows/posix.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/os/windows/posix.c b/os/windows/posix.c
> > index e3abf383..3e48c3ff 100644
> > --- a/os/windows/posix.c
> > +++ b/os/windows/posix.c
> > @@ -830,18 +830,24 @@ ssize_t pwrite(int fildes, const void *buf, size_t nbyte,
> >               off_t offset)
> >  {
> >       int64_t pos = _telli64(fildes);
> > -     ssize_t len = _write(fildes, buf, nbyte);
> > +     ssize_t len;
> >
> > +     _lseeki64(fildes, offset, SEEK_SET);
> > +     len = _write(fildes, buf, nbyte);
> >       _lseeki64(fildes, pos, SEEK_SET);
> > +
> >       return len;
> >  }
> >
> >  ssize_t pread(int fildes, void *buf, size_t nbyte, off_t offset)
> >  {
> >       int64_t pos = _telli64(fildes);
> > -     ssize_t len = read(fildes, buf, nbyte);
> > +     ssize_t len;
> >
> > +     _lseeki64(fildes, offset, SEEK_SET);
> > +     len = read(fildes, buf, nbyte);
> >       _lseeki64(fildes, pos, SEEK_SET);
> > +
> >       return len;
> >  }
>
> lol, how broken is that... Does windows have any offset read/write
> variants, or is lseek required?
>
> --
> Jens Axboe
>

I could not find a better way to do this but I am hoping someone who
knows more about Windows than I do can weigh in. Sitsofe?

Vincent

Re: [PATCH] windows: fix pread/pwrite

[Jens Axboe]

On 5/8/25 11:31 AM, Vincent Fu wrote:
> On Thu, May 8, 2025 at 1:18?PM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> On 5/8/25 11:14 AM, Vincent Fu wrote:
>>> The pread and pwrite functions for Windows posix emulation never actually seek
>>> to the requested offset. Fix this so that the psync ioengine works correctly on
>>> Windows.
>>>
>>> Signed-off-by: Vincent Fu <vincent.fu@samsung.com>
>>> ---
>>>  os/windows/posix.c | 10 ++++++++--
>>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/os/windows/posix.c b/os/windows/posix.c
>>> index e3abf383..3e48c3ff 100644
>>> --- a/os/windows/posix.c
>>> +++ b/os/windows/posix.c
>>> @@ -830,18 +830,24 @@ ssize_t pwrite(int fildes, const void *buf, size_t nbyte,
>>>               off_t offset)
>>>  {
>>>       int64_t pos = _telli64(fildes);
>>> -     ssize_t len = _write(fildes, buf, nbyte);
>>> +     ssize_t len;
>>>
>>> +     _lseeki64(fildes, offset, SEEK_SET);
>>> +     len = _write(fildes, buf, nbyte);
>>>       _lseeki64(fildes, pos, SEEK_SET);
>>> +
>>>       return len;
>>>  }
>>>
>>>  ssize_t pread(int fildes, void *buf, size_t nbyte, off_t offset)
>>>  {
>>>       int64_t pos = _telli64(fildes);
>>> -     ssize_t len = read(fildes, buf, nbyte);
>>> +     ssize_t len;
>>>
>>> +     _lseeki64(fildes, offset, SEEK_SET);
>>> +     len = read(fildes, buf, nbyte);
>>>       _lseeki64(fildes, pos, SEEK_SET);
>>> +
>>>       return len;
>>>  }
>>
>> lol, how broken is that... Does windows have any offset read/write
>> variants, or is lseek required?
>>
>> --
>> Jens Axboe
>>
> 
> I could not find a better way to do this but I am hoping someone who
> knows more about Windows than I do can weigh in. Sitsofe?

It's probably not a big deal. I did a quick search and doesn't seem like
one exists for that.

-- 
Jens Axboe

Re: fio CI test result

[fiotestbot]

[-- Attachment #1: Type: text/plain, Size: 144 bytes --]


The result of fio's continuous integration tests was: success

For more details see https://github.com/fiotestbot/fio/actions/runs/14912501902

Re: [PATCH] windows: fix pread/pwrite

[Vincent Fu]

On 5/8/25 1:35 PM, Jens Axboe wrote:
> On 5/8/25 11:31 AM, Vincent Fu wrote:
>> On Thu, May 8, 2025 at 1:18?PM Jens Axboe <axboe@kernel.dk> wrote:
>>>
>>> On 5/8/25 11:14 AM, Vincent Fu wrote:
>>>> The pread and pwrite functions for Windows posix emulation never actually seek
>>>> to the requested offset. Fix this so that the psync ioengine works correctly on
>>>> Windows.
>>>>
>>>> Signed-off-by: Vincent Fu <vincent.fu@samsung.com>
>>>> ---
>>>>   os/windows/posix.c | 10 ++++++++--
>>>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/os/windows/posix.c b/os/windows/posix.c
>>>> index e3abf383..3e48c3ff 100644
>>>> --- a/os/windows/posix.c
>>>> +++ b/os/windows/posix.c
>>>> @@ -830,18 +830,24 @@ ssize_t pwrite(int fildes, const void *buf, size_t nbyte,
>>>>                off_t offset)
>>>>   {
>>>>        int64_t pos = _telli64(fildes);
>>>> -     ssize_t len = _write(fildes, buf, nbyte);
>>>> +     ssize_t len;
>>>>
>>>> +     _lseeki64(fildes, offset, SEEK_SET);
>>>> +     len = _write(fildes, buf, nbyte);
>>>>        _lseeki64(fildes, pos, SEEK_SET);
>>>> +
>>>>        return len;
>>>>   }
>>>>
>>>>   ssize_t pread(int fildes, void *buf, size_t nbyte, off_t offset)
>>>>   {
>>>>        int64_t pos = _telli64(fildes);
>>>> -     ssize_t len = read(fildes, buf, nbyte);
>>>> +     ssize_t len;
>>>>
>>>> +     _lseeki64(fildes, offset, SEEK_SET);
>>>> +     len = read(fildes, buf, nbyte);
>>>>        _lseeki64(fildes, pos, SEEK_SET);
>>>> +
>>>>        return len;
>>>>   }
>>>
>>> lol, how broken is that... Does windows have any offset read/write
>>> variants, or is lseek required?
>>>
>>> --
>>> Jens Axboe
>>>
>>
>> I could not find a better way to do this but I am hoping someone who
>> knows more about Windows than I do can weigh in. Sitsofe?
> 
> It's probably not a big deal. I did a quick search and doesn't seem like
> one exists for that.
> 

Ok thanks. I will go ahead with it then.

Vincent

Re: [PATCH] windows: fix pread/pwrite

[Sitsofe Wheeler]

Hi Vincent,

On Thu, 8 May 2025 at 18:33, Vincent Fu <vincentfu@gmail.com> wrote:
>
> On Thu, May 8, 2025 at 1:18 PM Jens Axboe <axboe@kernel.dk> wrote:
> >
> > On 5/8/25 11:14 AM, Vincent Fu wrote:
> > > The pread and pwrite functions for Windows posix emulation never actually seek
> > > to the requested offset. Fix this so that the psync ioengine works correctly on
> > > Windows.

Oof! Good catch!

[...]

> > lol, how broken is that... Does windows have any offset read/write
> > variants, or is lseek required?
>
> I could not find a better way to do this but I am hoping someone who
> knows more about Windows than I do can weigh in. Sitsofe?

Sadly (?) I'm not a Windows expert (even if I've occasionally played one) but
according to an answer on
https://stackoverflow.com/questions/3852758/equivalent-to-pread-pwrite-in-msvc
the only thing that can take an offset with the request is ReadFile/WriteFile
and this is done by passing an OVERLAPPED structure
(https://learn.microsoft.com/en-us/windows/win32/api/minwinbase/ns-minwinbase-overlapped
) which in turn can specify an offset even for synchronous I/O (see
https://learn.microsoft.com/en-gb/windows/win32/api/fileapi/nf-fileapi-writefile?redirectedfrom=MSDN#synchronization-and-file-position
). Unfortunately ReadFile/WriteFile only work with Windows handles but I think
you can get one from a file descriptor via _get_osfhandle (see
https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/get-osfhandle?view=msvc-170).

--
Sitsofe

Re: [PATCH] windows: fix pread/pwrite

[Vincent Fu]

On 5/8/25 5:28 PM, Sitsofe Wheeler wrote:
> Hi Vincent,
> 
> On Thu, 8 May 2025 at 18:33, Vincent Fu <vincentfu@gmail.com> wrote:
>>
>> On Thu, May 8, 2025 at 1:18 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>
>>> On 5/8/25 11:14 AM, Vincent Fu wrote:
>>>> The pread and pwrite functions for Windows posix emulation never actually seek
>>>> to the requested offset. Fix this so that the psync ioengine works correctly on
>>>> Windows.
> 
> Oof! Good catch!
> 
> [...]
> 
>>> lol, how broken is that... Does windows have any offset read/write
>>> variants, or is lseek required?
>>
>> I could not find a better way to do this but I am hoping someone who
>> knows more about Windows than I do can weigh in. Sitsofe?
> 
> Sadly (?) I'm not a Windows expert (even if I've occasionally played one) but
> according to an answer on
> https://stackoverflow.com/questions/3852758/equivalent-to-pread-pwrite-in-msvc
> the only thing that can take an offset with the request is ReadFile/WriteFile
> and this is done by passing an OVERLAPPED structure
> (https://learn.microsoft.com/en-us/windows/win32/api/minwinbase/ns-minwinbase-overlapped
> ) which in turn can specify an offset even for synchronous I/O (see
> https://learn.microsoft.com/en-gb/windows/win32/api/fileapi/nf-fileapi-writefile?redirectedfrom=MSDN#synchronization-and-file-position
> ). Unfortunately ReadFile/WriteFile only work with Windows handles but I think
> you can get one from a file descriptor via _get_osfhandle (see
> https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/get-osfhandle?view=msvc-170).
> 
> --
> Sitsofe

Sitsofe, many thanks for taking a look. I think my patch is good enough 
for now but I'm pleased to know that someone who really caress about 
this may be able to eventually improve on it.

Vincent