On 5/5/22 7:12 PM, Jeff Layton wrote:
On Thu, 2022-05-05 at 19:05 +0800, Xiubo Li wrote:On 5/5/22 6:53 PM, Jeff Layton wrote:On Thu, 2022-05-05 at 17:34 +0800, Xiubo Li wrote:On 4/28/22 3:13 AM, Jeff Layton wrote:Allow writepage to issue encrypted writes. Extend out the requested size and offset to cover complete blocks, and then encrypt and write them to the OSDs. Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> --- fs/ceph/addr.c | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c index d65d431ec933..f54940fc96ee 100644 --- a/fs/ceph/addr.c +++ b/fs/ceph/addr.c @@ -586,10 +586,12 @@ static int writepage_nounlock(struct page *page, struct writeback_control *wbc) loff_t page_off = page_offset(page); int err; loff_t len = thp_size(page); + loff_t wlen; struct ceph_writeback_ctl ceph_wbc; struct ceph_osd_client *osdc = &fsc->client->osdc; struct ceph_osd_request *req; bool caching = ceph_is_cache_enabled(inode); + struct page *bounce_page = NULL;dout("writepage %p idx %lu\n", page, page->index); @@ -621,6 +623,8 @@ static int writepage_nounlock(struct page *page, struct writeback_control *wbc) if (ceph_wbc.i_size < page_off + len)len = ceph_wbc.i_size - page_off; + if (IS_ENCRYPTED(inode)) + wlen = round_up(len, CEPH_FSCRYPT_BLOCK_SIZE);dout("writepage %p page %p index %lu on %llu~%llu snapc %p seq %lld\n",inode, page, page->index, page_off, len, snapc, snapc->seq); @@ -629,24 +633,39 @@ static int writepage_nounlock(struct page *page, struct writeback_control *wbc) CONGESTION_ON_THRESH(fsc->mount_options->congestion_kb)) fsc->write_congested = true;- req = ceph_osdc_new_request(osdc, &ci->i_layout, ceph_vino(inode), page_off, &len, 0, 1,- CEPH_OSD_OP_WRITE, CEPH_OSD_FLAG_WRITE, snapc, - ceph_wbc.truncate_seq, ceph_wbc.truncate_size, - true); + req = ceph_osdc_new_request(osdc, &ci->i_layout, ceph_vino(inode), + page_off, &wlen, 0, 1, CEPH_OSD_OP_WRITE, + CEPH_OSD_FLAG_WRITE, snapc, + ceph_wbc.truncate_seq, + ceph_wbc.truncate_size, true); if (IS_ERR(req)) { redirty_page_for_writepage(wbc, page); return PTR_ERR(req); }+ if (wlen < len)+ len = wlen; + set_page_writeback(page); if (caching) ceph_set_page_fscache(page); ceph_fscache_write_to_cache(inode, page_off, len, caching);+ if (IS_ENCRYPTED(inode)) {+ bounce_page = fscrypt_encrypt_pagecache_blocks(page, CEPH_FSCRYPT_BLOCK_SIZE, + 0, GFP_NOFS); + if (IS_ERR(bounce_page)) { + err = PTR_ERR(bounce_page); + goto out; + } + }Here IMO we should redirty the page instead of detaching the page's private data in 'out:' ? -- XiuboGood catch. I think you're right. I'll fold the following delta into this patch: diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c index f54940fc96ee..b266656f2951 100644 --- a/fs/ceph/addr.c +++ b/fs/ceph/addr.c @@ -655,10 +655,12 @@ static int writepage_nounlock(struct page *page, struct writeback_control *wbc) bounce_page = fscrypt_encrypt_pagecache_blocks(page, CEPH_FSCRYPT_BLOCK_SIZE, 0, GFP_NOFS); if (IS_ERR(bounce_page)) { - err = PTR_ERR(bounce_page); - goto out; + redirty_page_for_writepage(wbc, page); + end_page_writeback(page); + return PTR_ERR(bounce_page);You should also call ceph_osdc_put_request() too ? -- XiuboDefinitely: diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c index f54940fc96ee..4dfa2892f3f5 100644 --- a/fs/ceph/addr.c +++ b/fs/ceph/addr.c @@ -655,10 +655,13 @@ static int writepage_nounlock(struct page *page, struct writeback_control *wbc) bounce_page = fscrypt_encrypt_pagecache_blocks(page, CEPH_FSCRYPT_BLOCK_SIZE, 0, GFP_NOFS); if (IS_ERR(bounce_page)) { - err = PTR_ERR(bounce_page); - goto out; + redirty_page_for_writepage(wbc, page); + end_page_writeback(page); + ceph_osdc_put_request(req); + return PTR_ERR(bounce_page); } } + /* it may be a short write due to an object boundary */ WARN_ON_ONCE(len > thp_size(page)); osd_req_op_extent_osd_data_pages(req, 0,
Please remove the 'out' tag, it's not needed any more. The others LGTM. -- Xiubo
} } + /* it may be a short write due to an object boundary */ WARN_ON_ONCE(len > thp_size(page)); osd_req_op_extent_osd_data_pages(req, 0,/* it may be a short write due to an object boundary */ WARN_ON_ONCE(len > thp_size(page)); - osd_req_op_extent_osd_data_pages(req, 0, &page, len, 0, false, false); - dout("writepage %llu~%llu (%llu bytes)\n", page_off, len, len); + osd_req_op_extent_osd_data_pages(req, 0, + bounce_page ? &bounce_page : &page, wlen, 0, + false, false); + dout("writepage %llu~%llu (%llu bytes, %sencrypted)\n", + page_off, len, wlen, IS_ENCRYPTED(inode) ? "" : "not ");req->r_mtime = inode->i_mtime;err = ceph_osdc_start_request(osdc, req, true); @@ -655,7 +674,8 @@ static int writepage_nounlock(struct page *page, struct writeback_control *wbc)ceph_update_write_metrics(&fsc->mdsc->metric, req->r_start_latency,req->r_end_latency, len, err); - + fscrypt_free_bounce_page(bounce_page); +out: ceph_osdc_put_request(req); if (err == 0) err = len;