Jeff Layton <jlayton@xxxxxxxxxx> writes:
> The only caller locks the page and then has to unlock it again once it
> returns, just have ceph_uninline_data do that itself. Also, in the case
> where we are allocating a fresh page for this, lock it to help simplify
> the code a bit.
>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> fs/ceph/addr.c | 25 +++++++++----------------
> 1 file changed, 9 insertions(+), 16 deletions(-)
>
> diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
> index 859d2cbfeccb..5f1e2b6577fb 100644
> --- a/fs/ceph/addr.c
> +++ b/fs/ceph/addr.c
> @@ -1542,14 +1542,7 @@ static vm_fault_t ceph_page_mkwrite(struct vm_fault *vmf)
> ceph_block_sigs(&oldset);
>
> if (ci->i_inline_version != CEPH_INLINE_NONE) {
> - struct page *locked_page = NULL;
> - if (off == 0) {
> - lock_page(page);
> - locked_page = page;
> - }
> - err = ceph_uninline_data(inode, locked_page);
> - if (locked_page)
> - unlock_page(locked_page);
> + err = ceph_uninline_data(inode, off == 0 ? page : NULL);
> if (err < 0)
> goto out_free;
> }
> @@ -1663,12 +1656,11 @@ void ceph_fill_inline_data(struct inode *inode, struct page *locked_page,
> }
> }
>
> -int ceph_uninline_data(struct inode *inode, struct page *locked_page)
> +int ceph_uninline_data(struct inode *inode, struct page *page)
> {
> struct ceph_inode_info *ci = ceph_inode(inode);
> struct ceph_fs_client *fsc = ceph_inode_to_client(inode);
> struct ceph_osd_request *req;
> - struct page *page = NULL;
> u64 len, inline_version;
> int err = 0;
> bool from_pagecache = false;
> @@ -1684,8 +1676,8 @@ int ceph_uninline_data(struct inode *inode, struct page *locked_page)
> inline_version == CEPH_INLINE_NONE)
> goto out;
>
> - if (locked_page) {
> - page = locked_page;
> + if (page) {
> + lock_page(page);
> WARN_ON(!PageUptodate(page));
> } else if (ceph_caps_issued(ci) &
> (CEPH_CAP_FILE_CACHE|CEPH_CAP_FILE_LAZYIO)) {
> @@ -1711,6 +1703,7 @@ int ceph_uninline_data(struct inode *inode, struct page *locked_page)
> err = -ENOMEM;
> goto out;
> }
> + lock_page(page);
> err = __ceph_do_getattr(inode, page,
> CEPH_STAT_CAP_INLINE_DATA, true);
> if (err < 0) {
> @@ -1782,11 +1775,11 @@ int ceph_uninline_data(struct inode *inode, struct page *locked_page)
> if (err == -ECANCELED)
> err = 0;
> out:
> - if (page && page != locked_page) {
> - if (from_pagecache) {
> - unlock_page(page);
> + if (page) {
> + unlock_page(page);
> + if (from_pagecache)
> put_page(page);
> - } else
> + else
> __free_pages(page, 0);
^^^^^^^^^^^^
If we get page as the function parameter we will end up freeing it here,
which isn't what we want. We can probably keep a local page variable so
that we compare it with whatever we received as parameter.
Cheers
--
Luis
