On Tue, Sep 16, 2014 at 3:19 PM, Jeff Moyer <jmoyer@xxxxxxxxxx> wrote: > Milosz Tanski <milosz@xxxxxxxxx> 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@xxxxxxxxx> > >> @@ -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@xxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
