| Message ID | 20200517214718.468-9-guoqing.jiang@cloud.ionos.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce attach/detach_page_private to cleanup code | expand |
I apologize for not mentioning that I ran this patch set through orangefs xfstests at 5.7 rc5 with no problems or regressions. -Mike On Sun, May 17, 2020 at 5:47 PM Guoqing Jiang <guoqing.jiang@cloud.ionos.com> wrote: > > Since the new pair function is introduced, we can call them to clean the > code in orangefs. > > Cc: Mike Marshall <hubcap@omnibond.com> > Cc: Martin Brandenburg <martin@omnibond.com> > Cc: devel@lists.orangefs.org > Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com> > --- > No change since RFC V3. > > RFC V2 -> RFC V3 > 1. rename clear_page_private to detach_page_private. > > RFC -> RFC V2 > 1. change the name of new functions to attach/clear_page_private. > 2. avoid potential use-after-free as suggested by Dave Chinner. > > fs/orangefs/inode.c | 32 ++++++-------------------------- > 1 file changed, 6 insertions(+), 26 deletions(-) > > diff --git a/fs/orangefs/inode.c b/fs/orangefs/inode.c > index 12ae630fbed7..48f0547d4850 100644 > --- a/fs/orangefs/inode.c > +++ b/fs/orangefs/inode.c > @@ -62,12 +62,7 @@ static int orangefs_writepage_locked(struct page *page, > } else { > ret = 0; > } > - if (wr) { > - kfree(wr); > - set_page_private(page, 0); > - ClearPagePrivate(page); > - put_page(page); > - } > + kfree(detach_page_private(page)); > return ret; > } > > @@ -409,9 +404,7 @@ static int orangefs_write_begin(struct file *file, > wr->len = len; > wr->uid = current_fsuid(); > wr->gid = current_fsgid(); > - SetPagePrivate(page); > - set_page_private(page, (unsigned long)wr); > - get_page(page); > + attach_page_private(page, wr); > okay: > return 0; > } > @@ -459,18 +452,12 @@ static void orangefs_invalidatepage(struct page *page, > wr = (struct orangefs_write_range *)page_private(page); > > if (offset == 0 && length == PAGE_SIZE) { > - kfree((struct orangefs_write_range *)page_private(page)); > - set_page_private(page, 0); > - ClearPagePrivate(page); > - put_page(page); > + kfree(detach_page_private(page)); > return; > /* write range entirely within invalidate range (or equal) */ > } else if (page_offset(page) + offset <= wr->pos && > wr->pos + wr->len <= page_offset(page) + offset + length) { > - kfree((struct orangefs_write_range *)page_private(page)); > - set_page_private(page, 0); > - ClearPagePrivate(page); > - put_page(page); > + kfree(detach_page_private(page)); > /* XXX is this right? only caller in fs */ > cancel_dirty_page(page); > return; > @@ -535,12 +522,7 @@ static int orangefs_releasepage(struct page *page, gfp_t foo) > > static void orangefs_freepage(struct page *page) > { > - if (PagePrivate(page)) { > - kfree((struct orangefs_write_range *)page_private(page)); > - set_page_private(page, 0); > - ClearPagePrivate(page); > - put_page(page); > - } > + kfree(detach_page_private(page)); > } > > static int orangefs_launder_page(struct page *page) > @@ -740,9 +722,7 @@ vm_fault_t orangefs_page_mkwrite(struct vm_fault *vmf) > wr->len = PAGE_SIZE; > wr->uid = current_fsuid(); > wr->gid = current_fsgid(); > - SetPagePrivate(page); > - set_page_private(page, (unsigned long)wr); > - get_page(page); > + attach_page_private(page, wr); > okay: > > file_update_time(vmf->vma->vm_file); > -- > 2.17.1 >
On 5/26/20 11:54 PM, Mike Marshall wrote: > I apologize for not mentioning that I ran this patch set > through orangefs xfstests at 5.7 rc5 with no problems > or regressions. Glad to hear that, thanks for your effort. Thanks, Guoqing
diff --git a/fs/orangefs/inode.c b/fs/orangefs/inode.c index 12ae630fbed7..48f0547d4850 100644 --- a/fs/orangefs/inode.c +++ b/fs/orangefs/inode.c @@ -62,12 +62,7 @@ static int orangefs_writepage_locked(struct page *page, } else { ret = 0; } - if (wr) { - kfree(wr); - set_page_private(page, 0); - ClearPagePrivate(page); - put_page(page); - } + kfree(detach_page_private(page)); return ret; } @@ -409,9 +404,7 @@ static int orangefs_write_begin(struct file *file, wr->len = len; wr->uid = current_fsuid(); wr->gid = current_fsgid(); - SetPagePrivate(page); - set_page_private(page, (unsigned long)wr); - get_page(page); + attach_page_private(page, wr); okay: return 0; } @@ -459,18 +452,12 @@ static void orangefs_invalidatepage(struct page *page, wr = (struct orangefs_write_range *)page_private(page); if (offset == 0 && length == PAGE_SIZE) { - kfree((struct orangefs_write_range *)page_private(page)); - set_page_private(page, 0); - ClearPagePrivate(page); - put_page(page); + kfree(detach_page_private(page)); return; /* write range entirely within invalidate range (or equal) */ } else if (page_offset(page) + offset <= wr->pos && wr->pos + wr->len <= page_offset(page) + offset + length) { - kfree((struct orangefs_write_range *)page_private(page)); - set_page_private(page, 0); - ClearPagePrivate(page); - put_page(page); + kfree(detach_page_private(page)); /* XXX is this right? only caller in fs */ cancel_dirty_page(page); return; @@ -535,12 +522,7 @@ static int orangefs_releasepage(struct page *page, gfp_t foo) static void orangefs_freepage(struct page *page) { - if (PagePrivate(page)) { - kfree((struct orangefs_write_range *)page_private(page)); - set_page_private(page, 0); - ClearPagePrivate(page); - put_page(page); - } + kfree(detach_page_private(page)); } static int orangefs_launder_page(struct page *page) @@ -740,9 +722,7 @@ vm_fault_t orangefs_page_mkwrite(struct vm_fault *vmf) wr->len = PAGE_SIZE; wr->uid = current_fsuid(); wr->gid = current_fsgid(); - SetPagePrivate(page); - set_page_private(page, (unsigned long)wr); - get_page(page); + attach_page_private(page, wr); okay: file_update_time(vmf->vma->vm_file);
Since the new pair function is introduced, we can call them to clean the code in orangefs. Cc: Mike Marshall <hubcap@omnibond.com> Cc: Martin Brandenburg <martin@omnibond.com> Cc: devel@lists.orangefs.org Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com> --- No change since RFC V3. RFC V2 -> RFC V3 1. rename clear_page_private to detach_page_private. RFC -> RFC V2 1. change the name of new functions to attach/clear_page_private. 2. avoid potential use-after-free as suggested by Dave Chinner. fs/orangefs/inode.c | 32 ++++++-------------------------- 1 file changed, 6 insertions(+), 26 deletions(-)