On Fri, Apr 26, 2019 at 11:00:01AM +0000, David Laight wrote:
From: Kirill Smelkov
Sent: 26 April 2019 08:46
...
I'm not sure I understand your comment completely, but we convert to stream_open only drivers that actually do _not_ use position at all, and that were already using nonseekable_open, thus pread and pwrite were already returning -ESPIPE for them (nonseekable_open clears FMODE_{PREAD,PWRITE} and ksys_{pread,pwrite}64 check for that flag). We also convert only drivers that use no_llseek for .llseek, so lseek on those files is/was always returning -ESPIPE as well.
If a driver uses position in its read and write and has support for pread/pwrite (FMODE_PREAD and FMODE_PWRITE), pread and pwrite are already working _without_ file->f_pos locking - because those system calls do not semantically update file->f_pos at all and thus do not take file->f_pos_lock - i.e. pread/pwrite can be run simultaneously already.
Looks like I knew that once :-) Mind you, 'man pread' on my system is somewhat uninformative.
Maybe pread() should always be allowed at offset 0. Then you wouldn't need all this extra logic.
I'm not sure I understand. Do you propose any change? If yes - what is the change you are proposing?
If libc implements pread as lseek+read it will work for a single user case (single thread, or fd not shared between processes), but it will break because of lseek+read non-atomicity if multiple preads are simultaneously used from several threads. And also for such emulation for multiple users case there is a chance for pread vs pwrite deadlock, since those system calls are using read and write and read and write take file->f_pos_lock.
I'd actually rather the pread() failed to compile.
Ok.
The actual implementation did 3 lseek()s (to save and restore the offset). A user level emulation could usually get away with one lseek().