On Aug 27, 2018, at 6:56 AM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > Implement stacked fadvise to fix syscalls readahead(2) and fadvise64(2) > on an overlayfs file. I was just looking into the existence of the "new" fadvise() method in the VFS being able to communicate application hints directly to the filesystem to see if it could be used to address the word size issue in https://bugzilla.kernel.org/show_bug.cgi?id=205957 without adding a new syscall, and came across this patch and the 4/6 patch that adds the vfs_fadvise() function itself (copied below for clarity). It seems to me that this implementation is broken? Only vfs_fadvise() is called from the fadvise64() syscall, and it will call f_op->fadvise() if the filesystem provides this method. Only overlayfs provides the .fadvise method today. However, it looks that ovl_fadvise() calls back into vfs_fadvise() again, in a seemingly endless loop? It seems like generic_fadvise() should be EXPORT_SYMBOL() so that any filesystem that implements its own .fadvise method can do its own thing, and then call generic_fadvise() to handle the remaining MM-specific work. Thoughts? > +int vfs_fadvise(struct file *file, loff_t offset, loff_t len, int advice) > +{ > + if (file->f_op->fadvise) > + return file->f_op->fadvise(file, offset, len, advice); > + > + return generic_fadvise(file, offset, len, advice); > +} > +EXPORT_SYMBOL(vfs_fadvise); > > +int ovl_fadvise(struct file *file, loff_t offset, loff_t len, int advice) > +{ > + struct fd real; > + int ret; > + > + ret = ovl_real_fdget(file, &real); > + if (ret) > + return ret; > + > + /* XXX: do we need mounter credentials? */ > + ret = vfs_fadvise(real.file, offset, len, advice); > + > + fdput(real); > + > + return ret; > +} Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP
