All of lore.kernel.org
 help / color / mirror / Atom feed
* [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.