Re: [PATCH 1/2] Remove misleading comment in do_fallocate()

[Rohan Puri <rohan.puri15@gmail.com>]

On Fri, Jul 18, 2014 at 4:47 PM, Lukáš Czerner <lczerner@redhat.com> wrote:
> On Fri, 18 Jul 2014, Rohan Puri wrote:
>
>> Date: Fri, 18 Jul 2014 16:13:21 +0530
>> From: Rohan Puri <rohan.puri15@gmail.com>
>> To: Lukáš Czerner <lczerner@redhat.com>
>> Cc: hch@infradead.org, viro@zeniv.linux.org.uk,
>>     Linux FS Devel <linux-fsdevel@vger.kernel.org>
>> Subject: Re: [PATCH 1/2] Remove misleading comment in do_fallocate()
>>
>> On Fri, Jul 18, 2014 at 4:05 PM, Lukáš Czerner <lczerner@redhat.com> wrote:
>>
>> > On Fri, 18 Jul 2014, Rohan Puri wrote:
>> >
>> > > Date: Fri, 18 Jul 2014 15:27:26 +0530
>> > > From: Rohan Puri <rohan.puri15@gmail.com>
>> > > To: hch@infradead.org, viro@zeniv.linux.org.uk
>> > > Cc: linux-fsdevel@vger.kernel.org
>> > > 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@gmail.com>
>> > > ---
>> > >  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.
sounds good. Thanks Lukas for clarification :)
>
> -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
>>

- Rohan
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html