From patchwork Mon Apr 23 17:50:49 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steve French X-Patchwork-Id: 10357857 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id DBA8C60209 for ; Mon, 23 Apr 2018 17:51:27 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C986F28BF2 for ; Mon, 23 Apr 2018 17:51:27 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id BE43928BF6; Mon, 23 Apr 2018 17:51:27 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.8 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, T_TVD_MIME_EPI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 23EF228BF4 for ; Mon, 23 Apr 2018 17:51:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932364AbeDWRvO (ORCPT ); Mon, 23 Apr 2018 13:51:14 -0400 Received: from mail-pg0-f54.google.com ([74.125.83.54]:36553 "EHLO mail-pg0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932157AbeDWRvL (ORCPT ); Mon, 23 Apr 2018 13:51:11 -0400 Received: by mail-pg0-f54.google.com with SMTP id i6so8896153pgv.3; Mon, 23 Apr 2018 10:51:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=97ZeR8SxsWdjOI6At50WvAeabKvcUyhf82IbB6ZUSYA=; b=KpibAkOsxMS2TILyYmYOEP4gnycP6W6xsvFZv2nTTq3suxXLejy6mhy3pOC8nbH1VZ bKza7O2L9bKoUhzG3YAx8KGovaAF7nHSANXZIq1F+Hc1Oc5ZfGF0RUedNVoBrEzZGqi1 OvSifWWYB62XJkaA6964mX72egaSsQkCALEb6H4AAQmf/DwoBUkWExQ6iNuQxhLxgzRT AOYtOzIR2Npv3FtmiTm4Sc+vSNkONHNL5S/LQ8mzLDlRjeU5i7wuRMbxWsz8NBTcm1lr 2khgBYDOfEvNmK9j3TH1yvIVgq4rwv5WOrmmn0x+tl14Ut3tgWNB6ZrdgiBfKvoITGXc PRrA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=97ZeR8SxsWdjOI6At50WvAeabKvcUyhf82IbB6ZUSYA=; b=nmqxxKtszBUHihpOJNH3Opnc1steG4VDj4MnsiDNrhkJGmu0pmbIhBP5xzPxVDiNVs jk4PP75bBve0jiLX+STx4nTSuynMaARRNTHMK4z9RjBGPzWopYZqb/qlgbxUwxd5oG8D zR31VUr6h82u6v9MTfFoy1EQX9CWc8TSJ8RWTPqReigiKsqsdvdQLDpg7F13edxUaFM7 oHQKgVYYTUyat5Y+nmSR/tOddP8ZofFw1WDvE/Ln+OebP3cAXtmU1AFnb3+PAxekS0L/ lmKDwxJHlHDukzhzpgDXhrozdUh7tE8/e4yrtM8PlWuLHPRQlhJXSP7wLjn1ByI3M0+f whag== X-Gm-Message-State: ALQs6tDq+uyM1w8FA/QVdKVtGa+XpgKdgida1I9T8pU7a1C8a8P3fWB4 yDFp68cJGVElyCArNLguZ9r1fO5duG16FYabcqw= X-Google-Smtp-Source: AIpwx49+UscXW14s738G6wLZwqNrcASiGXMajTezGywNA6MCF2m5ea2HlxTucSNRxgQ5o964AWwApxQInQVQz/ILxuI= X-Received: by 2002:a17:902:a603:: with SMTP id u3-v6mr21931978plq.214.1524505870079; Mon, 23 Apr 2018 10:51:10 -0700 (PDT) MIME-Version: 1.0 Received: by 10.100.152.97 with HTTP; Mon, 23 Apr 2018 10:50:49 -0700 (PDT) In-Reply-To: References: <20180417191710.14855-1-longli@linuxonhyperv.com> <20180417191710.14855-3-longli@linuxonhyperv.com> From: Steve French Date: Mon, 23 Apr 2018 12:50:49 -0500 Message-ID: Subject: Re: [Patch v2 3/6] cifs: smbd: Avoid allocating iov on the stack To: Long Li Cc: CIFS , samba-technical , LKML , "linux-rdma@vger.kernel.org" Sender: linux-cifs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP I am ok with the other style warning - so I just fixed up the one error (see attached) and merged into cifs-2.6.git for-next That leaves two more from your series to review (and for others to review). Let's just focus on those two. On Mon, Apr 23, 2018 at 12:33 PM, Long Li wrote: >> Subject: Re: [Patch v2 3/6] cifs: smbd: Avoid allocating iov on the stack >> >> Didn't see any obvious problems, but can you fix the checkpatch warnings >> and resend to the list (I am more concerned about the last two warnings >> rather than the first one). > > Yes, I will fix it and resend. > >> >> $ scripts/checkpatch.pl 0001-cifs-smbd-Avoid-allocating-iov-on-the- >> stack.patch >> WARNING: line over 80 characters >> #60: FILE: fs/cifs/smbdirect.c:2106: >> + log_write(ERR, "expected the pdu length in 1st iov, but got >> 0x%lu\n", rqst->rq_iov[0].iov_len); >> >> ERROR: Prefixing 0x with decimal output is defective >> #60: FILE: fs/cifs/smbdirect.c:2106: >> + log_write(ERR, "expected the pdu length in 1st iov, but got >> 0x%lu\n", rqst->rq_iov[0].iov_len); >> >> WARNING: braces {} are not necessary for single statement blocks >> #69: FILE: fs/cifs/smbdirect.c:2112: >> + for (i = 0; i < rqst->rq_nvec-1; i++) { >> buflen += iov[i].iov_len; >> } >> >> total: 1 errors, 2 warnings, 65 lines checked >> >> NOTE: For some of the reported defects, checkpatch may be able to >> mechanically convert to the typical style using --fix or --fix-inplace. >> >> 0001-cifs-smbd-Avoid-allocating-iov-on-the-stack.patch has style problems, >> please review. >> >> On Tue, Apr 17, 2018 at 2:17 PM, Long Li wrote: >> > From: Long Li >> > >> > It's not necessary to allocate another iov when going through the >> > buffers in smbd_send() through RDMA send. >> > >> > Remove it to reduce stack size. >> > >> > Signed-off-by: Long Li >> > Cc: stable@vger.kernel.org >> > --- >> > fs/cifs/smbdirect.c | 36 ++++++++++++------------------------ >> > 1 file changed, 12 insertions(+), 24 deletions(-) >> > >> > diff --git a/fs/cifs/smbdirect.c b/fs/cifs/smbdirect.c index >> > b5c6c0d..f575e9a 100644 >> > --- a/fs/cifs/smbdirect.c >> > +++ b/fs/cifs/smbdirect.c >> > @@ -2088,7 +2088,7 @@ int smbd_send(struct smbd_connection *info, >> struct smb_rqst *rqst) >> > int start, i, j; >> > int max_iov_size = >> > info->max_send_size - sizeof(struct smbd_data_transfer); >> > - struct kvec iov[SMBDIRECT_MAX_SGE]; >> > + struct kvec *iov; >> > int rc; >> > >> > info->smbd_send_pending++; >> > @@ -2099,32 +2099,20 @@ int smbd_send(struct smbd_connection *info, >> struct smb_rqst *rqst) >> > } >> > >> > /* >> > - * This usually means a configuration error >> > - * We use RDMA read/write for packet size > >> rdma_readwrite_threshold >> > - * as long as it's properly configured we should never get into this >> > - * situation >> > - */ >> > - if (rqst->rq_nvec + rqst->rq_npages > SMBDIRECT_MAX_SGE) { >> > - log_write(ERR, "maximum send segment %x exceeding %x\n", >> > - rqst->rq_nvec + rqst->rq_npages, SMBDIRECT_MAX_SGE); >> > - rc = -EINVAL; >> > - goto done; >> > - } >> > - >> > - /* >> > - * Remove the RFC1002 length defined in MS-SMB2 section 2.1 >> > - * It is used only for TCP transport >> > + * Skip the RFC1002 length defined in MS-SMB2 section 2.1 >> > + * It is used only for TCP transport in the iov[0] >> > * In future we may want to add a transport layer under protocol >> > * layer so this will only be issued to TCP transport >> > */ >> > - iov[0].iov_base = (char *)rqst->rq_iov[0].iov_base + 4; >> > - iov[0].iov_len = rqst->rq_iov[0].iov_len - 4; >> > - buflen += iov[0].iov_len; >> > + >> > + if (rqst->rq_iov[0].iov_len != 4) { >> > + log_write(ERR, "expected the pdu length in 1st iov, but got >> 0x%lu\n", rqst->rq_iov[0].iov_len); >> > + return -EINVAL; >> > + } >> > + iov = &rqst->rq_iov[1]; >> > >> > /* total up iov array first */ >> > - for (i = 1; i < rqst->rq_nvec; i++) { >> > - iov[i].iov_base = rqst->rq_iov[i].iov_base; >> > - iov[i].iov_len = rqst->rq_iov[i].iov_len; >> > + for (i = 0; i < rqst->rq_nvec-1; i++) { >> > buflen += iov[i].iov_len; >> > } >> > >> > @@ -2197,14 +2185,14 @@ int smbd_send(struct smbd_connection *info, >> struct smb_rqst *rqst) >> > goto done; >> > } >> > i++; >> > - if (i == rqst->rq_nvec) >> > + if (i == rqst->rq_nvec-1) >> > break; >> > } >> > start = i; >> > buflen = 0; >> > } else { >> > i++; >> > - if (i == rqst->rq_nvec) { >> > + if (i == rqst->rq_nvec-1) { >> > /* send out all remaining vecs */ >> > remaining_data_length -= buflen; >> > log_write(INFO, >> > -- >> > 2.7.4 >> > >> > -- >> > To unsubscribe from this list: send the line "unsubscribe linux-cifs" >> > in the body of a message to majordomo@vger.kernel.org More >> majordomo >> > info at >> > >> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.k >> > ernel.org%2Fmajordomo- >> info.html&data=02%7C01%7Clongli%40microsoft.com% >> > >> 7C23fbdbb0598a497f624708d5a92f6435%7C72f988bf86f141af91ab2d7cd011d >> b47% >> > >> 7C1%7C0%7C636600943380887988&sdata=imh9cWDQQEXqxHdUrCuEykgiLfKs >> UAl20jB >> > yPOS7FrI%3D&reserved=0 >> >> >> >> -- >> Thanks, >> >> Steve >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the >> body of a message to majordomo@vger.kernel.org More majordomo info at >> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.ke >> rnel.org%2Fmajordomo- >> info.html&data=02%7C01%7Clongli%40microsoft.com%7C23fbdbb0598a497f >> 624708d5a92f6435%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63 >> 6600943380887988&sdata=imh9cWDQQEXqxHdUrCuEykgiLfKsUAl20jByPOS7F >> rI%3D&reserved=0 From 553018657de265a7c999f4fd12864fd0646319a1 Mon Sep 17 00:00:00 2001 From: Long Li Date: Tue, 17 Apr 2018 12:17:07 -0700 Subject: [PATCH] cifs: smbd: Avoid allocating iov on the stack It's not necessary to allocate another iov when going through the buffers in smbd_send() through RDMA send. Remove it to reduce stack size. Signed-off-by: Long Li Cc: stable@vger.kernel.org --- fs/cifs/smbdirect.c | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/fs/cifs/smbdirect.c b/fs/cifs/smbdirect.c index 87817ddcc096..24cea63e17f5 100644 --- a/fs/cifs/smbdirect.c +++ b/fs/cifs/smbdirect.c @@ -2086,7 +2086,7 @@ int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst) int start, i, j; int max_iov_size = info->max_send_size - sizeof(struct smbd_data_transfer); - struct kvec iov[SMBDIRECT_MAX_SGE]; + struct kvec *iov; int rc; info->smbd_send_pending++; @@ -2096,32 +2096,20 @@ int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst) } /* - * This usually means a configuration error - * We use RDMA read/write for packet size > rdma_readwrite_threshold - * as long as it's properly configured we should never get into this - * situation - */ - if (rqst->rq_nvec + rqst->rq_npages > SMBDIRECT_MAX_SGE) { - log_write(ERR, "maximum send segment %x exceeding %x\n", - rqst->rq_nvec + rqst->rq_npages, SMBDIRECT_MAX_SGE); - rc = -EINVAL; - goto done; - } - - /* - * Remove the RFC1002 length defined in MS-SMB2 section 2.1 - * It is used only for TCP transport + * Skip the RFC1002 length defined in MS-SMB2 section 2.1 + * It is used only for TCP transport in the iov[0] * In future we may want to add a transport layer under protocol * layer so this will only be issued to TCP transport */ - iov[0].iov_base = (char *)rqst->rq_iov[0].iov_base + 4; - iov[0].iov_len = rqst->rq_iov[0].iov_len - 4; - buflen += iov[0].iov_len; + + if (rqst->rq_iov[0].iov_len != 4) { + log_write(ERR, "expected the pdu length in 1st iov, but got %lu\n", rqst->rq_iov[0].iov_len); + return -EINVAL; + } + iov = &rqst->rq_iov[1]; /* total up iov array first */ - for (i = 1; i < rqst->rq_nvec; i++) { - iov[i].iov_base = rqst->rq_iov[i].iov_base; - iov[i].iov_len = rqst->rq_iov[i].iov_len; + for (i = 0; i < rqst->rq_nvec-1; i++) { buflen += iov[i].iov_len; } @@ -2198,14 +2186,14 @@ int smbd_send(struct smbd_connection *info, struct smb_rqst *rqst) goto done; } i++; - if (i == rqst->rq_nvec) + if (i == rqst->rq_nvec-1) break; } start = i; buflen = 0; } else { i++; - if (i == rqst->rq_nvec) { + if (i == rqst->rq_nvec-1) { /* send out all remaining vecs */ remaining_data_length -= buflen; log_write(INFO, -- 2.14.1