On Fri, 18 Jul 2014, Rohan Puri wrote: > Date: Fri, 18 Jul 2014 16:13:21 +0530 > From: Rohan Puri <rohan.puri15@xxxxxxxxx> > To: Lukáš Czerner <lczerner@xxxxxxxxxx> > Cc: hch@xxxxxxxxxxxxx, viro@xxxxxxxxxxxxxxxxxx, > Linux FS Devel <linux-fsdevel@xxxxxxxxxxxxxxx> > Subject: Re: [PATCH 1/2] Remove misleading comment in do_fallocate() > > On Fri, Jul 18, 2014 at 4:05 PM, Lukáš Czerner <lczerner@xxxxxxxxxx> wrote: > > > On Fri, 18 Jul 2014, Rohan Puri wrote: > > > > > Date: Fri, 18 Jul 2014 15:27:26 +0530 > > > From: Rohan Puri <rohan.puri15@xxxxxxxxx> > > > To: hch@xxxxxxxxxxxxx, viro@xxxxxxxxxxxxxxxxxx > > > Cc: linux-fsdevel@xxxxxxxxxxxxxxx > > > Subject: [PATCH 1/2] Remove misleading comment in do_fallocate() > > > > > > Preallocation for directory gets blocked at 2 places & never gets > > propogated > > > to the underlying file system : - > > > > > > - open(2) with O_DIRECTORY | O_WRONLY or O_DIRECTORY | O_RDWR > > > at do_last()->may_open(). > > > - open(2) with O_DIRECTORY | O_RDONLY -> fallocate(2) at do_fallocate(). > > > > > > Signed-off-by: Rohan Puri <rohan.puri15@xxxxxxxxx> > > > --- > > > fs/open.c | 4 ---- > > > 1 file changed, 4 deletions(-) > > > > > > diff --git a/fs/open.c b/fs/open.c > > > index 9d64679..5d4f372 100644 > > > --- a/fs/open.c > > > +++ b/fs/open.c > > > @@ -280,10 +280,6 @@ int do_fallocate(struct file *file, int mode, > > loff_t offset, loff_t len) > > > if (S_ISFIFO(inode->i_mode)) > > > return -ESPIPE; > > > > > > - /* > > > - * Let individual file system decide if it supports preallocation > > > - * for directories or not. > > > - */ > > > > What's wrong about this comment ? We're only going to let through > > regular files and directories and it's up to the file system to > > decide what it want to support. > > > I agree, but AFAIK for directories we are never going to reach upto the > file systems, since directory file opened with O_WRONLY or O_RDWR flag gets > blocked by open(2)->do_last()->may_open() and directory with O_RDONLY gets > blocked by do_fallocate() returning -EBADF. That's something that needs to be resolved when someone decides to add fallocate support for directories. There has already been a small discussion about that idea. So I think that the comment is just fine from the fallocate perspective. -Lukas > So this comment is misleading in terms that for directory we never go to > file systems during fallocate. > > > > > -Lukas > > > > > if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode)) > > > return -ENODEV; > > > > > > > > > > - Rohan >
