Re: [PATCH] nfs:Fix deadlock occurrence issue on error path in the function nfs_vm_page_mkwrite
On Sat, Aug 22, 2015 at 9:17 PM, Nicholas Krause wrote: > > This fixes a possible deadlock occurrence issue on the error path > in the function nfs_vm_page_mkwrite for checking if the calls to > the functions nfs_flus_incompatible and nfs_updatepage have failed > by incorrectly not jumping to the correct goto label out_unlock > for unlocking the locked structure page pointer thus causing a > deadlock due to not correctly unlocking the internal structure > page pointer used by the function nfs_vm_page_mkwrite that had > been previously locked with the function lock_page. Further more > we also remove the goto label out as no error paths in this function > now use this label and this label should be removed to avoid build > warnings related to having a undefined label. > > Signed-off-by: Nicholas Krause > --- > fs/nfs/file.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/fs/nfs/file.c b/fs/nfs/file.c > index cc4fa1e..fe68ecc 100644 > --- a/fs/nfs/file.c > +++ b/fs/nfs/file.c > @@ -629,12 +629,11 @@ static int nfs_vm_page_mkwrite(struct vm_area_struct > *vma, struct vm_fault *vmf) > ret = VM_FAULT_LOCKED; > if (nfs_flush_incompatible(filp, page) == 0 && > nfs_updatepage(filp, page, 0, pagelen) == 0) > - goto out; > + goto out_unlock; > > ret = VM_FAULT_SIGBUS; > out_unlock: > unlock_page(page); > -out: > return ret; > } > > That's the success path that you are modifying, not the error path. Furthermore, the return value is still VM_FAULT_LOCKED, which means the caller expects the page to be returned in the locked state. So NACK Trond -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] nfs:Fix deadlock occurrence issue on error path in the function nfs_vm_page_mkwrite
On Sat, Aug 22, 2015 at 9:17 PM, Nicholas Krause xerofo...@gmail.com wrote: This fixes a possible deadlock occurrence issue on the error path in the function nfs_vm_page_mkwrite for checking if the calls to the functions nfs_flus_incompatible and nfs_updatepage have failed by incorrectly not jumping to the correct goto label out_unlock for unlocking the locked structure page pointer thus causing a deadlock due to not correctly unlocking the internal structure page pointer used by the function nfs_vm_page_mkwrite that had been previously locked with the function lock_page. Further more we also remove the goto label out as no error paths in this function now use this label and this label should be removed to avoid build warnings related to having a undefined label. Signed-off-by: Nicholas Krause xerofo...@gmail.com --- fs/nfs/file.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/nfs/file.c b/fs/nfs/file.c index cc4fa1e..fe68ecc 100644 --- a/fs/nfs/file.c +++ b/fs/nfs/file.c @@ -629,12 +629,11 @@ static int nfs_vm_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf) ret = VM_FAULT_LOCKED; if (nfs_flush_incompatible(filp, page) == 0 nfs_updatepage(filp, page, 0, pagelen) == 0) - goto out; + goto out_unlock; ret = VM_FAULT_SIGBUS; out_unlock: unlock_page(page); -out: return ret; } That's the success path that you are modifying, not the error path. Furthermore, the return value is still VM_FAULT_LOCKED, which means the caller expects the page to be returned in the locked state. So NACK Trond -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
