From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 38248C43142 for ; Sun, 24 Jun 2018 01:58:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E3AA424D80 for ; Sun, 24 Jun 2018 01:58:45 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E3AA424D80 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=talpey.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752014AbeFXB6o (ORCPT ); Sat, 23 Jun 2018 21:58:44 -0400 Received: from p3plsmtpa12-09.prod.phx3.secureserver.net ([68.178.252.238]:36260 "EHLO p3plsmtpa12-09.prod.phx3.secureserver.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751869AbeFXB6m (ORCPT ); Sat, 23 Jun 2018 21:58:42 -0400 Received: from [192.168.0.67] ([24.218.182.144]) by :SMTPAUTH: with ESMTPSA id WuIafdSTA669vWuIafOV0L; Sat, 23 Jun 2018 18:58:41 -0700 Subject: Re: [Patch v2 03/15] CIFS: Use offset when reading pages To: longli@microsoft.com, Steve French , linux-cifs@vger.kernel.org, samba-technical@lists.samba.org, linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org References: <20180530194807.31657-1-longli@linuxonhyperv.com> <20180530194807.31657-4-longli@linuxonhyperv.com> From: Tom Talpey Message-ID: Date: Sat, 23 Jun 2018 21:58:41 -0400 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <20180530194807.31657-4-longli@linuxonhyperv.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-CMAE-Envelope: MS4wfDkiRFK9/EpuaL+aDDvxlsChUV0YwZx+XTo00ZG7ra9j4QFkj99fNBxqTeI8ooumcpj3BHMLAGppvCSzjMOKsIpDEbTJfiORiWsZ8S5pUAlesZ5LQbtP eYXFklpejcfE8EJNxEjIJTuMV1U5J8l1UK4oKXyFMJnRBTFgd9rlyTdxWo0dkYzRHL5Opa9/QmaJeEvsqdnMx9v8kL6SgDO01ibAxOcmVj9R1LfVeV8udAgw 78lZbzSo7vxTnl2Oo3EO6KsCKuQ6LngKWiMiTlzhNoHy36jwXPXcr3DC+fOPRuMvZ86C2mWSAjhFBmc9EOFYsLLeqU8B7rtsBwTQ91YlhTe+oE1K2K+bpEdq vMsP5V0e Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 5/30/2018 3:47 PM, Long Li wrote: > From: Long Li > > With offset defined in rdata, transport functions need to look at this > offset when reading data into the correct places in pages. > > Signed-off-by: Long Li > --- > fs/cifs/cifsproto.h | 4 +++- > fs/cifs/connect.c | 5 +++-- > fs/cifs/file.c | 52 +++++++++++++++++++++++++++++++++++++--------------- > fs/cifs/smb2ops.c | 2 +- > fs/cifs/smb2pdu.c | 1 + > 5 files changed, 45 insertions(+), 19 deletions(-) > > diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h > index dc80f84..1f27d8e 100644 > --- a/fs/cifs/cifsproto.h > +++ b/fs/cifs/cifsproto.h > @@ -203,7 +203,9 @@ extern void dequeue_mid(struct mid_q_entry *mid, bool malformed); > extern int cifs_read_from_socket(struct TCP_Server_Info *server, char *buf, > unsigned int to_read); > extern int cifs_read_page_from_socket(struct TCP_Server_Info *server, > - struct page *page, unsigned int to_read); > + struct page *page, > + unsigned int page_offset, > + unsigned int to_read); > extern int cifs_setup_cifs_sb(struct smb_vol *pvolume_info, > struct cifs_sb_info *cifs_sb); > extern int cifs_match_super(struct super_block *, void *); > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > index 83b0234..8501da0 100644 > --- a/fs/cifs/connect.c > +++ b/fs/cifs/connect.c > @@ -594,10 +594,11 @@ cifs_read_from_socket(struct TCP_Server_Info *server, char *buf, > > int > cifs_read_page_from_socket(struct TCP_Server_Info *server, struct page *page, > - unsigned int to_read) > + unsigned int page_offset, unsigned int to_read) > { > struct msghdr smb_msg; > - struct bio_vec bv = {.bv_page = page, .bv_len = to_read}; > + struct bio_vec bv = { > + .bv_page = page, .bv_len = to_read, .bv_offset = page_offset}; > iov_iter_bvec(&smb_msg.msg_iter, READ | ITER_BVEC, &bv, 1, to_read); > return cifs_readv_from_socket(server, &smb_msg); > } > diff --git a/fs/cifs/file.c b/fs/cifs/file.c > index 1c98293..87eece6 100644 > --- a/fs/cifs/file.c > +++ b/fs/cifs/file.c > @@ -3026,12 +3026,20 @@ uncached_fill_pages(struct TCP_Server_Info *server, > int result = 0; > unsigned int i; > unsigned int nr_pages = rdata->nr_pages; > + unsigned int page_offset = rdata->page_offset; > > rdata->got_bytes = 0; > rdata->tailsz = PAGE_SIZE; > for (i = 0; i < nr_pages; i++) { > struct page *page = rdata->pages[i]; > size_t n; > + unsigned int segment_size = rdata->pagesz; > + > + if (i == 0) > + segment_size -= page_offset; > + else > + page_offset = 0; > + > > if (len <= 0) { > /* no need to hold page hostage */ > @@ -3040,24 +3048,25 @@ uncached_fill_pages(struct TCP_Server_Info *server, > put_page(page); > continue; > } > + > n = len; > - if (len >= PAGE_SIZE) { > + if (len >= segment_size) > /* enough data to fill the page */ > - n = PAGE_SIZE; > - len -= n; > - } else { > - zero_user(page, len, PAGE_SIZE - len); > + n = segment_size; > + else > rdata->tailsz = len; > - len = 0; > - } > + len -= n; > + > if (iter) > - result = copy_page_from_iter(page, 0, n, iter); > + result = copy_page_from_iter( > + page, page_offset, n, iter); > #ifdef CONFIG_CIFS_SMB_DIRECT > else if (rdata->mr) > result = n; > #endif I hadn't noticed this type of xonditional code before. It's kind of ugly to modify the data handling code like this. Do you plan to break this out into an smbdirect-specific hander, like the cases above and below? > else > - result = cifs_read_page_from_socket(server, page, n); > + result = cifs_read_page_from_socket( > + server, page, page_offset, n); > if (result < 0) > break; > > @@ -3130,6 +3139,7 @@ cifs_send_async_read(loff_t offset, size_t len, struct cifsFileInfo *open_file, > rdata->bytes = cur_len; > rdata->pid = pid; > rdata->pagesz = PAGE_SIZE; > + rdata->tailsz = PAGE_SIZE; > rdata->read_into_pages = cifs_uncached_read_into_pages; > rdata->copy_into_pages = cifs_uncached_copy_into_pages; > rdata->credits = credits; > @@ -3574,6 +3584,7 @@ readpages_fill_pages(struct TCP_Server_Info *server, > u64 eof; > pgoff_t eof_index; > unsigned int nr_pages = rdata->nr_pages; > + unsigned int page_offset = rdata->page_offset; > > /* determine the eof that the server (probably) has */ > eof = CIFS_I(rdata->mapping->host)->server_eof; > @@ -3584,13 +3595,21 @@ readpages_fill_pages(struct TCP_Server_Info *server, > rdata->tailsz = PAGE_SIZE; > for (i = 0; i < nr_pages; i++) { > struct page *page = rdata->pages[i]; > - size_t n = PAGE_SIZE; > + unsigned int to_read = rdata->pagesz; > + size_t n; > + > + if (i == 0) > + to_read -= page_offset; > + else > + page_offset = 0; > + > + n = to_read; > > - if (len >= PAGE_SIZE) { > - len -= PAGE_SIZE; > + if (len >= to_read) { > + len -= to_read; > } else if (len > 0) { > /* enough for partial page, fill and zero the rest */ > - zero_user(page, len, PAGE_SIZE - len); > + zero_user(page, len + page_offset, to_read - len); > n = rdata->tailsz = len; > len = 0; > } else if (page->index > eof_index) { > @@ -3622,13 +3641,15 @@ readpages_fill_pages(struct TCP_Server_Info *server, > } > > if (iter) > - result = copy_page_from_iter(page, 0, n, iter); > + result = copy_page_from_iter( > + page, page_offset, n, iter); > #ifdef CONFIG_CIFS_SMB_DIRECT > else if (rdata->mr) > result = n; > #endif Ditto previous comment/question. > else > - result = cifs_read_page_from_socket(server, page, n); > + result = cifs_read_page_from_socket( > + server, page, page_offset, n); > if (result < 0) > break; > > @@ -3807,6 +3828,7 @@ static int cifs_readpages(struct file *file, struct address_space *mapping, > rdata->bytes = bytes; > rdata->pid = pid; > rdata->pagesz = PAGE_SIZE; > + rdata->tailsz = PAGE_SIZE; > rdata->read_into_pages = cifs_readpages_read_into_pages; > rdata->copy_into_pages = cifs_readpages_copy_into_pages; > rdata->credits = credits; > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c > index 7c0edd2..1fa1c29 100644 > --- a/fs/cifs/smb2ops.c > +++ b/fs/cifs/smb2ops.c > @@ -2467,7 +2467,7 @@ read_data_into_pages(struct TCP_Server_Info *server, struct page **pages, > zero_user(page, len, PAGE_SIZE - len); > len = 0; > } > - length = cifs_read_page_from_socket(server, page, n); > + length = cifs_read_page_from_socket(server, page, 0, n); > if (length < 0) > return length; > server->total_read += length; > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c > index a02f6b6..f603fbe 100644 > --- a/fs/cifs/smb2pdu.c > +++ b/fs/cifs/smb2pdu.c > @@ -2683,6 +2683,7 @@ smb2_readv_callback(struct mid_q_entry *mid) > struct smb_rqst rqst = { .rq_iov = rdata->iov, > .rq_nvec = 2, > .rq_pages = rdata->pages, > + .rq_offset = rdata->page_offset, > .rq_npages = rdata->nr_pages, > .rq_pagesz = rdata->pagesz, > .rq_tailsz = rdata->tailsz }; >