Re: [PATCH v14 58/64] ceph: add encryption support to writepage

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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:' ?

-- Xiubo


Good 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 ?

-- Xiubo

Definitely:

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;




[Index of Archives]     [CEPH Users]     [Ceph Large]     [Ceph Dev]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux