Shouldn't this be -EOPNOTSUPP, since -EINVAL is for incorrect arguments, which isn't the case here. Cheers, Andreas > On Jan 3, 2014, at 3:35, Jan Kara <jack@xxxxxxx> wrote: > > Generic implementation of SEEK_HOLE & SEEK_DATA in > generic_file_llseek_size() and default_llseek() behaved as if everything > within i_size is data and everything beyond i_size is a hole. That makes > sense at the first sight (and definitely is a valid implementation of > the spec) but at the second sight it isn't very useful. If anyone > bothers with looking for holes / data, he should be better told we don't > really know so that he can fall back to his chosen backup strategy > (think of e.g. cp(1)). > > This is a userspace API change however the kernel interface is there > only for two years so hopefully userspace is still prepared to handle > EINVAL return value. > > Signed-off-by: Jan Kara <jack@xxxxxxx> > --- > fs/read_write.c | 38 ++------------------------------------ > 1 file changed, 2 insertions(+), 36 deletions(-) > > So given our discussion in > http://www.spinics.net/lists/linux-fsdevel/msg71363.html > I've decided to throw this patch in. Al, what do you think about it? > > diff --git a/fs/read_write.c b/fs/read_write.c > index 58e440df1bc6..40a324f1d93b 100644 > --- a/fs/read_write.c > +++ b/fs/read_write.c > @@ -111,22 +111,8 @@ generic_file_llseek_size(struct file *file, loff_t offset, int whence, > spin_unlock(&file->f_lock); > return offset; > case SEEK_DATA: > - /* > - * In the generic case the entire file is data, so as long as > - * offset isn't at the end of the file then the offset is data. > - */ > - if (offset >= eof) > - return -ENXIO; > - break; > case SEEK_HOLE: > - /* > - * There is a virtual hole at the end of the file, so as long as > - * offset isn't i_size or larger, return i_size. > - */ > - if (offset >= eof) > - return -ENXIO; > - offset = eof; > - break; > + return -EINVAL; > } > > return vfs_setpos(file, offset, maxsize); > @@ -214,28 +200,8 @@ loff_t default_llseek(struct file *file, loff_t offset, int whence) > offset += file->f_pos; > break; > case SEEK_DATA: > - /* > - * In the generic case the entire file is data, so as > - * long as offset isn't at the end of the file then the > - * offset is data. > - */ > - if (offset >= inode->i_size) { > - retval = -ENXIO; > - goto out; > - } > - break; > case SEEK_HOLE: > - /* > - * There is a virtual hole at the end of the file, so > - * as long as offset isn't i_size or larger, return > - * i_size. > - */ > - if (offset >= inode->i_size) { > - retval = -ENXIO; > - goto out; > - } > - offset = inode->i_size; > - break; > + return -EINVAL; > } > retval = -EINVAL; > if (offset >= 0 || unsigned_offsets(file)) { > -- > 1.8.1.4 > > -- > 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 -- 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
