From: Milosz Tanski <milosz@adfin.com> To: Jeff Moyer <jmoyer@redhat.com> Cc: LKML <linux-kernel@vger.kernel.org>, Christoph Hellwig <hch@infradead.org>, "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>, linux-aio@kvack.org, Mel Gorman <mgorman@suse.de>, Volker Lendecke <Volker.Lendecke@sernet.de>, Tejun Heo <tj@kernel.org> Subject: Re: [PATCH 4/7] O_NONBLOCK flag for readv2/preadv2 Date: Tue, 16 Sep 2014 15:44:13 -0400 [thread overview] Message-ID: <CANP1eJEiGxQ0fZMkOY1utmkUOkGC4gOHYrXt+yO-9+tnzA24hw@mail.gmail.com> (raw) In-Reply-To: <x4961gnz844.fsf@segfault.boston.devel.redhat.com> On Tue, Sep 16, 2014 at 3:19 PM, Jeff Moyer <jmoyer@redhat.com> wrote: > Milosz Tanski <milosz@adfin.com> writes: > >> Filesystems that generic_file_read_iter will not be allowed to perform >> non-blocking reads. This only will read data if it's in the page cache and if >> there is no page error (causing a re-read). >> >> Signed-off-by: Milosz Tanski <milosz@adfin.com> > >> @@ -1662,6 +1676,10 @@ no_cached_page: >> goto out; >> } >> goto readpage; >> + >> +would_block: >> + error = -EAGAIN; >> + goto out; >> } > > Why did you put the wouldblock label inside the loop? That should be > pushed down to just above out, and then you can get rid of the goto. When I put the code outside the loop it actually looked worse (imo): } goto out; would_block: error = -EAGAIN; out: ... > > Other than that, it looks like you put the check in all the right places > in that function. > >> out: >> @@ -1697,6 +1715,9 @@ generic_file_read_iter(struct kiocb *iocb, struct iov_iter *iter, int flags) >> size_t count = iov_iter_count(iter); >> loff_t size; >> >> + if (flags & O_NONBLOCK) >> + return -EAGAIN; >> + > > If a program is attempting non-blocking reads on a file opened with > O_DIRECT, I think returning -EAGAIN is very misleading. Better to > return -EINVAL in this case, and maybe check that earlier in the stack? Point taken and I can fix this for the next version further up the stack. A longer term question is how the flags the file is open with interact with the read/write flags ... since I imagine folks will want to add other flags (like force a SYNC write). > > Cheers, > Jeff -- Milosz Tanski CTO 16 East 34th Street, 15th floor New York, NY 10016 p: 646-253-9055 e: milosz@adfin.com
WARNING: multiple messages have this Message-ID (diff)
From: Milosz Tanski <milosz@adfin.com> To: Jeff Moyer <jmoyer@redhat.com> Cc: LKML <linux-kernel@vger.kernel.org>, Christoph Hellwig <hch@infradead.org>, "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>, linux-aio@kvack.org, Mel Gorman <mgorman@suse.de>, Volker Lendecke <Volker.Lendecke@sernet.de>, Tejun Heo <tj@kernel.org> Subject: Re: [PATCH 4/7] O_NONBLOCK flag for readv2/preadv2 Date: Tue, 16 Sep 2014 15:44:13 -0400 [thread overview] Message-ID: <CANP1eJEiGxQ0fZMkOY1utmkUOkGC4gOHYrXt+yO-9+tnzA24hw@mail.gmail.com> (raw) In-Reply-To: <x4961gnz844.fsf@segfault.boston.devel.redhat.com> On Tue, Sep 16, 2014 at 3:19 PM, Jeff Moyer <jmoyer@redhat.com> wrote: > Milosz Tanski <milosz@adfin.com> writes: > >> Filesystems that generic_file_read_iter will not be allowed to perform >> non-blocking reads. This only will read data if it's in the page cache and if >> there is no page error (causing a re-read). >> >> Signed-off-by: Milosz Tanski <milosz@adfin.com> > >> @@ -1662,6 +1676,10 @@ no_cached_page: >> goto out; >> } >> goto readpage; >> + >> +would_block: >> + error = -EAGAIN; >> + goto out; >> } > > Why did you put the wouldblock label inside the loop? That should be > pushed down to just above out, and then you can get rid of the goto. When I put the code outside the loop it actually looked worse (imo): } goto out; would_block: error = -EAGAIN; out: ... > > Other than that, it looks like you put the check in all the right places > in that function. > >> out: >> @@ -1697,6 +1715,9 @@ generic_file_read_iter(struct kiocb *iocb, struct iov_iter *iter, int flags) >> size_t count = iov_iter_count(iter); >> loff_t size; >> >> + if (flags & O_NONBLOCK) >> + return -EAGAIN; >> + > > If a program is attempting non-blocking reads on a file opened with > O_DIRECT, I think returning -EAGAIN is very misleading. Better to > return -EINVAL in this case, and maybe check that earlier in the stack? Point taken and I can fix this for the next version further up the stack. A longer term question is how the flags the file is open with interact with the read/write flags ... since I imagine folks will want to add other flags (like force a SYNC write). > > Cheers, > Jeff -- Milosz Tanski CTO 16 East 34th Street, 15th floor New York, NY 10016 p: 646-253-9055 e: milosz@adfin.com -- To unsubscribe, send a message with 'unsubscribe linux-aio' in the body to majordomo@kvack.org. For more info on Linux AIO, see: http://www.kvack.org/aio/ Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
next prev parent reply other threads:[~2014-09-16 19:44 UTC|newest] Thread overview: 167+ messages / expand[flat|nested] mbox.gz Atom feed top 2014-09-15 20:20 [RFC PATCH 0/7] Non-blockling buffered fs read (page cache only) Milosz Tanski 2014-09-15 20:20 ` Milosz Tanski 2014-09-15 20:20 ` [PATCH 1/7] Prepare for adding a new readv/writev with user flags Milosz Tanski 2014-09-15 20:20 ` Milosz Tanski 2014-09-15 20:28 ` Al Viro 2014-09-15 21:15 ` Christoph Hellwig 2014-09-15 21:15 ` Christoph Hellwig 2014-09-15 21:44 ` Milosz Tanski 2014-09-15 21:44 ` Milosz Tanski 2014-09-15 20:20 ` [PATCH 2/7] Define new syscalls readv2,preadv2,writev2,pwritev2 Milosz Tanski 2014-09-15 20:20 ` Milosz Tanski 2014-09-16 19:20 ` Jeff Moyer 2014-09-16 19:20 ` Jeff Moyer 2014-09-16 19:54 ` Milosz Tanski 2014-09-16 19:54 ` Milosz Tanski 2014-09-16 21:03 ` Christoph Hellwig 2014-09-16 21:03 ` Christoph Hellwig 2014-09-17 15:43 ` Theodore Ts'o 2014-09-17 15:43 ` Theodore Ts'o 2014-09-17 16:05 ` Milosz Tanski 2014-09-17 16:05 ` Milosz Tanski 2014-09-17 16:59 ` Theodore Ts'o 2014-09-17 16:59 ` Theodore Ts'o 2014-09-17 17:24 ` Zach Brown 2014-09-17 17:24 ` Zach Brown 2014-09-15 20:20 ` [PATCH 3/7] Export new vector IO (with flags) to userland Milosz Tanski 2014-09-15 20:20 ` Milosz Tanski 2014-09-15 20:21 ` [PATCH 4/7] O_NONBLOCK flag for readv2/preadv2 Milosz Tanski 2014-09-15 20:21 ` Milosz Tanski 2014-09-16 19:19 ` Jeff Moyer 2014-09-16 19:19 ` Jeff Moyer 2014-09-16 19:44 ` Milosz Tanski [this message] 2014-09-16 19:44 ` Milosz Tanski 2014-09-16 19:53 ` Jeff Moyer 2014-09-16 19:53 ` Jeff Moyer 2014-09-15 20:21 ` [PATCH 5/7] documentation updates Christoph Hellwig 2014-09-15 20:21 ` Christoph Hellwig 2014-09-15 20:21 ` [PATCH 6/7] move flags enforcement to vfs_preadv/vfs_pwritev Christoph Hellwig 2014-09-15 21:15 ` Christoph Hellwig 2014-09-15 21:15 ` Christoph Hellwig 2014-09-15 21:45 ` Milosz Tanski 2014-09-15 21:45 ` Milosz Tanski 2014-09-15 20:22 ` [PATCH 7/7] check for O_NONBLOCK in all read_iter instances Christoph Hellwig 2014-09-15 20:22 ` Christoph Hellwig 2014-09-16 19:27 ` Jeff Moyer 2014-09-16 19:27 ` Jeff Moyer 2014-09-16 19:45 ` Milosz Tanski 2014-09-16 19:45 ` Milosz Tanski 2014-09-16 21:42 ` Dave Chinner 2014-09-16 21:42 ` Dave Chinner 2014-09-17 12:24 ` Benjamin LaHaise 2014-09-17 12:24 ` Benjamin LaHaise 2014-09-17 13:47 ` Theodore Ts'o 2014-09-17 13:47 ` Theodore Ts'o 2014-09-17 13:56 ` Benjamin LaHaise 2014-09-17 13:56 ` Benjamin LaHaise 2014-09-17 15:33 ` Milosz Tanski 2014-09-17 15:33 ` Milosz Tanski 2014-09-17 15:49 ` Theodore Ts'o 2014-09-17 15:49 ` Theodore Ts'o 2014-09-17 15:52 ` Zach Brown 2014-09-17 15:52 ` Zach Brown 2014-09-16 21:04 ` Christoph Hellwig 2014-09-16 21:04 ` Christoph Hellwig 2014-09-16 21:24 ` Jeff Moyer 2014-09-16 21:24 ` Jeff Moyer 2014-09-15 20:27 ` [RFC PATCH 0/7] Non-blockling buffered fs read (page cache only) Milosz Tanski 2014-09-15 20:27 ` Milosz Tanski 2014-09-15 21:33 ` Andreas Dilger 2014-09-15 22:13 ` Milosz Tanski 2014-09-15 22:13 ` Milosz Tanski 2014-09-15 22:36 ` Elliott, Robert (Server Storage) 2014-09-15 22:36 ` Elliott, Robert (Server Storage) 2014-09-16 18:24 ` Zach Brown 2014-09-16 18:24 ` Zach Brown 2014-09-19 11:21 ` Christoph Hellwig 2014-09-19 11:21 ` Christoph Hellwig 2014-09-22 15:48 ` Jeff Moyer 2014-09-22 15:48 ` Jeff Moyer 2014-09-22 16:32 ` Milosz Tanski 2014-09-22 16:32 ` Milosz Tanski 2014-09-22 16:42 ` Christoph Hellwig 2014-09-22 17:02 ` Milosz Tanski 2014-09-22 17:02 ` Milosz Tanski 2014-09-22 16:25 ` Elliott, Robert (Server Storage) 2014-09-15 21:58 ` Jeff Moyer 2014-09-15 21:58 ` Jeff Moyer 2014-09-15 22:27 ` Milosz Tanski 2014-09-15 22:27 ` Milosz Tanski 2014-09-16 13:44 ` Jeff Moyer 2014-09-16 13:44 ` Jeff Moyer 2014-09-19 11:23 ` Christoph Hellwig 2014-09-19 11:23 ` Christoph Hellwig 2014-09-16 19:30 ` Jeff Moyer 2014-09-16 19:30 ` Jeff Moyer 2014-09-16 20:34 ` Milosz Tanski 2014-09-16 20:34 ` Milosz Tanski 2014-09-16 20:49 ` Jeff Moyer 2014-09-16 20:49 ` Jeff Moyer 2014-09-17 14:49 ` [RFC 1/2] aio: async readahead Benjamin LaHaise 2014-09-17 14:49 ` Benjamin LaHaise 2014-09-17 15:26 ` [RFC 2/2] ext4: async readpage for indirect style inodes Benjamin LaHaise 2014-09-17 15:26 ` Benjamin LaHaise 2014-09-19 11:26 ` [RFC 1/2] aio: async readahead Christoph Hellwig 2014-09-19 11:26 ` Christoph Hellwig 2014-09-19 16:01 ` Benjamin LaHaise 2014-09-19 16:01 ` Benjamin LaHaise 2014-09-17 22:20 ` [RFC v2 0/5] Non-blockling buffered fs read (page cache only) Milosz Tanski 2014-09-17 22:20 ` Milosz Tanski 2014-09-17 22:20 ` [RFC v2 1/5] Prepare for adding a new readv/writev with user flags Milosz Tanski 2014-09-17 22:20 ` Milosz Tanski 2014-09-17 22:20 ` [RFC v2 2/5] Define new syscalls readv2,preadv2,writev2,pwritev2 Milosz Tanski 2014-09-17 22:20 ` Milosz Tanski 2014-09-18 18:48 ` Darrick J. Wong 2014-09-18 18:48 ` Darrick J. Wong 2014-09-19 10:52 ` Christoph Hellwig 2014-09-19 10:52 ` Christoph Hellwig 2014-09-20 0:19 ` Darrick J. Wong 2014-09-20 0:19 ` Darrick J. Wong 2014-09-17 22:20 ` [RFC v2 3/5] Export new vector IO (with flags) to userland Milosz Tanski 2014-09-17 22:20 ` Milosz Tanski 2014-09-17 22:20 ` [RFC v2 4/5] O_NONBLOCK flag for readv2/preadv2 Milosz Tanski 2014-09-17 22:20 ` Milosz Tanski 2014-09-19 11:27 ` Christoph Hellwig 2014-09-19 11:27 ` Christoph Hellwig 2014-09-19 11:59 ` Milosz Tanski 2014-09-19 11:59 ` Milosz Tanski 2014-09-22 17:12 ` Jeff Moyer 2014-09-22 17:12 ` Jeff Moyer 2014-09-17 22:20 ` [RFC v2 5/5] Check for O_NONBLOCK in all read_iter instances Milosz Tanski 2014-09-17 22:20 ` Milosz Tanski 2014-09-19 11:26 ` Christoph Hellwig 2014-09-19 11:26 ` Christoph Hellwig 2014-09-19 14:42 ` [RFC v2 0/5] Non-blockling buffered fs read (page cache only) Jonathan Corbet 2014-09-19 14:42 ` Jonathan Corbet 2014-09-19 16:13 ` Volker Lendecke 2014-09-19 16:13 ` Volker Lendecke 2014-09-19 17:19 ` Milosz Tanski 2014-09-19 17:19 ` Milosz Tanski 2014-09-19 17:33 ` Milosz Tanski 2014-09-19 17:33 ` Milosz Tanski 2014-09-22 14:12 ` Jonathan Corbet 2014-09-22 14:12 ` Jonathan Corbet 2014-09-22 14:24 ` Jeff Moyer 2014-09-22 14:24 ` Jeff Moyer 2014-09-22 14:25 ` Christoph Hellwig 2014-09-22 14:25 ` Christoph Hellwig 2014-09-22 14:30 ` Milosz Tanski 2014-09-22 14:30 ` Milosz Tanski 2014-09-24 21:46 ` [RFC v3 0/4] vfs: " Milosz Tanski 2014-09-24 21:46 ` Milosz Tanski 2014-09-24 21:46 ` [RFC v3 1/4] vfs: Prepare for adding a new preadv/pwritev with user flags Milosz Tanski 2014-09-24 21:46 ` Milosz Tanski 2014-09-24 21:46 ` [RFC v3 2/4] vfs: Define new syscalls preadv2,pwritev2 Milosz Tanski 2014-09-24 21:46 ` Milosz Tanski 2014-09-24 21:46 ` [RFC v3 3/4] vfs: Export new vector IO syscalls (with flags) to userland Milosz Tanski 2014-09-24 21:46 ` Milosz Tanski 2014-09-24 21:46 ` [RFC v3 4/4] vfs: RWF_NONBLOCK flag for preadv2 Milosz Tanski 2014-09-24 21:46 ` Milosz Tanski 2014-09-25 4:06 ` [RFC v3 0/4] vfs: Non-blockling buffered fs read (page cache only) Michael Kerrisk 2014-09-25 4:06 ` Michael Kerrisk 2014-09-25 11:16 ` Jan Kara 2014-09-25 11:16 ` Jan Kara 2014-09-25 15:48 ` Milosz Tanski 2014-09-25 15:48 ` Milosz Tanski 2014-10-08 2:53 ` Milosz Tanski 2014-10-08 2:53 ` Milosz Tanski
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CANP1eJEiGxQ0fZMkOY1utmkUOkGC4gOHYrXt+yO-9+tnzA24hw@mail.gmail.com \ --to=milosz@adfin.com \ --cc=Volker.Lendecke@sernet.de \ --cc=hch@infradead.org \ --cc=jmoyer@redhat.com \ --cc=linux-aio@kvack.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mgorman@suse.de \ --cc=tj@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.