mbox series

[RESEND,0/1] Use inode_lock/unlock class of provided APIs in filesystems

Message ID 20200101105248.25304-1-riteshh@linux.ibm.com (mailing list archive)
Headers show
Series Use inode_lock/unlock class of provided APIs in filesystems | expand

Message

Ritesh Harjani Jan. 1, 2020, 10:52 a.m. UTC
Al, any comments?
Resending this after adding Reviewed-by/Acked-by tags.


From previous version:-
Matthew Wilcox in [1] suggested that it will be a good idea
to define some missing API instead of directly using i_rwsem in
filesystems drivers for lock/unlock/downgrade purposes.

This patch does that work. No functionality change in this patch.

After this there are only lockdep class of APIs at certain places
in filesystems which are directly using i_rwsem and second is XFS,
but it seems to be anyway defining it's own xfs_ilock/iunlock set
of APIs and 'iolock' naming convention for this lock.

[1]: https://www.spinics.net/lists/linux-ext4/msg68689.html

Ritesh Harjani (1):
  fs: Use inode_lock/unlock class of provided APIs in filesystems

 fs/btrfs/delayed-inode.c |  2 +-
 fs/btrfs/ioctl.c         |  4 ++--
 fs/ceph/io.c             | 24 ++++++++++++------------
 fs/nfs/io.c              | 24 ++++++++++++------------
 fs/orangefs/file.c       |  4 ++--
 fs/overlayfs/readdir.c   |  2 +-
 fs/readdir.c             |  4 ++--
 include/linux/fs.h       | 21 +++++++++++++++++++++
 8 files changed, 53 insertions(+), 32 deletions(-)

Comments

Mike Marshall Jan. 2, 2020, 9:01 p.m. UTC | #1
Ritesh -

I just loaded your patch on top of 5.5-rc4 and it looks fine to me
and xfstests :-) ... I pointed ftrace at the orangefs function you
modified while xfstests was running, and it got called about a
jillion times...

-Mike

On Wed, Jan 1, 2020 at 5:53 AM Ritesh Harjani <riteshh@linux.ibm.com> wrote:
>
> Al, any comments?
> Resending this after adding Reviewed-by/Acked-by tags.
>
>
> From previous version:-
> Matthew Wilcox in [1] suggested that it will be a good idea
> to define some missing API instead of directly using i_rwsem in
> filesystems drivers for lock/unlock/downgrade purposes.
>
> This patch does that work. No functionality change in this patch.
>
> After this there are only lockdep class of APIs at certain places
> in filesystems which are directly using i_rwsem and second is XFS,
> but it seems to be anyway defining it's own xfs_ilock/iunlock set
> of APIs and 'iolock' naming convention for this lock.
>
> [1]: https://www.spinics.net/lists/linux-ext4/msg68689.html
>
> Ritesh Harjani (1):
>   fs: Use inode_lock/unlock class of provided APIs in filesystems
>
>  fs/btrfs/delayed-inode.c |  2 +-
>  fs/btrfs/ioctl.c         |  4 ++--
>  fs/ceph/io.c             | 24 ++++++++++++------------
>  fs/nfs/io.c              | 24 ++++++++++++------------
>  fs/orangefs/file.c       |  4 ++--
>  fs/overlayfs/readdir.c   |  2 +-
>  fs/readdir.c             |  4 ++--
>  include/linux/fs.h       | 21 +++++++++++++++++++++
>  8 files changed, 53 insertions(+), 32 deletions(-)
>
> --
> 2.21.0
>
Ritesh Harjani Jan. 8, 2020, 8:38 a.m. UTC | #2
On 1/3/20 2:31 AM, Mike Marshall wrote:
> Ritesh -
> 
> I just loaded your patch on top of 5.5-rc4 and it looks fine to me
> and xfstests :-) ... I pointed ftrace at the orangefs function you
> modified while xfstests was running, and it got called about a
> jillion times...

Thanks Mike for testing this. Shall I add your Tested-by?

-ritesh


> 
> -Mike
> 
> On Wed, Jan 1, 2020 at 5:53 AM Ritesh Harjani <riteshh@linux.ibm.com> wrote:
>>
>> Al, any comments?
>> Resending this after adding Reviewed-by/Acked-by tags.
>>
>>
>>  From previous version:-
>> Matthew Wilcox in [1] suggested that it will be a good idea
>> to define some missing API instead of directly using i_rwsem in
>> filesystems drivers for lock/unlock/downgrade purposes.
>>
>> This patch does that work. No functionality change in this patch.
>>
>> After this there are only lockdep class of APIs at certain places
>> in filesystems which are directly using i_rwsem and second is XFS,
>> but it seems to be anyway defining it's own xfs_ilock/iunlock set
>> of APIs and 'iolock' naming convention for this lock.
>>
>> [1]: https://www.spinics.net/lists/linux-ext4/msg68689.html
>>
>> Ritesh Harjani (1):
>>    fs: Use inode_lock/unlock class of provided APIs in filesystems
>>
>>   fs/btrfs/delayed-inode.c |  2 +-
>>   fs/btrfs/ioctl.c         |  4 ++--
>>   fs/ceph/io.c             | 24 ++++++++++++------------
>>   fs/nfs/io.c              | 24 ++++++++++++------------
>>   fs/orangefs/file.c       |  4 ++--
>>   fs/overlayfs/readdir.c   |  2 +-
>>   fs/readdir.c             |  4 ++--
>>   include/linux/fs.h       | 21 +++++++++++++++++++++
>>   8 files changed, 53 insertions(+), 32 deletions(-)
>>
>> --
>> 2.21.0
>>
Mike Marshall Jan. 8, 2020, 1:01 p.m. UTC | #3
Yes, please do.

-Mike

On Wed, Jan 8, 2020 at 3:38 AM Ritesh Harjani <riteshh@linux.ibm.com> wrote:
>
>
>
> On 1/3/20 2:31 AM, Mike Marshall wrote:
> > Ritesh -
> >
> > I just loaded your patch on top of 5.5-rc4 and it looks fine to me
> > and xfstests :-) ... I pointed ftrace at the orangefs function you
> > modified while xfstests was running, and it got called about a
> > jillion times...
>
> Thanks Mike for testing this. Shall I add your Tested-by?
>
> -ritesh
>
>
> >
> > -Mike
> >
> > On Wed, Jan 1, 2020 at 5:53 AM Ritesh Harjani <riteshh@linux.ibm.com> wrote:
> >>
> >> Al, any comments?
> >> Resending this after adding Reviewed-by/Acked-by tags.
> >>
> >>
> >>  From previous version:-
> >> Matthew Wilcox in [1] suggested that it will be a good idea
> >> to define some missing API instead of directly using i_rwsem in
> >> filesystems drivers for lock/unlock/downgrade purposes.
> >>
> >> This patch does that work. No functionality change in this patch.
> >>
> >> After this there are only lockdep class of APIs at certain places
> >> in filesystems which are directly using i_rwsem and second is XFS,
> >> but it seems to be anyway defining it's own xfs_ilock/iunlock set
> >> of APIs and 'iolock' naming convention for this lock.
> >>
> >> [1]: https://www.spinics.net/lists/linux-ext4/msg68689.html
> >>
> >> Ritesh Harjani (1):
> >>    fs: Use inode_lock/unlock class of provided APIs in filesystems
> >>
> >>   fs/btrfs/delayed-inode.c |  2 +-
> >>   fs/btrfs/ioctl.c         |  4 ++--
> >>   fs/ceph/io.c             | 24 ++++++++++++------------
> >>   fs/nfs/io.c              | 24 ++++++++++++------------
> >>   fs/orangefs/file.c       |  4 ++--
> >>   fs/overlayfs/readdir.c   |  2 +-
> >>   fs/readdir.c             |  4 ++--
> >>   include/linux/fs.h       | 21 +++++++++++++++++++++
> >>   8 files changed, 53 insertions(+), 32 deletions(-)
> >>
> >> --
> >> 2.21.0
> >>
>
Al Viro May 1, 2020, 4:37 a.m. UTC | #4
On Wed, Jan 01, 2020 at 04:22:47PM +0530, Ritesh Harjani wrote:
> Al, any comments?
> Resending this after adding Reviewed-by/Acked-by tags.

.... argh.  My apologies - that got fallen through the cracks.
Could you rebase and resend it?
Ritesh Harjani May 1, 2020, 6:37 a.m. UTC | #5
On 5/1/20 10:07 AM, Al Viro wrote:
> On Wed, Jan 01, 2020 at 04:22:47PM +0530, Ritesh Harjani wrote:
>> Al, any comments?
>> Resending this after adding Reviewed-by/Acked-by tags.
> 
> .... argh.  My apologies - that got fallen through the cracks.
> Could you rebase and resend it?
> 
Np.
Sure, will rebase and resend.

-ritesh