* [PATCH 0/2] fallocate(2) related cleanups
@ 2014-07-18 9:50 Rohan Puri
2014-07-18 9:57 ` [PATCH 1/2] Remove misleading comment in do_fallocate() Rohan Puri
2014-07-18 9:58 ` [PATCH 2/2] Remove redundant inode mode checks for ceph & ext4 Rohan Puri
0 siblings, 2 replies; 11+ messages in thread
From: Rohan Puri @ 2014-07-18 9:50 UTC (permalink / raw)
To: rohan.puri15; +Cc: linux-fsdevel
This patch series contains fallocate(2) related cleanups
Rohan Puri (2):
Remove misleading comment in do_fallocate()
Remove redundant inode mode checks for ceph & ext4
fs/ceph/file.c | 3 ---
fs/ext4/inode.c | 3 ---
fs/open.c | 4 ----
3 files changed, 10 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] Remove misleading comment in do_fallocate()
2014-07-18 9:50 [PATCH 0/2] fallocate(2) related cleanups Rohan Puri
@ 2014-07-18 9:57 ` Rohan Puri
2014-07-18 10:35 ` Lukáš Czerner
2014-07-18 9:58 ` [PATCH 2/2] Remove redundant inode mode checks for ceph & ext4 Rohan Puri
1 sibling, 1 reply; 11+ messages in thread
From: Rohan Puri @ 2014-07-18 9:57 UTC (permalink / raw)
To: hch, viro; +Cc: linux-fsdevel
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.
- */
if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
return -ENODEV;
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] Remove redundant inode mode checks for ceph & ext4
2014-07-18 9:50 [PATCH 0/2] fallocate(2) related cleanups Rohan Puri
2014-07-18 9:57 ` [PATCH 1/2] Remove misleading comment in do_fallocate() Rohan Puri
@ 2014-07-18 9:58 ` Rohan Puri
2014-07-18 10:32 ` Lukáš Czerner
1 sibling, 1 reply; 11+ messages in thread
From: Rohan Puri @ 2014-07-18 9:58 UTC (permalink / raw)
To: hch, viro; +Cc: linux-fsdevel
These checks are not needed here, since they are already performed
by do_fallocate()
Signed-off-by: Rohan Puri <rohan.puri15@gmail.com>
---
fs/ceph/file.c | 3 ---
fs/ext4/inode.c | 3 ---
2 files changed, 6 deletions(-)
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 88a6df4..687a72e 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -1218,9 +1218,6 @@ static long ceph_fallocate(struct file *file, int mode,
loff_t endoff = 0;
loff_t size;
- if (!S_ISREG(inode->i_mode))
- return -EOPNOTSUPP;
-
mutex_lock(&inode->i_mutex);
if (ceph_snap(inode) != CEPH_NOSNAP) {
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 7fcd68e..22c9868 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3532,9 +3532,6 @@ int ext4_punch_hole(struct inode *inode, loff_t offset, loff_t length)
unsigned int credits;
int ret = 0;
- if (!S_ISREG(inode->i_mode))
- return -EOPNOTSUPP;
-
trace_ext4_punch_hole(inode, offset, length, 0);
/*
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Remove redundant inode mode checks for ceph & ext4
2014-07-18 9:58 ` [PATCH 2/2] Remove redundant inode mode checks for ceph & ext4 Rohan Puri
@ 2014-07-18 10:32 ` Lukáš Czerner
2014-07-18 10:46 ` Rohan Puri
0 siblings, 1 reply; 11+ messages in thread
From: Lukáš Czerner @ 2014-07-18 10:32 UTC (permalink / raw)
To: Rohan Puri; +Cc: hch, viro, linux-fsdevel
On Fri, 18 Jul 2014, Rohan Puri wrote:
> Date: Fri, 18 Jul 2014 15:28:38 +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 2/2] Remove redundant inode mode checks for ceph & ext4
>
> These checks are not needed here, since they are already performed
> by do_fallocate()
NAK, they are actually needed because we do not support fallocate on
directories here.
-Lukas
>
> Signed-off-by: Rohan Puri <rohan.puri15@gmail.com>
> ---
> fs/ceph/file.c | 3 ---
> fs/ext4/inode.c | 3 ---
> 2 files changed, 6 deletions(-)
>
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index 88a6df4..687a72e 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -1218,9 +1218,6 @@ static long ceph_fallocate(struct file *file, int mode,
> loff_t endoff = 0;
> loff_t size;
>
> - if (!S_ISREG(inode->i_mode))
> - return -EOPNOTSUPP;
> -
> mutex_lock(&inode->i_mutex);
>
> if (ceph_snap(inode) != CEPH_NOSNAP) {
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 7fcd68e..22c9868 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -3532,9 +3532,6 @@ int ext4_punch_hole(struct inode *inode, loff_t offset, loff_t length)
> unsigned int credits;
> int ret = 0;
>
> - if (!S_ISREG(inode->i_mode))
> - return -EOPNOTSUPP;
> -
> trace_ext4_punch_hole(inode, offset, length, 0);
>
> /*
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Remove misleading comment in do_fallocate()
2014-07-18 9:57 ` [PATCH 1/2] Remove misleading comment in do_fallocate() Rohan Puri
@ 2014-07-18 10:35 ` Lukáš Czerner
[not found] ` <CALJfu6Ok_jdNqacsEKnnVvzxSxCuRC7dA0abve28BASaAgB31A@mail.gmail.com>
0 siblings, 1 reply; 11+ messages in thread
From: Lukáš Czerner @ 2014-07-18 10:35 UTC (permalink / raw)
To: Rohan Puri; +Cc: hch, viro, linux-fsdevel
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.
-Lukas
> if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
> return -ENODEV;
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Remove redundant inode mode checks for ceph & ext4
2014-07-18 10:32 ` Lukáš Czerner
@ 2014-07-18 10:46 ` Rohan Puri
2014-07-18 11:18 ` Lukáš Czerner
0 siblings, 1 reply; 11+ messages in thread
From: Rohan Puri @ 2014-07-18 10:46 UTC (permalink / raw)
To: Lukáš Czerner; +Cc: hch, viro, Linux FS Devel
On Fri, Jul 18, 2014 at 4:02 PM, Lukáš Czerner <lczerner@redhat.com> wrote:
> On Fri, 18 Jul 2014, Rohan Puri wrote:
>
>> Date: Fri, 18 Jul 2014 15:28:38 +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 2/2] Remove redundant inode mode checks for ceph & ext4
>>
>> These checks are not needed here, since they are already performed
>> by do_fallocate()
>
> NAK, they are actually needed because we do not support fallocate on
> directories here.
>
But we never get to file systems for fallocate in directory case, we
either return from open() itself
or do_fallocate() before calling f_ops->fallocate()
> -Lukas
>
>>
>> Signed-off-by: Rohan Puri <rohan.puri15@gmail.com>
>> ---
>> fs/ceph/file.c | 3 ---
>> fs/ext4/inode.c | 3 ---
>> 2 files changed, 6 deletions(-)
>>
>> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
>> index 88a6df4..687a72e 100644
>> --- a/fs/ceph/file.c
>> +++ b/fs/ceph/file.c
>> @@ -1218,9 +1218,6 @@ static long ceph_fallocate(struct file *file, int mode,
>> loff_t endoff = 0;
>> loff_t size;
>>
>> - if (!S_ISREG(inode->i_mode))
>> - return -EOPNOTSUPP;
>> -
>> mutex_lock(&inode->i_mutex);
>>
>> if (ceph_snap(inode) != CEPH_NOSNAP) {
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index 7fcd68e..22c9868 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -3532,9 +3532,6 @@ int ext4_punch_hole(struct inode *inode, loff_t offset, loff_t length)
>> unsigned int credits;
>> int ret = 0;
>>
>> - if (!S_ISREG(inode->i_mode))
>> - return -EOPNOTSUPP;
>> -
>> trace_ext4_punch_hole(inode, offset, length, 0);
>>
>> /*
>>
--
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Fwd: [PATCH 1/2] Remove misleading comment in do_fallocate()
[not found] ` <CALJfu6Ok_jdNqacsEKnnVvzxSxCuRC7dA0abve28BASaAgB31A@mail.gmail.com>
@ 2014-07-18 10:47 ` Rohan Puri
2014-07-18 11:17 ` Lukáš Czerner
1 sibling, 0 replies; 11+ messages in thread
From: Rohan Puri @ 2014-07-18 10:47 UTC (permalink / raw)
To: Linux FS Devel
Adding fs-devel
---------- Forwarded message ----------
From: Rohan Puri <rohan.puri15@gmail.com>
Date: Fri, Jul 18, 2014 at 4:13 PM
Subject: Re: [PATCH 1/2] Remove misleading comment in do_fallocate()
To: Lukáš Czerner <lczerner@redhat.com>
Cc: hch@infradead.org, viro@zeniv.linux.org.uk, Linux FS Devel
<linux-fsdevel@vger.kernel.org>
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.
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
--
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Remove misleading comment in do_fallocate()
[not found] ` <CALJfu6Ok_jdNqacsEKnnVvzxSxCuRC7dA0abve28BASaAgB31A@mail.gmail.com>
2014-07-18 10:47 ` Fwd: " Rohan Puri
@ 2014-07-18 11:17 ` Lukáš Czerner
2014-07-18 12:32 ` Rohan Puri
1 sibling, 1 reply; 11+ messages in thread
From: Lukáš Czerner @ 2014-07-18 11:17 UTC (permalink / raw)
To: Rohan Puri; +Cc: hch, viro, Linux FS Devel
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2621 bytes --]
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.
-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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Remove redundant inode mode checks for ceph & ext4
2014-07-18 10:46 ` Rohan Puri
@ 2014-07-18 11:18 ` Lukáš Czerner
2014-07-18 12:33 ` Rohan Puri
0 siblings, 1 reply; 11+ messages in thread
From: Lukáš Czerner @ 2014-07-18 11:18 UTC (permalink / raw)
To: Rohan Puri; +Cc: hch, viro, Linux FS Devel
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2382 bytes --]
On Fri, 18 Jul 2014, Rohan Puri wrote:
> Date: Fri, 18 Jul 2014 16:16:26 +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 2/2] Remove redundant inode mode checks for ceph & ext4
>
> On Fri, Jul 18, 2014 at 4:02 PM, Lukáš Czerner <lczerner@redhat.com> wrote:
> > On Fri, 18 Jul 2014, Rohan Puri wrote:
> >
> >> Date: Fri, 18 Jul 2014 15:28:38 +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 2/2] Remove redundant inode mode checks for ceph & ext4
> >>
> >> These checks are not needed here, since they are already performed
> >> by do_fallocate()
> >
> > NAK, they are actually needed because we do not support fallocate on
> > directories here.
> >
> But we never get to file systems for fallocate in directory case, we
> either return from open() itself
> or do_fallocate() before calling f_ops->fallocate()
Yes, but this is future proof for the time that someone adds
fallocate support for directories. Let's leave it here, it really
does not hurt.
Thanks!
-Lukas
> > -Lukas
> >
> >>
> >> Signed-off-by: Rohan Puri <rohan.puri15@gmail.com>
> >> ---
> >> fs/ceph/file.c | 3 ---
> >> fs/ext4/inode.c | 3 ---
> >> 2 files changed, 6 deletions(-)
> >>
> >> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> >> index 88a6df4..687a72e 100644
> >> --- a/fs/ceph/file.c
> >> +++ b/fs/ceph/file.c
> >> @@ -1218,9 +1218,6 @@ static long ceph_fallocate(struct file *file, int mode,
> >> loff_t endoff = 0;
> >> loff_t size;
> >>
> >> - if (!S_ISREG(inode->i_mode))
> >> - return -EOPNOTSUPP;
> >> -
> >> mutex_lock(&inode->i_mutex);
> >>
> >> if (ceph_snap(inode) != CEPH_NOSNAP) {
> >> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> >> index 7fcd68e..22c9868 100644
> >> --- a/fs/ext4/inode.c
> >> +++ b/fs/ext4/inode.c
> >> @@ -3532,9 +3532,6 @@ int ext4_punch_hole(struct inode *inode, loff_t offset, loff_t length)
> >> unsigned int credits;
> >> int ret = 0;
> >>
> >> - if (!S_ISREG(inode->i_mode))
> >> - return -EOPNOTSUPP;
> >> -
> >> trace_ext4_punch_hole(inode, offset, length, 0);
> >>
> >> /*
> >>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Remove misleading comment in do_fallocate()
2014-07-18 11:17 ` Lukáš Czerner
@ 2014-07-18 12:32 ` Rohan Puri
0 siblings, 0 replies; 11+ messages in thread
From: Rohan Puri @ 2014-07-18 12:32 UTC (permalink / raw)
To: Lukáš Czerner; +Cc: hch, viro, Linux FS Devel
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Remove redundant inode mode checks for ceph & ext4
2014-07-18 11:18 ` Lukáš Czerner
@ 2014-07-18 12:33 ` Rohan Puri
0 siblings, 0 replies; 11+ messages in thread
From: Rohan Puri @ 2014-07-18 12:33 UTC (permalink / raw)
To: Lukáš Czerner; +Cc: hch, viro, Linux FS Devel
On Fri, Jul 18, 2014 at 4:48 PM, Lukáš Czerner <lczerner@redhat.com> wrote:
> On Fri, 18 Jul 2014, Rohan Puri wrote:
>
>> Date: Fri, 18 Jul 2014 16:16:26 +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 2/2] Remove redundant inode mode checks for ceph & ext4
>>
>> On Fri, Jul 18, 2014 at 4:02 PM, Lukáš Czerner <lczerner@redhat.com> wrote:
>> > On Fri, 18 Jul 2014, Rohan Puri wrote:
>> >
>> >> Date: Fri, 18 Jul 2014 15:28:38 +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 2/2] Remove redundant inode mode checks for ceph & ext4
>> >>
>> >> These checks are not needed here, since they are already performed
>> >> by do_fallocate()
>> >
>> > NAK, they are actually needed because we do not support fallocate on
>> > directories here.
>> >
>> But we never get to file systems for fallocate in directory case, we
>> either return from open() itself
>> or do_fallocate() before calling f_ops->fallocate()
>
> Yes, but this is future proof for the time that someone adds
> fallocate support for directories. Let's leave it here, it really
> does not hurt.
>
Thanks for clarification :)
> Thanks!
> -Lukas
>
>> > -Lukas
>> >
>> >>
>> >> Signed-off-by: Rohan Puri <rohan.puri15@gmail.com>
>> >> ---
>> >> fs/ceph/file.c | 3 ---
>> >> fs/ext4/inode.c | 3 ---
>> >> 2 files changed, 6 deletions(-)
>> >>
>> >> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
>> >> index 88a6df4..687a72e 100644
>> >> --- a/fs/ceph/file.c
>> >> +++ b/fs/ceph/file.c
>> >> @@ -1218,9 +1218,6 @@ static long ceph_fallocate(struct file *file, int mode,
>> >> loff_t endoff = 0;
>> >> loff_t size;
>> >>
>> >> - if (!S_ISREG(inode->i_mode))
>> >> - return -EOPNOTSUPP;
>> >> -
>> >> mutex_lock(&inode->i_mutex);
>> >>
>> >> if (ceph_snap(inode) != CEPH_NOSNAP) {
>> >> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> >> index 7fcd68e..22c9868 100644
>> >> --- a/fs/ext4/inode.c
>> >> +++ b/fs/ext4/inode.c
>> >> @@ -3532,9 +3532,6 @@ int ext4_punch_hole(struct inode *inode, loff_t offset, loff_t length)
>> >> unsigned int credits;
>> >> int ret = 0;
>> >>
>> >> - if (!S_ISREG(inode->i_mode))
>> >> - return -EOPNOTSUPP;
>> >> -
>> >> trace_ext4_punch_hole(inode, offset, length, 0);
>> >>
>> >> /*
>> >>
>>
- 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
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-07-18 12:33 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-18 9:50 [PATCH 0/2] fallocate(2) related cleanups Rohan Puri
2014-07-18 9:57 ` [PATCH 1/2] Remove misleading comment in do_fallocate() Rohan Puri
2014-07-18 10:35 ` Lukáš Czerner
[not found] ` <CALJfu6Ok_jdNqacsEKnnVvzxSxCuRC7dA0abve28BASaAgB31A@mail.gmail.com>
2014-07-18 10:47 ` Fwd: " Rohan Puri
2014-07-18 11:17 ` Lukáš Czerner
2014-07-18 12:32 ` Rohan Puri
2014-07-18 9:58 ` [PATCH 2/2] Remove redundant inode mode checks for ceph & ext4 Rohan Puri
2014-07-18 10:32 ` Lukáš Czerner
2014-07-18 10:46 ` Rohan Puri
2014-07-18 11:18 ` Lukáš Czerner
2014-07-18 12:33 ` Rohan Puri
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.