Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Kirill Smelkov <kirr@nexedi.com>
Subject: Re: [PATCH AUTOSEL 5.0 59/66] fs: stream_open - opener for stream-like files so that read and write can run simultaneously without deadlock
To:     David Laight <David.Laight@ACULAB.COM>
Cc:     Linus Torvalds <torvalds@linux-foundation.org>,
        Sasha Levin <sashal@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Linux List Kernel Mailing <linux-kernel@vger.kernel.org>,
        stable <stable@vger.kernel.org>,
        Michael Kerrisk <mtk.manpages@gmail.com>,
        Yongzhi Pan <panyongzhi@gmail.com>,
        Jonathan Corbet <corbet@lwn.net>,
        David Vrabel <david.vrabel@citrix.com>,
        Juergen Gross <jgross@suse.com>,
        Miklos Szeredi <miklos@szeredi.hu>, Tejun Heo <tj@kernel.org>,
        Kirill Tkhai <ktkhai@virtuozzo.com>,
        Arnd Bergmann <arnd@arndb.de>, Christoph Hellwig <hch@lst.de>,
        Julia Lawall <Julia.Lawall@lip6.fr>,
        Nikolaus Rath <Nikolaus@rath.org>,
        Han-Wen Nienhuys <hanwen@google.com>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>
Message-Id: <20190426182014.GA23128@deco.navytux.spb.ru>
References: <20190424143341.27665-1-sashal@kernel.org> <20190424143341.27665-59-sashal@kernel.org> <20190424163415.GB21413@kroah.com> <CAHk-=wjEOyba5As1PEMk6RitNVOJH9oJ_Jbg4y=5B0fcX1iKGw@mail.gmail.com> <20190424171926.GA17719@sasha-vm> <CAHk-=wgh234SyBG810=vB360PCzVkAhQRqGg8aFdATZd+daCFw@mail.gmail.com> <20190424183012.GB3798@deco.navytux.spb.ru> <4d366f81f90442cb9da7ad393680d004@AcuMS.aculab.com> <20190426074522.GA16247@deco.navytux.spb.ru> <073e5def9e654a1d80cdd79cdcf23361@AcuMS.aculab.com>
In-Reply-To: <073e5def9e654a1d80cdd79cdcf23361@AcuMS.aculab.com>
Date:   Fri, 26 Apr 2019 18:20:24 +0000
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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().