Re: [f2fs-dev] [PATCH 2/3] ext4: fix inode rwsem regression

[Theodore Y. Ts'o]

On Mon, Sep 23, 2019 at 12:10:42PM +0200, Jan Kara wrote:
> On Wed 11-09-19 11:45:16, Goldwyn Rodrigues wrote:
> > From: Goldwyn Rodrigues 
> > 
> > This is similar to 942491c9e6d6 ("xfs: fix AIM7 regression")
> > Apparently our current rwsem code doesn't like doing the trylock, then
> > lock for real scheme.  So change our read/write methods to just do the
> > trylock for the RWF_NOWAIT case.
> > 
> > Fixes: 728fbc0e10b7 ("ext4: nowait aio support")
> > Signed-off-by: Goldwyn Rodrigues 
> 
> Thanks for fixing this! The patch looks good to me. You can add:
> 
> Reviewed-by: Jan Kara 
> 
> BTW, I've also added Ted as ext4 maintainer to CC.

Thanks, I've been following along, and once the merge window is over
I'll start going through the patch backlog.

Cheers,

- Ted


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 2/3] ext4: fix inode rwsem regression

[Jan Kara]

On Wed 11-09-19 11:45:16, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues 
> 
> This is similar to 942491c9e6d6 ("xfs: fix AIM7 regression")
> Apparently our current rwsem code doesn't like doing the trylock, then
> lock for real scheme.  So change our read/write methods to just do the
> trylock for the RWF_NOWAIT case.
> 
> Fixes: 728fbc0e10b7 ("ext4: nowait aio support")
> Signed-off-by: Goldwyn Rodrigues 

Thanks for fixing this! The patch looks good to me. You can add:

Reviewed-by: Jan Kara 

BTW, I've also added Ted as ext4 maintainer to CC.

Honza

> ---
>  fs/ext4/file.c | 17 +++--
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index 70b0438dbc94..d5b2d0cc325d 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -40,11 +40,13 @@ static ssize_t ext4_dax_read_iter(struct kiocb *iocb, 
> struct iov_iter *to)
>   struct inode *inode = file_inode(iocb->ki_filp);
>   ssize_t ret;
>  
> - if (!inode_trylock_shared(inode)) {
> - if (iocb->ki_flags & IOCB_NOWAIT)
> + if (iocb->ki_flags & IOCB_NOWAIT) {
> + if (!inode_trylock_shared(inode))
>   return -EAGAIN;
> + } else {
>   inode_lock_shared(inode);
>   }
> +
>   /*
>* Recheck under inode lock - at this point we are sure it cannot
>* change anymore
> @@ -190,11 +192,13 @@ ext4_dax_write_iter(struct kiocb *iocb, struct iov_iter 
> *from)
>   struct inode *inode = file_inode(iocb->ki_filp);
>   ssize_t ret;
>  
> - if (!inode_trylock(inode)) {
> - if (iocb->ki_flags & IOCB_NOWAIT)
> + if (iocb->ki_flags & IOCB_NOWAIT) {
> + if (!inode_trylock(inode))
>   return -EAGAIN;
> + } else {
>   inode_lock(inode);
>   }
> +
>   ret = ext4_write_checks(iocb, from);
>   if (ret <= 0)
>   goto out;
> @@ -233,9 +237,10 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter 
> *from)
>   if (!o_direct && (iocb->ki_flags & IOCB_NOWAIT))
>   return -EOPNOTSUPP;
>  
> - if (!inode_trylock(inode)) {
> - if (iocb->ki_flags & IOCB_NOWAIT)
> + if (iocb->ki_flags & IOCB_NOWAIT) {
> + if (!inode_trylock(inode))
>   return -EAGAIN;
> + } else {
>   inode_lock(inode);
>   }
>  
> -- 
> 2.16.4
> 
-- 
Jan Kara 
SUSE Labs, CR


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 2/3] ext4: fix inode rwsem regression

[Matthew Bobrowski]

On Thu, Sep 12, 2019 at 02:22:35PM +0530, Ritesh Harjani wrote:
> cc'd Matthew as well.
> 
> > This is similar to 942491c9e6d6 ("xfs: fix AIM7 regression")
> > Apparently our current rwsem code doesn't like doing the trylock, then
> > lock for real scheme.  So change our read/write methods to just do the
> > trylock for the RWF_NOWAIT case.
> > 
> > Fixes: 728fbc0e10b7 ("ext4: nowait aio support")
> > Signed-off-by: Goldwyn Rodrigues 
> 
> This patch will conflict with recent iomap patch series.
> So if this is getting queued up before, so iomap patch series will
> need to rebase and factor these changes in the new APIs.

Noted. I've been keeping my eye on this thread, so I'm aware of this.




___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 2/3] ext4: fix inode rwsem regression

[Ritesh Harjani]

cc'd Matthew as well.

On 9/11/19 10:15 PM, Goldwyn Rodrigues wrote:

From: Goldwyn Rodrigues 

This is similar to 942491c9e6d6 ("xfs: fix AIM7 regression")
Apparently our current rwsem code doesn't like doing the trylock, then
lock for real scheme.  So change our read/write methods to just do the
trylock for the RWF_NOWAIT case.

Fixes: 728fbc0e10b7 ("ext4: nowait aio support")
Signed-off-by: Goldwyn Rodrigues 


This patch will conflict with recent iomap patch series.
So if this is getting queued up before, so iomap patch series will
need to rebase and factor these changes in the new APIs.

Otherwise looks good to me!

Reviewed-by: Ritesh Harjani 



---
  fs/ext4/file.c | 17 +++--
  1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 70b0438dbc94..d5b2d0cc325d 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -40,11 +40,13 @@ static ssize_t ext4_dax_read_iter(struct kiocb *iocb, 
struct iov_iter *to)
struct inode *inode = file_inode(iocb->ki_filp);
ssize_t ret;
  
-	if (!inode_trylock_shared(inode)) {

-   if (iocb->ki_flags & IOCB_NOWAIT)
+   if (iocb->ki_flags & IOCB_NOWAIT) {
+   if (!inode_trylock_shared(inode))
return -EAGAIN;
+   } else {
inode_lock_shared(inode);
}
+
/*
 * Recheck under inode lock - at this point we are sure it cannot
 * change anymore
@@ -190,11 +192,13 @@ ext4_dax_write_iter(struct kiocb *iocb, struct iov_iter 
*from)
struct inode *inode = file_inode(iocb->ki_filp);
ssize_t ret;
  
-	if (!inode_trylock(inode)) {

-   if (iocb->ki_flags & IOCB_NOWAIT)
+   if (iocb->ki_flags & IOCB_NOWAIT) {
+   if (!inode_trylock(inode))
return -EAGAIN;
+   } else {
inode_lock(inode);
}
+
ret = ext4_write_checks(iocb, from);
if (ret <= 0)
goto out;
@@ -233,9 +237,10 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter 
*from)
if (!o_direct && (iocb->ki_flags & IOCB_NOWAIT))
return -EOPNOTSUPP;
  
-	if (!inode_trylock(inode)) {

-   if (iocb->ki_flags & IOCB_NOWAIT)
+   if (iocb->ki_flags & IOCB_NOWAIT) {
+   if (!inode_trylock(inode))
return -EAGAIN;
+   } else {
inode_lock(inode);
}
  





___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH 2/3] ext4: fix inode rwsem regression

[Goldwyn Rodrigues]

From: Goldwyn Rodrigues 

This is similar to 942491c9e6d6 ("xfs: fix AIM7 regression")
Apparently our current rwsem code doesn't like doing the trylock, then
lock for real scheme.  So change our read/write methods to just do the
trylock for the RWF_NOWAIT case.

Fixes: 728fbc0e10b7 ("ext4: nowait aio support")
Signed-off-by: Goldwyn Rodrigues 
---
 fs/ext4/file.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 70b0438dbc94..d5b2d0cc325d 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -40,11 +40,13 @@ static ssize_t ext4_dax_read_iter(struct kiocb *iocb, 
struct iov_iter *to)
struct inode *inode = file_inode(iocb->ki_filp);
ssize_t ret;
 
-   if (!inode_trylock_shared(inode)) {
-   if (iocb->ki_flags & IOCB_NOWAIT)
+   if (iocb->ki_flags & IOCB_NOWAIT) {
+   if (!inode_trylock_shared(inode))
return -EAGAIN;
+   } else {
inode_lock_shared(inode);
}
+
/*
 * Recheck under inode lock - at this point we are sure it cannot
 * change anymore
@@ -190,11 +192,13 @@ ext4_dax_write_iter(struct kiocb *iocb, struct iov_iter 
*from)
struct inode *inode = file_inode(iocb->ki_filp);
ssize_t ret;
 
-   if (!inode_trylock(inode)) {
-   if (iocb->ki_flags & IOCB_NOWAIT)
+   if (iocb->ki_flags & IOCB_NOWAIT) {
+   if (!inode_trylock(inode))
return -EAGAIN;
+   } else {
inode_lock(inode);
}
+
ret = ext4_write_checks(iocb, from);
if (ret <= 0)
goto out;
@@ -233,9 +237,10 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter 
*from)
if (!o_direct && (iocb->ki_flags & IOCB_NOWAIT))
return -EOPNOTSUPP;
 
-   if (!inode_trylock(inode)) {
-   if (iocb->ki_flags & IOCB_NOWAIT)
+   if (iocb->ki_flags & IOCB_NOWAIT) {
+   if (!inode_trylock(inode))
return -EAGAIN;
+   } else {
inode_lock(inode);
}
 
-- 
2.16.4



___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel